From bbffdcd0ffe1702734c35e244f7b1f34afa99159 Mon Sep 17 00:00:00 2001 From: Sheng Chen Date: Thu, 18 Jun 2026 14:47:45 +0800 Subject: [PATCH 01/14] fix: update @github/copilot-language-server dependencies to version 1.502.5 (#296) --- .../copilot-agent/package-lock.json | 66 +++++++++---------- .../copilot-agent/package.json | 12 ++-- 2 files changed, 39 insertions(+), 39 deletions(-) diff --git a/com.microsoft.copilot.eclipse.core/copilot-agent/package-lock.json b/com.microsoft.copilot.eclipse.core/copilot-agent/package-lock.json index 07efeace..f389abb6 100644 --- a/com.microsoft.copilot.eclipse.core/copilot-agent/package-lock.json +++ b/com.microsoft.copilot.eclipse.core/copilot-agent/package-lock.json @@ -9,18 +9,18 @@ "version": "0.0.1", "hasInstallScript": true, "dependencies": { - "@github/copilot-language-server": "1.488.0", - "@github/copilot-language-server-darwin-arm64": "1.488.0", - "@github/copilot-language-server-darwin-x64": "1.488.0", - "@github/copilot-language-server-linux-arm64": "1.488.0", - "@github/copilot-language-server-linux-x64": "1.488.0", - "@github/copilot-language-server-win32-x64": "1.488.0" + "@github/copilot-language-server": "1.502.5", + "@github/copilot-language-server-darwin-arm64": "1.502.5", + "@github/copilot-language-server-darwin-x64": "1.502.5", + "@github/copilot-language-server-linux-arm64": "1.502.5", + "@github/copilot-language-server-linux-x64": "1.502.5", + "@github/copilot-language-server-win32-x64": "1.502.5" } }, "node_modules/@github/copilot-language-server": { - "version": "1.488.0", - "resolved": "https://registry.npmjs.org/@github/copilot-language-server/-/copilot-language-server-1.488.0.tgz", - "integrity": "sha512-zCyItvYtqrtQzpdAv6nTjph4Nfws5xTMNAw7cn2gIvBEBHT5NbnaceSwxJm2I96Ll2Jrq4uQG+wifYVMHjQDwg==", + "version": "1.502.5", + "resolved": "https://registry.npmjs.org/@github/copilot-language-server/-/copilot-language-server-1.502.5.tgz", + "integrity": "sha512-vIFbb145TwhDD1KTdJjySAokBcJla6NWsfvqjotM2AsDu3lLtQjPFbt6P90vnMVbQ6Ixc/IbDqsjToaWmXqqIA==", "license": "MIT", "dependencies": { "vscode-languageserver-protocol": "^3.17.5" @@ -29,18 +29,18 @@ "copilot-language-server": "dist/language-server.js" }, "optionalDependencies": { - "@github/copilot-language-server-darwin-arm64": "1.488.0", - "@github/copilot-language-server-darwin-x64": "1.488.0", - "@github/copilot-language-server-linux-arm64": "1.488.0", - "@github/copilot-language-server-linux-x64": "1.488.0", - "@github/copilot-language-server-win32-arm64": "1.488.0", - "@github/copilot-language-server-win32-x64": "1.488.0" + "@github/copilot-language-server-darwin-arm64": "1.502.5", + "@github/copilot-language-server-darwin-x64": "1.502.5", + "@github/copilot-language-server-linux-arm64": "1.502.5", + "@github/copilot-language-server-linux-x64": "1.502.5", + "@github/copilot-language-server-win32-arm64": "1.502.5", + "@github/copilot-language-server-win32-x64": "1.502.5" } }, "node_modules/@github/copilot-language-server-darwin-arm64": { - "version": "1.488.0", - "resolved": "https://registry.npmjs.org/@github/copilot-language-server-darwin-arm64/-/copilot-language-server-darwin-arm64-1.488.0.tgz", - "integrity": "sha512-X4jqAKJwNJIM2Ua22UZvPM6/3x7ZiX4lgedvqWadoFA9SJmxF6OlQoe2L6coeC1U8RWj5WEyTqIDkyJkhsF24Q==", + "version": "1.502.5", + "resolved": "https://registry.npmjs.org/@github/copilot-language-server-darwin-arm64/-/copilot-language-server-darwin-arm64-1.502.5.tgz", + "integrity": "sha512-3QTwdHjX2qpOwRhKIcXj3PU+wr5FodMcHPRnXK2qMv4xiRtibuuqicFVPF7ckO7Xym2o2FfJHfS8e58NNl278w==", "cpu": [ "arm64" ], @@ -50,9 +50,9 @@ ] }, "node_modules/@github/copilot-language-server-darwin-x64": { - "version": "1.488.0", - "resolved": "https://registry.npmjs.org/@github/copilot-language-server-darwin-x64/-/copilot-language-server-darwin-x64-1.488.0.tgz", - "integrity": "sha512-JHmdBwxwfZfxWkJLNp4M2bydrkD/TsxPoPeojcIOscrHhTj2riWSe7zqUn61IAdtwG0z2HwK1tGS35KOJhW8+A==", + "version": "1.502.5", + "resolved": "https://registry.npmjs.org/@github/copilot-language-server-darwin-x64/-/copilot-language-server-darwin-x64-1.502.5.tgz", + "integrity": "sha512-qz3aGl2KtAA/pM65esyXrsv4wBDFoKwUrP4hBAm2XplIPVF931HOluCXGxL0GDU2OMsEAxgFy5xTGODMg1U7kQ==", "cpu": [ "x64" ], @@ -62,9 +62,9 @@ ] }, "node_modules/@github/copilot-language-server-linux-arm64": { - "version": "1.488.0", - "resolved": "https://registry.npmjs.org/@github/copilot-language-server-linux-arm64/-/copilot-language-server-linux-arm64-1.488.0.tgz", - "integrity": "sha512-t+MZuQl76nMAs0qzAZDb9hr3SC53cW87CmDI+lgfLfXay8cdunNirjI/IloRnXybYYKFCkjvqDl9cYcLdCA1/g==", + "version": "1.502.5", + "resolved": "https://registry.npmjs.org/@github/copilot-language-server-linux-arm64/-/copilot-language-server-linux-arm64-1.502.5.tgz", + "integrity": "sha512-8xabGJgGzpa4KVDQc1ZYH3elTzyzVRUWJLlBBQbK4vSpNtsnmGOk4TkY7uJZ7uXtaX1/xDD1venbPWWNOABCig==", "cpu": [ "arm64" ], @@ -74,9 +74,9 @@ ] }, "node_modules/@github/copilot-language-server-linux-x64": { - "version": "1.488.0", - "resolved": "https://registry.npmjs.org/@github/copilot-language-server-linux-x64/-/copilot-language-server-linux-x64-1.488.0.tgz", - "integrity": "sha512-hhJ2ZVweqLCk0lptMXyHlXPrkSiHypcpza5gVUwMGMuw7Z87o3SkK8fD5jDpBvKJ0gJTGjPZ2yXzGtErpY20Sw==", + "version": "1.502.5", + "resolved": "https://registry.npmjs.org/@github/copilot-language-server-linux-x64/-/copilot-language-server-linux-x64-1.502.5.tgz", + "integrity": "sha512-7JcmuKj4yOoDjU3kx4JlVPzC2jDES6nFFqUoNAEnKPFPUjIwf4RJKI2heonIUtFnFXZ9JWT09vqe477oO0F80w==", "cpu": [ "x64" ], @@ -86,9 +86,9 @@ ] }, "node_modules/@github/copilot-language-server-win32-arm64": { - "version": "1.488.0", - "resolved": "https://registry.npmjs.org/@github/copilot-language-server-win32-arm64/-/copilot-language-server-win32-arm64-1.488.0.tgz", - "integrity": "sha512-ke8czQwJcOtZpD9CnyBE7nVrFU9oa8nrLMp8xWoxKybs77Rpt36v2CGw535QHrih9f3Hp4dQtFlaRdvjnL1vZg==", + "version": "1.502.5", + "resolved": "https://registry.npmjs.org/@github/copilot-language-server-win32-arm64/-/copilot-language-server-win32-arm64-1.502.5.tgz", + "integrity": "sha512-7/QHmdWSIOKvbr5s5tH1u8/nQ+AfvqqGbPlpZeokv/hTVwCZZ69ntMVGkoSQVKcSAYVL2h97M5+0zNbofYitow==", "cpu": [ "arm64" ], @@ -99,9 +99,9 @@ ] }, "node_modules/@github/copilot-language-server-win32-x64": { - "version": "1.488.0", - "resolved": "https://registry.npmjs.org/@github/copilot-language-server-win32-x64/-/copilot-language-server-win32-x64-1.488.0.tgz", - "integrity": "sha512-TOxA86DcpO7VYfsePj5PmbzqKaTwGXse6OBdMAPKPBBBCiusEI1kmYO3O/ywi8n5cCF1X9LvrqbunNRzokxRug==", + "version": "1.502.5", + "resolved": "https://registry.npmjs.org/@github/copilot-language-server-win32-x64/-/copilot-language-server-win32-x64-1.502.5.tgz", + "integrity": "sha512-9jdvyPZnu2muS9Jo6LtmP33NkpTht/2Gc9pWRrkQizuNabWy7jq2KuJdYf1/vwiyEioMiZznGBv+hscrJ9z5LA==", "cpu": [ "x64" ], diff --git a/com.microsoft.copilot.eclipse.core/copilot-agent/package.json b/com.microsoft.copilot.eclipse.core/copilot-agent/package.json index 06f8e77a..7702b184 100644 --- a/com.microsoft.copilot.eclipse.core/copilot-agent/package.json +++ b/com.microsoft.copilot.eclipse.core/copilot-agent/package.json @@ -12,11 +12,11 @@ "postinstall": "node copy-binaries.js" }, "dependencies": { - "@github/copilot-language-server": "1.488.0", - "@github/copilot-language-server-win32-x64": "1.488.0", - "@github/copilot-language-server-darwin-x64": "1.488.0", - "@github/copilot-language-server-darwin-arm64": "1.488.0", - "@github/copilot-language-server-linux-x64": "1.488.0", - "@github/copilot-language-server-linux-arm64": "1.488.0" + "@github/copilot-language-server": "1.502.5", + "@github/copilot-language-server-win32-x64": "1.502.5", + "@github/copilot-language-server-darwin-x64": "1.502.5", + "@github/copilot-language-server-darwin-arm64": "1.502.5", + "@github/copilot-language-server-linux-x64": "1.502.5", + "@github/copilot-language-server-linux-arm64": "1.502.5" } } From 1c740ce2c29d0d9e866be8f136bf96aa335e4dfd Mon Sep 17 00:00:00 2001 From: Sheng Chen Date: Mon, 22 Jun 2026 10:00:19 +0800 Subject: [PATCH 02/14] fix: keep Preferences dialog stable on Tool Auto Approve page (#298) The Tool Auto Approve page stacked four rule sections (~850px) in an uncapped container, so JFace's PreferenceDialog grew the shell toward screen height and never shrank it. Wrap the sections in a height-capped ScrolledComposite whose computeSize bounds the height reported to the dialog's PageLayout, so the shell stays a normal size and the content scrolls within a single page-level scrollbar. Unify the cap as PreferencePageUtils.STANDARD_CONTENT_HEIGHT (520), shared with McpPreferencePage's two stacked groups, so Copilot preference pages settle at a consistent size. Add a generic preference-pages test plan with a dialog-size regression case. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../preference-pages/preference-pages.md | 81 +++++++++++++++++++ .../AutoApprovePreferencePage.java | 34 +++++++- .../ui/preferences/McpPreferencePage.java | 2 +- .../ui/preferences/PreferencePageUtils.java | 9 +++ 4 files changed, 121 insertions(+), 5 deletions(-) create mode 100644 com.microsoft.copilot.eclipse.swtbot.test/test-plans/preference-pages/preference-pages.md diff --git a/com.microsoft.copilot.eclipse.swtbot.test/test-plans/preference-pages/preference-pages.md b/com.microsoft.copilot.eclipse.swtbot.test/test-plans/preference-pages/preference-pages.md new file mode 100644 index 00000000..08555695 --- /dev/null +++ b/com.microsoft.copilot.eclipse.swtbot.test/test-plans/preference-pages/preference-pages.md @@ -0,0 +1,81 @@ +# GitHub Copilot Preference Pages + +## Overview +Covers behavior shared by **all** GitHub Copilot preference pages +(**Preferences → GitHub Copilot → …**), independent of any single feature. +Every Copilot page is hosted in the same JFace `PreferenceDialog`, so concerns +like dialog sizing, layout stability, and navigation are cross-cutting. + +JFace grows the shared dialog to the **tallest** page visited and never shrinks +it again. To keep the dialog a stable size, each Copilot page keeps its content +within a common target height — `PreferencePageUtils.STANDARD_CONTENT_HEIGHT`. +Pages whose natural content is taller (e.g. **Tool Auto Approve**, which stacks +four rule sections) scroll within a height-capped `ScrolledComposite` instead of +ballooning the dialog. The same constant drives `McpPreferencePage`'s two groups, +so the pages settle at a consistent size. + +Entry points exercised: +- **Preferences → GitHub Copilot** — the root category page (only hyperlinks; no + sign-in, always valid). A stable baseline for size comparisons. +- **Preferences → GitHub Copilot → Tool Auto Approve** — the tallest page; the + regression magnet for dialog ballooning. +- Other child pages (MCP, Completions, Chat, …) — for navigation stability. + +--- + +## Prerequisites +- Eclipse IDE with the GitHub Copilot for Eclipse plugin installed and activated. +- No sign-in required: the pages below render independently of language-server + data. + +--- + +## 1. Dialog size stability + +### TC-001: Opening a tall page does not balloon the Preferences dialog + +**Type:** `Regression` +**Priority:** `P1` + +#### Steps +1. Open **Window → Preferences** and select **GitHub Copilot** (root page). Note + the dialog's size. +2. In the tree, select **GitHub Copilot → Tool Auto Approve**. +3. Observe the dialog does **not** grow toward screen height — it stays close to + the root page's size. +4. Confirm all four sections (Terminal, File Operation, MCP, Global) are + reachable by scrolling **within the page** using a single page-level vertical + scrollbar. +5. Navigate to a couple of sibling pages (e.g. **MCP**, **Completions**) and back + to Tool Auto Approve; the dialog size stays stable throughout. +6. Close Preferences. + +#### Expected Result +- The Preferences dialog stays a normal, stable size as the selection moves + between Copilot pages; Tool Auto Approve does not balloon toward screen height. +- Tool Auto Approve content is reachable via the page's own single vertical + scrollbar; no second (dialog-level) scrollbar appears. + +#### Reference measurements (Eclipse 2025-03, macOS) +- Root **GitHub Copilot** page dialog height: **~551px**. +- **Tool Auto Approve** dialog height (fixed by the height cap): **~656px** — + deterministic and screen-independent. +- Pre-fix, Tool Auto Approve ballooned to ~screen height (≈986px or + screen-clamped). + +#### 📸 Key Screenshots +- [ ] Dialog on the root **GitHub Copilot** page. +- [ ] Dialog on **Tool Auto Approve** — only slightly taller, content scrolling + within the page (single page scrollbar). + +--- + +## Notes +- Page height is **data-independent** (table/tree height hints are fixed), so MCP + servers loading asynchronously does not change the page height. +- The height cap is unified via `PreferencePageUtils.STANDARD_CONTENT_HEIGHT`, + shared by `AutoApprovePreferencePage` (scroller cap) and `McpPreferencePage` + (two stacked groups). Keep the pages within this height when adding content. +- Use the root **GitHub Copilot** page as the size baseline — it is always valid + and needs no sign-in, whereas the MCP page can enter an invalid state in an + unauthenticated sandbox and pop a JFace validation dialog. diff --git a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/preferences/AutoApprovePreferencePage.java b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/preferences/AutoApprovePreferencePage.java index b4b8b3f9..fb0be2c0 100644 --- a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/preferences/AutoApprovePreferencePage.java +++ b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/preferences/AutoApprovePreferencePage.java @@ -6,6 +6,8 @@ import org.eclipse.jface.preference.IPreferenceStore; import org.eclipse.jface.preference.PreferencePage; import org.eclipse.swt.SWT; +import org.eclipse.swt.custom.ScrolledComposite; +import org.eclipse.swt.graphics.Point; import org.eclipse.swt.layout.GridData; import org.eclipse.swt.layout.GridLayout; import org.eclipse.swt.widgets.Composite; @@ -51,12 +53,28 @@ protected Control createContents(Composite parent) { Messages.preferences_page_auto_approve_disabled_by_organization); } - Composite root = new Composite(parent, SWT.NONE); + // Wrap the sections in a height-capped ScrolledComposite. The override on + // computeSize bounds the height reported to the dialog's PageLayout so the + // Preferences shell stays stable; the real (taller) content scrolls here. + // This scroller is also the parent the section tables forward wheel events + // to (see forwardVerticalMouseWheelToParentScrollerAtBoundary). + ScrolledComposite scrolled = new ScrolledComposite(parent, SWT.V_SCROLL) { + @Override + public Point computeSize(int widthHint, int heightHint, boolean changed) { + Point size = super.computeSize(widthHint, heightHint, changed); + size.y = Math.min(size.y, PreferencePageUtils.STANDARD_CONTENT_HEIGHT); + return size; + } + }; + scrolled.setExpandHorizontal(true); + scrolled.setExpandVertical(true); + scrolled.setLayoutData(new GridData(SWT.FILL, SWT.FILL, true, true)); + + Composite root = new Composite(scrolled, SWT.NONE); GridLayout layout = new GridLayout(1, false); layout.marginWidth = 0; layout.marginHeight = 0; root.setLayout(layout); - root.setLayoutData(new GridData(SWT.FILL, SWT.TOP, true, false)); IPreferenceStore store = getPreferenceStore(); terminalSection = new TerminalAutoApproveSection(root, SWT.NONE); @@ -72,9 +90,17 @@ protected Control createContents(Composite parent) { globalSection = new GlobalAutoApproveSection(root, SWT.NONE); globalSection.loadFromPreferences(store); - root.addDisposeListener(e -> unbindMcpConfigService()); + scrolled.setContent(root); + scrolled.setMinSize(root.computeSize(SWT.DEFAULT, SWT.DEFAULT)); + // Track width so wrapping labels reflow and only vertical scrolling occurs. + scrolled.addListener(SWT.Resize, e -> { + int width = scrolled.getClientArea().width; + scrolled.setMinSize(root.computeSize(width, SWT.DEFAULT)); + }); + + scrolled.addDisposeListener(e -> unbindMcpConfigService()); - return root; + return scrolled; } @Override diff --git a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/preferences/McpPreferencePage.java b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/preferences/McpPreferencePage.java index c58c3947..c41975d0 100644 --- a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/preferences/McpPreferencePage.java +++ b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/preferences/McpPreferencePage.java @@ -83,7 +83,7 @@ public class McpPreferencePage extends FieldEditorPreferencePage implements IWorkbenchPreferencePage { public static final String ID = "com.microsoft.copilot.eclipse.ui.preferences.McpPreferencePage"; - private static final int GROUP_HEIGHT_HINT = 260; + private static final int GROUP_HEIGHT_HINT = PreferencePageUtils.STANDARD_CONTENT_HEIGHT / 2; private static final Gson GSON = new Gson(); private Group toolsGroup; diff --git a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/preferences/PreferencePageUtils.java b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/preferences/PreferencePageUtils.java index ab676ebf..13ed7ea9 100644 --- a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/preferences/PreferencePageUtils.java +++ b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/preferences/PreferencePageUtils.java @@ -25,6 +25,15 @@ */ public final class PreferencePageUtils { + /** + * Target content height, in pixels, for Copilot preference pages. JFace grows the shared Preferences dialog to + * the tallest page's preferred height and never shrinks it, so each page keeps its scrollable content within + * this height to hold the dialog at a stable size while the user navigates. Pages enforce it differently: + * {@code McpPreferencePage} divides it across two stacked groups; {@code AutoApprovePreferencePage} caps its + * {@code ScrolledComposite} at this height. + */ + public static final int STANDARD_CONTENT_HEIGHT = 520; + // Private constructor to prevent instantiation private PreferencePageUtils() { } From 799f24b67a1901f51f102f57cc03856f0c15cd4f Mon Sep 17 00:00:00 2001 From: Ethan Hou <149548697+ethanyhou@users.noreply.github.com> Date: Wed, 24 Jun 2026 11:17:25 +0800 Subject: [PATCH 03/14] fix: Updated token pricing model with context size and tier details (#316) --- .../lsp/CopilotLanguageServerConnection.java | 2 +- .../core/lsp/protocol/ContextSizeInfo.java | 1 + .../core/lsp/protocol/CopilotModel.java | 38 +++++++- .../eclipse/core/lsp/protocol/ModelInfo.java | 3 +- .../chat/contextwindow/ContextSizeDonut.java | 2 +- .../contextwindow/ContextWindowPopup.java | 12 +-- .../contextwindow/ContextWindowService.java | 71 ++++++++++++++- .../ui/chat/services/ChatServiceManager.java | 2 +- .../copilot/eclipse/ui/i18n/Messages.java | 2 +- .../eclipse/ui/i18n/messages.properties | 2 +- .../ui/swt/ModelHoverContentProvider.java | 88 ++----------------- .../copilot/eclipse/ui/utils/ModelUtils.java | 56 +++++++++++- 12 files changed, 176 insertions(+), 103 deletions(-) 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 85e26f98..de116abb 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 @@ -721,6 +721,6 @@ private static ModelInfo buildModelInfo(CopilotModel activeModel, String reasoni } String id = activeModel != null ? activeModel.getId() : null; String providerName = activeModel != null ? activeModel.getProviderName() : null; - return new ModelInfo(id, providerName, reasoningEffort); + return new ModelInfo(id, providerName, reasoningEffort, null); } } diff --git a/com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/lsp/protocol/ContextSizeInfo.java b/com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/lsp/protocol/ContextSizeInfo.java index 0edd83a8..2331bab7 100644 --- a/com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/lsp/protocol/ContextSizeInfo.java +++ b/com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/lsp/protocol/ContextSizeInfo.java @@ -9,6 +9,7 @@ */ public record ContextSizeInfo( int totalTokenLimit, + int reservedOutputTokens, int systemPromptTokens, int toolDefinitionTokens, int userMessagesTokens, diff --git a/com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/lsp/protocol/CopilotModel.java b/com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/lsp/protocol/CopilotModel.java index fe54ea25..dfc8bd3c 100644 --- a/com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/lsp/protocol/CopilotModel.java +++ b/com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/lsp/protocol/CopilotModel.java @@ -6,6 +6,7 @@ import java.util.List; import java.util.Objects; +import com.google.gson.annotations.SerializedName; import org.apache.commons.lang3.builder.ToStringBuilder; /** @@ -97,17 +98,46 @@ public String toString() { } /** - * Per-token prices for the model, returned in USD. + * Per-tier token prices, quoted in USD per {@link CopilotModelBillingTokenPrices#batchSize} tokens and applying up + * to {@code maxContext} context tokens. Requests larger than the {@code default} tier's {@code maxContext} are + * billed at the {@code longContext} tier. All components are optional ({@code null} when the server does not provide + * a value). + * + * @param cachePrice the price for cached input tokens + * @param inputPrice the price for input tokens + * @param outputPrice the price for output tokens + * @param maxContext the maximum number of context (input) tokens this tier applies to */ - public record CopilotModelBillingTokenPrices(Double cachePrice, Double inputPrice, Double outputPrice, - Double tokenUnit) { + public record CopilotModelTokenPriceTier(Double cachePrice, Double inputPrice, Double outputPrice, + Integer maxContext) { @Override public String toString() { ToStringBuilder builder = new ToStringBuilder(this); builder.append("cachePrice", cachePrice); builder.append("inputPrice", inputPrice); builder.append("outputPrice", outputPrice); - builder.append("tokenUnit", tokenUnit); + builder.append("maxContext", maxContext); + return builder.toString(); + } + } + + /** + * Per-tier token prices for the model. When token-based billing is enabled the server returns a {@code default} + * tier and, for models that support long context, a {@code longContext} tier. + * + * @param batchSize the number of tokens each tier price is quoted per + * @param defaultTier the {@code default} price tier (deserialized from the {@code default} JSON field) + * @param longContext the {@code longContext} price tier, or {@code null} when the model does not support long + * context + */ + public record CopilotModelBillingTokenPrices(Double batchSize, + @SerializedName("default") CopilotModelTokenPriceTier defaultTier, CopilotModelTokenPriceTier longContext) { + @Override + public String toString() { + ToStringBuilder builder = new ToStringBuilder(this); + builder.append("batchSize", batchSize); + builder.append("defaultTier", defaultTier); + builder.append("longContext", longContext); return builder.toString(); } } diff --git a/com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/lsp/protocol/ModelInfo.java b/com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/lsp/protocol/ModelInfo.java index 729e9768..de61df10 100644 --- a/com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/lsp/protocol/ModelInfo.java +++ b/com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/lsp/protocol/ModelInfo.java @@ -15,6 +15,7 @@ * @param id model identifier (optional) * @param providerName provider name (optional) * @param reasoningEffort user-selected reasoning effort (optional) + * @param contextSize context size (optional) */ -public record ModelInfo(String id, String providerName, String reasoningEffort) { +public record ModelInfo(String id, String providerName, String reasoningEffort, String contextSize) { } diff --git a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/contextwindow/ContextSizeDonut.java b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/contextwindow/ContextSizeDonut.java index c6cc8630..6c226499 100644 --- a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/contextwindow/ContextSizeDonut.java +++ b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/contextwindow/ContextSizeDonut.java @@ -63,7 +63,7 @@ public ContextSizeDonut(Composite parent, ContextWindowService contextWindowServ e.gc.drawArc(arcOffset, arcOffset, arcSize, arcSize, 0, 360); // Used portion starting from 12 o'clock (90°) going clockwise (negative angle) - double pct = Math.min(info.utilizationPercentage(), 100.0); + double pct = Math.min(contextWindowService.getDisplayUtilizationPercentage(info), 100.0); int filledAngle = (int) Math.round(pct / 100.0 * 360); if (filledAngle > 0) { Color filledColor = pct >= 90 ? CssConstants.getDonutWarningColor(e.display) diff --git a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/contextwindow/ContextWindowPopup.java b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/contextwindow/ContextWindowPopup.java index 7f67021a..15f38afd 100644 --- a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/contextwindow/ContextWindowPopup.java +++ b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/contextwindow/ContextWindowPopup.java @@ -67,12 +67,12 @@ protected void populateContent(Composite parent) { tokenUsageLabel = createSecondaryTextLabel(tokenRow, formatTokenRow(latestInfo)); tokenUsageLabel.setLayoutData(new GridData(SWT.FILL, SWT.NONE, true, false)); percentageLabel = createSecondaryTextLabel(tokenRow, - formatPercentage(latestInfo.utilizationPercentage())); + formatPercentage(contextWindowService.getDisplayUtilizationPercentage(latestInfo))); percentageLabel.setLayoutData(new GridData(SWT.RIGHT, SWT.NONE, false, false)); progressBar = new ContextWindowBar(parent, SWT.NONE); progressBar.setLayoutData(new GridData(SWT.FILL, SWT.CENTER, true, false)); - progressBar.setPercentage((int) Math.round(latestInfo.utilizationPercentage())); + progressBar.setPercentage((int) Math.round(contextWindowService.getDisplayUtilizationPercentage(latestInfo))); addSeparator(parent, SECTION_SPACING); @@ -107,7 +107,7 @@ private void updateLabels(ContextSizeInfo info) { return; } setLabelText(tokenUsageLabel, formatTokenRow(info)); - setLabelText(percentageLabel, formatPercentage(info.utilizationPercentage())); + setLabelText(percentageLabel, formatPercentage(contextWindowService.getDisplayUtilizationPercentage(info))); setLabelText(systemInstructionsValue, percentageOf(info.systemPromptTokens(), info.totalTokenLimit())); setLabelText(toolDefinitionsValue, @@ -120,7 +120,7 @@ private void updateLabels(ContextSizeInfo info) { setLabelText(toolResultsValue, percentageOf(info.toolResultsTokens(), info.totalTokenLimit())); if (progressBar != null && !progressBar.isDisposed()) { - progressBar.setPercentage((int) Math.round(info.utilizationPercentage())); + progressBar.setPercentage((int) Math.round(contextWindowService.getDisplayUtilizationPercentage(info))); } shell.requestLayout(); } @@ -146,9 +146,9 @@ private static String formatPercentage(double pct) { return String.format("%.1f%%", pct); } - private static String formatTokenRow(ContextSizeInfo info) { + private String formatTokenRow(ContextSizeInfo info) { return MessageFormat.format(Messages.context_window_tokens, - formatTokens(info.totalUsedTokens()), formatTokens(info.totalTokenLimit())); + formatTokens(info.totalUsedTokens()), formatTokens(contextWindowService.getDisplayTokenLimit(info))); } private static String percentageOf(int tokens, int totalLimit) { diff --git a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/contextwindow/ContextWindowService.java b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/contextwindow/ContextWindowService.java index 412c41ea..d1b7b781 100644 --- a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/contextwindow/ContextWindowService.java +++ b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/contextwindow/ContextWindowService.java @@ -17,6 +17,9 @@ import org.eclipse.swt.widgets.Display; import com.microsoft.copilot.eclipse.core.lsp.protocol.ContextSizeInfo; +import com.microsoft.copilot.eclipse.core.lsp.protocol.CopilotModel; +import com.microsoft.copilot.eclipse.ui.chat.services.ModelService; +import com.microsoft.copilot.eclipse.ui.utils.ModelUtils; import com.microsoft.copilot.eclipse.ui.utils.SwtUtils; /** @@ -27,11 +30,15 @@ public class ContextWindowService { private IObservableValue contextSizeObservable; private final Map popupSideEffects = new HashMap<>(); + private final ModelService modelService; /** * Creates the service and initializes the observable state on the UI realm. + * + * @param modelService the model service used to resolve the active model's context window */ - public ContextWindowService() { + public ContextWindowService(ModelService modelService) { + this.modelService = modelService; AtomicReference> observableRef = new AtomicReference<>(); SwtUtils.invokeOnDisplayThread(() -> { Realm realm = Realm.getDefault(); @@ -58,6 +65,68 @@ public ContextSizeInfo getState() { return result.get(); } + /** + * Returns the context-window limit shown in the donut popup. Prefer the active model's resolved full context window, + * falling back to the language-server usage snapshot when the model metadata is unavailable. + * + * @param info the context usage snapshot + * @return the display context-window limit + */ + public int getDisplayTokenLimit(ContextSizeInfo info) { + if (info == null) { + return 0; + } + + Integer outputLimit = getActiveModelOutputLimit(); + if (info.reservedOutputTokens() > 0) { + outputLimit = info.reservedOutputTokens(); + } + if (outputLimit != null && outputLimit > 0) { + return info.totalTokenLimit() + outputLimit; + } + + Integer modelContextWindow = getActiveModelContextWindow(); + return modelContextWindow != null && modelContextWindow > 0 ? modelContextWindow : info.totalTokenLimit(); + } + + /** + * Returns the utilization percentage against the displayed context window. + * + * @param info the context usage snapshot + * @return the utilization percentage + */ + public double getDisplayUtilizationPercentage(ContextSizeInfo info) { + if (info == null) { + return 0; + } + int displayLimit = getDisplayTokenLimit(info); + if (displayLimit <= 0) { + return info.utilizationPercentage(); + } + return (double) info.totalUsedTokens() / displayLimit * 100; + } + + private Integer getActiveModelContextWindow() { + CopilotModel activeModel = getActiveModel(); + if (activeModel == null) { + return null; + } + return ModelUtils.resolveContextWindowSize(activeModel); + } + + private Integer getActiveModelOutputLimit() { + CopilotModel activeModel = getActiveModel(); + if (activeModel == null || activeModel.getCapabilities() == null + || activeModel.getCapabilities().limits() == null) { + return null; + } + return activeModel.getCapabilities().limits().maxOutputTokens(); + } + + private CopilotModel getActiveModel() { + return modelService == null ? null : modelService.getActiveModel(); + } + /** * Updates the current context size state and notifies bound UI. * diff --git a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/services/ChatServiceManager.java b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/services/ChatServiceManager.java index ac3d955a..0883074f 100644 --- a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/services/ChatServiceManager.java +++ b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/services/ChatServiceManager.java @@ -54,7 +54,7 @@ public ChatServiceManager() { mcpRuntimeLogger = new McpRuntimeLogger(); persistenceManager = new ConversationPersistenceManager(this.authStatusManager); chatFontService = new ChatFontService(); - contextWindowService = new ContextWindowService(); + contextWindowService = new ContextWindowService(modelService); } /** diff --git a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/i18n/Messages.java b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/i18n/Messages.java index f8ecd839..cb3fc2dd 100644 --- a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/i18n/Messages.java +++ b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/i18n/Messages.java @@ -199,7 +199,7 @@ public final class Messages extends NLS { public static String model_billing_multiplier_suffix; public static String model_billing_multiplier_variable; public static String model_preview_suffix; - public static String model_hover_contextSize; + public static String model_hover_contextWindow; public static String model_hover_cost; public static String model_hover_thinkingEffort; public static String model_hover_thinkingEffort_default_suffix; diff --git a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/i18n/messages.properties b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/i18n/messages.properties index 16e4f4f1..3974ed60 100644 --- a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/i18n/messages.properties +++ b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/i18n/messages.properties @@ -193,7 +193,7 @@ generateCommitMessage_noStagedFiles_message=There are no staged files to generat addToReference_addFile_title=Add File to Chat addToReference_addFolder_title=Add Folder to Chat -model_hover_contextSize=Context Size: +model_hover_contextWindow=Context Window: model_hover_cost=Cost: model_hover_thinkingEffort=Thinking Effort model_hover_thinkingEffort_default_suffix={0} (default) diff --git a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/swt/ModelHoverContentProvider.java b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/swt/ModelHoverContentProvider.java index a1698ffe..1c963f89 100644 --- a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/swt/ModelHoverContentProvider.java +++ b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/swt/ModelHoverContentProvider.java @@ -19,7 +19,6 @@ import org.eclipse.swt.graphics.Rectangle; import org.eclipse.swt.layout.GridData; import org.eclipse.swt.layout.GridLayout; -import org.eclipse.swt.layout.RowLayout; import org.eclipse.swt.widgets.Composite; import org.eclipse.swt.widgets.Control; import org.eclipse.swt.widgets.Display; @@ -27,7 +26,6 @@ import org.eclipse.ui.PlatformUI; import com.microsoft.copilot.eclipse.core.lsp.protocol.CopilotModel; -import com.microsoft.copilot.eclipse.core.lsp.protocol.CopilotModel.CopilotModelCapabilitiesLimits; import com.microsoft.copilot.eclipse.core.lsp.protocol.CopilotModel.CopilotModelCapabilitiesSupports; import com.microsoft.copilot.eclipse.ui.CopilotUi; import com.microsoft.copilot.eclipse.ui.chat.services.ModelService; @@ -37,7 +35,7 @@ /** * Renders the full hover UI for model items in the model picker dropdown. The layout consists of the bold title header, - * an optional category badge, an optional degradation warning, and model-specific details such as context size and + * an optional category badge, an optional degradation warning, and model-specific details such as context window and * token pricing. */ public class ModelHoverContentProvider implements IDropdownItemHoverProvider { @@ -50,8 +48,6 @@ public class ModelHoverContentProvider implements IDropdownItemHoverProvider { /** Vertical padding inside a thinking effort row, so the hover background has breathing room. */ private static final int THINKING_EFFORT_ROW_V_PADDING = 2; - private static Image arrowUpIcon; - private static Image arrowDownIcon; private static Image effortCheckIcon; private final CopilotModel model; @@ -80,9 +76,7 @@ public void configureHover(Composite parent, DropdownItem item, Runnable closeRe addWarningRow(parent, model.getDegradationReason()); } - CopilotModelCapabilitiesLimits limits = model.getCapabilities() != null ? model.getCapabilities().limits() : null; - - addContextSizeSection(parent, limits); + addContextWindowSection(parent); addPricingSection(parent, model.getModelPickerPriceCategory()); addThinkingEffortSection(parent, closeRequest); } @@ -95,49 +89,14 @@ private void renderHeader(Composite parent, DropdownItem item) { titleLabel.setLayoutData(headerGd); } - private void addContextSizeSection(Composite parent, CopilotModelCapabilitiesLimits limits) { - if (limits == null) { - return; - } - boolean hasInput = isPositive(limits.maxInputTokens()); - boolean hasOutput = isPositive(limits.maxOutputTokens()); - if (!hasInput && !hasOutput) { + private void addContextWindowSection(Composite parent) { + String contextWindowText = ModelUtils.getContextWindowText(model); + if (StringUtils.isBlank(contextWindowText)) { return; } addSeparator(parent); - - Composite row = createKeyValueRow(parent); - ((GridData) row.getLayoutData()).verticalIndent = SECTION_SPACING; - - // Context Size: - Label keyLabel = createSecondaryTextLabel(row, Messages.model_hover_contextSize); - keyLabel.setLayoutData(new GridData(SWT.LEFT, SWT.NONE, false, false)); - - Composite valueComp = new Composite(row, SWT.NONE); - valueComp.setLayoutData(new GridData(SWT.RIGHT, SWT.NONE, true, false)); - RowLayout valueLayout = new RowLayout(SWT.HORIZONTAL); - valueLayout.marginTop = 0; - valueLayout.marginBottom = 0; - valueLayout.marginLeft = 0; - valueLayout.marginRight = 0; - - // Add spacing between input and output token labels if both are present - if (hasInput && hasOutput) { - valueLayout.spacing = 4; - } else { - valueLayout.spacing = 0; - } - valueComp.setLayout(valueLayout); - - // ex. ↑128K - if (hasInput) { - addArrowTokenLabel(valueComp, true, ModelUtils.formatTokenCount(limits.maxInputTokens())); - } - // ex. ↓16K - if (hasOutput) { - addArrowTokenLabel(valueComp, false, ModelUtils.formatTokenCount(limits.maxOutputTokens())); - } + addKeyValueRow(parent, Messages.model_hover_contextWindow, contextWindowText); } private void addPricingSection(Composite parent, String priceCategory) { @@ -325,42 +284,7 @@ private Composite createKeyValueRow(Composite parent) { return row; } - private void addArrowTokenLabel(Composite parent, boolean isInput, String tokenText) { - GridLayout pairLayout = new GridLayout(2, false); - pairLayout.marginWidth = 0; - pairLayout.marginHeight = 0; - pairLayout.horizontalSpacing = 0; - Composite pairComp = new Composite(parent, SWT.NONE); - pairComp.setLayout(pairLayout); - - initArrowIcons(pairComp); - Label arrowLabel = new Label(pairComp, SWT.NONE); - Image arrowImage = isInput ? arrowUpIcon : arrowDownIcon; - arrowLabel.setImage(arrowImage); - - createSecondaryTextLabel(pairComp, tokenText); - } - - private static void initArrowIcons(Composite parent) { - if (arrowUpIcon == null || arrowUpIcon.isDisposed()) { - boolean isDark = UiUtils.isDarkTheme(); - arrowUpIcon = UiUtils.buildImageFromPngPath(isDark ? "/icons/dropdown/context_size_arrow_up_dark.png" - : "/icons/dropdown/context_size_arrow_up_light.png"); - arrowDownIcon = UiUtils.buildImageFromPngPath(isDark ? "/icons/dropdown/context_size_arrow_down_dark.png" - : "/icons/dropdown/context_size_arrow_down_light.png"); - parent.getDisplay().addListener(SWT.Dispose, e -> disposeStaticIcons()); - } - } - private static void disposeStaticIcons() { - if (arrowUpIcon != null && !arrowUpIcon.isDisposed()) { - arrowUpIcon.dispose(); - arrowUpIcon = null; - } - if (arrowDownIcon != null && !arrowDownIcon.isDisposed()) { - arrowDownIcon.dispose(); - arrowDownIcon = null; - } if (effortCheckIcon != null && !effortCheckIcon.isDisposed()) { effortCheckIcon.dispose(); effortCheckIcon = null; diff --git a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/utils/ModelUtils.java b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/utils/ModelUtils.java index e7d18484..f806727f 100644 --- a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/utils/ModelUtils.java +++ b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/utils/ModelUtils.java @@ -14,6 +14,7 @@ import com.microsoft.copilot.eclipse.core.lsp.protocol.CopilotModel.CopilotModelCapabilities; import com.microsoft.copilot.eclipse.core.lsp.protocol.CopilotModel.CopilotModelCapabilitiesLimits; import com.microsoft.copilot.eclipse.core.lsp.protocol.CopilotModel.CopilotModelCapabilitiesSupports; +import com.microsoft.copilot.eclipse.core.lsp.protocol.CopilotModel.CopilotModelTokenPriceTier; import com.microsoft.copilot.eclipse.core.lsp.protocol.CopilotScope; import com.microsoft.copilot.eclipse.core.lsp.protocol.byok.ByokModel; import com.microsoft.copilot.eclipse.core.lsp.protocol.byok.ByokModelCapabilities; @@ -162,15 +163,62 @@ public static String formatPriceCategory(String priceCategory) { /** * Returns the formatted context window size for the model, or {@code null} if unavailable. */ - private static String getContextWindowText(CopilotModel model) { + public static String getContextWindowText(CopilotModel model) { + Integer contextWindow = resolveContextWindowSize(model); + if (contextWindow == null || contextWindow <= 0) { + return null; + } + return formatTokenCount(contextWindow); + } + + /** + * Resolves the user-facing context window size for the model, mirroring the language-server / IntelliJ behavior. + * + *

When the model advertises a {@code default} price tier with its own {@code maxContext} (the input budget), the + * full window is {@code maxContext + maxOutputTokens}. Otherwise, token-based billing models fall back to + * {@code maxInputTokens + maxOutputTokens}, and finally to the advertised {@code maxContextWindowTokens}. + * + * @param model the model + * @return the context window size in tokens, or {@code null} when it cannot be determined + */ + public static Integer resolveContextWindowSize(CopilotModel model) { if (model.getCapabilities() == null || model.getCapabilities().limits() == null) { return null; } - Integer maxContextWindowTokens = model.getCapabilities().limits().maxContextWindowTokens(); - if (maxContextWindowTokens == null || maxContextWindowTokens <= 0) { + CopilotModelCapabilitiesLimits limits = model.getCapabilities().limits(); + Integer maxOutputTokens = limits.maxOutputTokens(); + int output = maxOutputTokens == null ? 0 : maxOutputTokens; + + CopilotModelTokenPriceTier defaultTier = getDefaultTokenPriceTier(model); + if (defaultTier != null && defaultTier.maxContext() != null) { + return defaultTier.maxContext() + output; + } + + // TODO: Remove this legacy fallback after TBB is officially released. + if (isTokenBasedBillingEnabled(model)) { + Integer maxInputTokens = limits.maxInputTokens(); + if (maxInputTokens != null && maxOutputTokens != null) { + return maxInputTokens + maxOutputTokens; + } + } + + return limits.maxContextWindowTokens(); + } + + // TODO: Remove this legacy fallback after TBB is officially released. + private static boolean isTokenBasedBillingEnabled(CopilotModel model) { + return model.getBilling() != null && model.getBilling().tokenBasedBillingEnabled(); + } + + /** + * Returns the model's {@code default} token price tier, or {@code null} when the model carries no token-based + * pricing. + */ + private static CopilotModelTokenPriceTier getDefaultTokenPriceTier(CopilotModel model) { + if (model.getBilling() == null || model.getBilling().tokenPrices() == null) { return null; } - return formatTokenCount(maxContextWindowTokens); + return model.getBilling().tokenPrices().defaultTier(); } /** From 1c72c04b9fea8ba30bb64be1095f11cbf37c5013 Mon Sep 17 00:00:00 2001 From: Sheng Chen Date: Wed, 24 Jun 2026 17:18:21 +0800 Subject: [PATCH 04/14] fix: Adjust menu contribution location for Copilot menu (#292) --- com.microsoft.copilot.eclipse.ui/plugin.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/com.microsoft.copilot.eclipse.ui/plugin.xml b/com.microsoft.copilot.eclipse.ui/plugin.xml index a3d4a20a..8771f7ac 100644 --- a/com.microsoft.copilot.eclipse.ui/plugin.xml +++ b/com.microsoft.copilot.eclipse.ui/plugin.xml @@ -29,7 +29,7 @@ + locationURI="menu:org.eclipse.ui.main.menu?before=help">

Date: Wed, 24 Jun 2026 17:19:34 +0800 Subject: [PATCH 05/14] docs: Add endgame test cases (#310) --- .../cls-session-persistence.md | 30 ++++ .../editor-indentation-preferences.md | 168 ++++++++++++++++++ .../local-file-edit-and-create-tools.md | 65 +++++++ .../test-plans/subagent/subagent.md | 45 +++++ .../thinking-effort/thinking-effort.md | 45 +++++ 5 files changed, 353 insertions(+) create mode 100644 com.microsoft.copilot.eclipse.swtbot.test/test-plans/editor-indentation-preferences/editor-indentation-preferences.md diff --git a/com.microsoft.copilot.eclipse.swtbot.test/test-plans/cls-session-persistence/cls-session-persistence.md b/com.microsoft.copilot.eclipse.swtbot.test/test-plans/cls-session-persistence/cls-session-persistence.md index 454c5b8b..813bffab 100644 --- a/com.microsoft.copilot.eclipse.swtbot.test/test-plans/cls-session-persistence/cls-session-persistence.md +++ b/com.microsoft.copilot.eclipse.swtbot.test/test-plans/cls-session-persistence/cls-session-persistence.md @@ -181,6 +181,34 @@ This feature integrates CLS (Copilot Language Server) server-side session persis --- +### TC-008: Cancelled terminal tool call stays cancelled after restoration + +**Type:** `Regression` +**Priority:** `P0` + +#### Preconditions +- Copilot Chat is open in Agent mode +- A workspace is open so the `run_in_terminal` tool can execute +- A model that supports tool calls is selected + +#### Steps +1. Send a message that makes Copilot use the `run_in_terminal` tool to perform a longer-running task (e.g. "run a command that takes a while, such as `ping -n 30 127.0.0.1`") +2. While the terminal tool call is still running (spinner active on the tool status), click Cancel on the chat response +3. Verify the terminal tool call status updates to the **cancelled** icon (no longer spinning) +4. Click "New Chat" to start a fresh conversation +5. Open chat history and select the original conversation to restore it + +#### Expected Result +- After restoration, the terminal tool call still shows the **cancelled** icon +- The tool call status does **not** revert to running/spinning (no rotating/ongoing indicator reappears) +- The cancelled status is consistent between the live cancel and the restored view + +#### 📸 Key Screenshots +- [ ] **After cancel** — Terminal tool call showing the cancelled icon (not spinning) +- [ ] **After restore** — Same terminal tool call restored with the cancelled icon, not reverted to running + +--- + ## Screenshots Checklist > Consolidated list of all key screenshot moments. @@ -195,3 +223,5 @@ This feature integrates CLS (Copilot Language Server) server-side session persis - [ ] `TC-006` Restored history with successful follow-up and no 400 error - [ ] `TC-007` Original conversation showing assistant reply - [ ] `TC-007` Restored conversation with correctly attributed messages and no duplicates +- [ ] `TC-008` Cancelled terminal tool call showing cancelled icon after cancel +- [ ] `TC-008` Restored terminal tool call still cancelled, not reverted to running diff --git a/com.microsoft.copilot.eclipse.swtbot.test/test-plans/editor-indentation-preferences/editor-indentation-preferences.md b/com.microsoft.copilot.eclipse.swtbot.test/test-plans/editor-indentation-preferences/editor-indentation-preferences.md new file mode 100644 index 00000000..eab55438 --- /dev/null +++ b/com.microsoft.copilot.eclipse.swtbot.test/test-plans/editor-indentation-preferences/editor-indentation-preferences.md @@ -0,0 +1,168 @@ +# Editor Indentation Preferences for Inline Completions + +## Overview +When Copilot produces an **inline completion (ghost text)**, the indentation it +suggests (tab character vs. spaces and the tab width) should follow the +platform's **default text editor preferences** instead of always defaulting to +spaces with a tab size of 4. + +The formatting options are resolved by `FormatOptionProvider` and sent on each +completion request as `CompletionDocument.insertSpaces` / `tabSize` by +`CompletionProvider`. For Java and C/C++ projects the language-specific +formatter settings continue to apply. For every other case — an unknown +language, a file with no extension, or a file with no project — the provider +now reads the Eclipse text editor preferences +`org.eclipse.ui.editors/spacesForTabs` and `org.eclipse.ui.editors/tabWidth`. +If those preferences cannot be read it falls back to the previous hardcoded +defaults (spaces, tab width 4). + +> **Important:** these options affect **inline completions only**, not the +> Agent-mode file create/edit tools. The verification must therefore be done +> by triggering ghost-text completions in an editor, not by asking Agent mode +> to write a file. + +These preferences are configured under +**Preferences → General → Editors → Text Editors**: +- **"Insert spaces for tabs"** maps to `spacesForTabs`. +- **"Displayed tab width"** maps to `tabWidth`. + +Entry points: +- Window → Preferences → General → Editors → Text Editors +- Typing in a text editor to trigger an inline completion (ghost text) + +Not exercised: +- Direct unit-level invocation of `FormatOptionProvider` (covered by + `FormatOptionProviderTests`). +- Agent-mode file create/edit tools (they do not consult these options). +- Java/C-specific formatter configuration screens. + +--- + +## Prerequisites + +- Eclipse IDE with the GitHub Copilot for Eclipse plugin installed and + activated. +- The user is signed in to GitHub Copilot and inline completions are enabled. +- A workspace with at least one open project that is **neither a Java nor a + C/C++ project** (a General/empty project), so the editor text preferences + are the deciding factor. +- Knowledge of the current values of **Preferences → General → Editors → + Text Editors → "Insert spaces for tabs"** and **"Displayed tab width"** so + they can be restored after testing. +- Because completion content is model-influenced, prefer a prompt/context that + reliably forces an **indented multi-line suggestion** (e.g. an opening + brace/block), and inspect the **raw whitespace** of the accepted text rather + than relying on the displayed tab width. + +--- + +## 1. Spaces configuration + +### TC-001: Inline completion uses spaces when "insert spaces for tabs" is on + +**Type:** `Happy Path` +**Priority:** `P0` + +#### Preconditions +- A non-Java/non-C project is open in the workspace. +- A file whose language is unknown / has no extension (e.g. a file named + `sample` with code-like content) is open in the editor. + +#### Steps +1. Open **Preferences → General → Editors → Text Editors**, check + **"Insert spaces for tabs"**, set **"Displayed tab width"** to `2`, then + click **Apply and Close**. +2. In the editor, type content that forces an indented multi-line + continuation, for example an opening block and a newline: + `function greet() {` then press Enter so Copilot suggests the indented + body as ghost text. +3. Accept the inline completion (Tab). +4. Reveal whitespace ("Show whitespace characters") or inspect the raw bytes + of the inserted lines. + +#### Expected Result +- The accepted completion indents the body with **spaces** (not tabs), at + **2 spaces** per level, matching the configured tab width. +- No error dialog appears and the Eclipse error log has no formatting + exception. + +#### 📸 Key Screenshots +- [ ] **Editor preferences** — "Insert spaces for tabs" checked, tab width 2. +- [ ] **Accepted completion whitespace** — Indentation shown as 2-space groups. + +--- + +## 2. Tabs configuration + +### TC-002: Inline completion uses tabs, and switching the preference updates the next request + +**Type:** `Happy Path` +**Priority:** `P0` + +#### Preconditions +- A non-Java/non-C project is open in the workspace. +- An unknown-language / extensionless file is open in the editor. + +#### Steps +1. Open **Preferences → General → Editors → Text Editors**, **uncheck** + "Insert spaces for tabs", set **"Displayed tab width"** to `4`, then click + **Apply and Close**. +2. In the editor, trigger an indented multi-line completion (as in TC-001) and + accept it. Reveal whitespace / inspect raw bytes. +3. Change editor preferences to **spaces**, tab width `2`. Apply and close. +4. Trigger another indented completion in the same or a new unknown-language + file and accept it. Reveal whitespace. + +#### Expected Result +- The first accepted completion indents with **tab characters** (`\t`). +- After switching the preference, the next completion indents with **2 + spaces**. +- The provider reads the current editor preference value on **each completion + request** rather than caching the first observed value. +- No error dialog appears and the Eclipse error log has no formatting + exception. + +#### 📸 Key Screenshots +- [ ] **Tabs completion** — Tab indentation under the tabs preference. +- [ ] **Spaces completion** — 2-space indentation after switching the preference. + +--- + +## 3. Regression — language-specific projects are unaffected + +### TC-003: Java completion still uses the Java formatter settings, not editor preferences + +**Type:** `Regression` +**Priority:** `P0` + +#### Preconditions +- A **Java** project is open in the workspace with its own + formatter/indentation settings (e.g., tabs for Java). +- Editor preferences (General → Text Editors) are set to a **different** + value than the Java formatter (e.g., spaces, tab width 2). +- A `.java` source file is open in the editor. + +#### Steps +1. In the Java file, trigger an indented multi-line inline completion (e.g. + inside a method body) and accept it. +2. Reveal whitespace / inspect raw bytes of the inserted lines. + +#### Expected Result +- The Java completion follows the **Java formatter** indentation settings, not + the generic text editor preferences. +- Only unknown/extensionless/projectless cases consult the editor + preferences; Java (and C/C++) behavior is unchanged. + +#### 📸 Key Screenshots +- [ ] **Java completion whitespace** — Indentation matches the Java formatter, + independent of the General text-editor preference. + +--- + +## Screenshots Checklist + +- [ ] TC-001 — Editor preferences (spaces, width 2) + accepted completion with + 2-space indentation. +- [ ] TC-002 — Tabs completion vs. spaces completion after a preference switch. +- [ ] TC-003 — Java completion indentation unchanged by the General + text-editor preference. diff --git a/com.microsoft.copilot.eclipse.swtbot.test/test-plans/file-system/local-file-edit-and-create-tools.md b/com.microsoft.copilot.eclipse.swtbot.test/test-plans/file-system/local-file-edit-and-create-tools.md index 7f9a18cf..0e55246e 100644 --- a/com.microsoft.copilot.eclipse.swtbot.test/test-plans/file-system/local-file-edit-and-create-tools.md +++ b/com.microsoft.copilot.eclipse.swtbot.test/test-plans/file-system/local-file-edit-and-create-tools.md @@ -219,3 +219,68 @@ Not exercised: #### Key Screenshots - [ ] **Local file tool link** -- The tool result shows a clickable absolute path outside the workspace. - [ ] **External local file editor** -- The external local file opens in an Eclipse editor. + +### TC-007: Workspace directory and project links reveal in the Project Explorer + +**Type:** `Happy Path` +**Priority:** `P1` + +#### Preconditions +- The Eclipse workbench is open with at least one project (e.g., `demo`) that contains a sub-folder (e.g., `src`). +- The **Project Explorer** view is open. +- Copilot Chat is open in Agent mode. + +#### Steps +1. Send a prompt that causes Agent mode to reference the workspace sub-folder by path (for example, ask it to list the + contents of the `src` folder so the tool result renders a directory link). +2. When the tool call appears in the Chat view, click the directory link for the `src` folder. +3. Verify the `src` folder is selected and revealed in the **Project Explorer** (no external browser opens). +4. Send a prompt that causes Agent mode to reference the project root, then click the project link in the tool result. +5. Verify the project is selected and revealed in the **Project Explorer**. + +#### Expected Result +- Clicking a workspace folder or project link reveals and selects that resource in the Project Explorer. +- No external browser or web page is opened for directory/project links. +- No error dialog is shown and the Eclipse error log has no navigation exception. + +#### Key Screenshots +- [ ] **Directory tool link** -- The tool result shows a clickable workspace folder link. +- [ ] **Folder revealed** -- The `src` folder is selected and revealed in the Project Explorer. +- [ ] **Project revealed** -- The project root is selected and revealed in the Project Explorer. + +#### Notes on failure modes +- Clicking the directory link opens a browser or does nothing -- the folder/project branch may not route through + `UiUtils.revealInExplorer`. +- The resource is opened in an editor instead of being revealed -- folder/project types may be incorrectly treated as + files. + +### TC-008: File URI link with a line-number fragment opens the external local file + +**Type:** `Edge Case` +**Priority:** `P2` + +#### Preconditions +- The Eclipse workbench is open. +- Copilot Chat is open in Agent mode. +- The local test directory outside the workspace exists and contains `existing-local-file.txt` with multiple lines. + +#### Steps +1. Send a prompt that causes Agent mode to reference `existing-local-file.txt` by absolute path with a line-number + fragment (for example, a link ending in `#L10` or a `file:` URI with a `#L10` fragment). +2. When the tool call appears in the Chat view, click the file path link. +3. Verify Eclipse opens `existing-local-file.txt` in an editor (the trailing line-number fragment is treated as a + fragment, not part of the file name). + +#### Expected Result +- The line-number fragment is stripped when resolving the local path, so the correct external file opens. +- Eclipse does not report a missing file named `existing-local-file.txt#L10` or a path-resolution error. +- No error dialog is shown and the Eclipse error log has no local file navigation exception. + +#### Key Screenshots +- [ ] **Fragment file link** -- The tool result shows a clickable local path with a `#L10` fragment. +- [ ] **External file opened** -- The external local file opens in an Eclipse editor despite the fragment. + +#### Notes on failure modes +- Eclipse reports the file cannot be found -- the `#` fragment may not be stripped before resolving the local path. +- A relative-path or `https:` link is unexpectedly opened as a local file -- the URI-scheme guard in + `FileUtils.getLocalFilePath` may be bypassed. diff --git a/com.microsoft.copilot.eclipse.swtbot.test/test-plans/subagent/subagent.md b/com.microsoft.copilot.eclipse.swtbot.test/test-plans/subagent/subagent.md index ed950cc6..49b667ab 100644 --- a/com.microsoft.copilot.eclipse.swtbot.test/test-plans/subagent/subagent.md +++ b/com.microsoft.copilot.eclipse.swtbot.test/test-plans/subagent/subagent.md @@ -15,6 +15,7 @@ Entry points exercised: - Agent mode with a prompt that triggers `run_subagent`. - Chat history panel: switching to a different conversation and back. - Cancel (stop) button during subagent execution. +- Cancel button on a subagent tool-confirmation dialog. - `conversation/destroy` LSP call on session switch. Not exercised: @@ -222,3 +223,47 @@ Not exercised: #### Key Screenshots - [ ] **After restore** — conversation unchanged, no extra messages. + +--- + +## 6. Tool-confirmation dialog cancel layout + +### TC-007: Subagent panel collapses after confirmation dialog is cancelled + +**Type:** `Regression` +**Priority:** `P1` + +#### Preconditions +- The Chat view is open in Agent mode. +- A new conversation (fresh session). +- The selected agent triggers a tool that requires confirmation + (e.g., a command that needs to run in the terminal). + +#### Steps +1. Send a prompt that triggers subagent execution where the subagent + wants to run a command (e.g., `scan for CVEs`, or + `run ls in the terminal`). +2. Wait for the subagent's tool **confirmation dialog** to appear inside + the subagent card (Allow / Skip buttons). +3. Click **Skip** on the confirmation dialog. +4. Observe the subagent card area after the dialog disposes. + +#### Expected Result +- The subagent panel resizes/collapses to fit its remaining content. +- **No empty/blank gap** is left at the bottom of the subagent area where + the confirmation dialog used to be (the bug in #169). +- A skip label/message is shown where appropriate, and the scroller + minimum height is recomputed in a single layout pass. +- The send button returns to the send state. + +#### Key Screenshots +- [ ] **Confirmation dialog visible** — Allow / Skip buttons shown + inside the subagent card. +- [ ] **After skip** — subagent panel collapsed with no empty area below. + +#### Notes on failure modes +- Empty area left below the subagent card → `cancelConfirmation()` did not + trigger `requestRefreshScrollerLayout()`; the `ScrolledComposite` + `setMinHeight()` was never recomputed after the dialog disposal. +- Verify the Allow path is unaffected: clicking **Allow** on a + separate invocation should still lay out correctly (no regression). diff --git a/com.microsoft.copilot.eclipse.swtbot.test/test-plans/thinking-effort/thinking-effort.md b/com.microsoft.copilot.eclipse.swtbot.test/test-plans/thinking-effort/thinking-effort.md index 02f9e900..3eacc8d1 100644 --- a/com.microsoft.copilot.eclipse.swtbot.test/test-plans/thinking-effort/thinking-effort.md +++ b/com.microsoft.copilot.eclipse.swtbot.test/test-plans/thinking-effort/thinking-effort.md @@ -79,3 +79,48 @@ Entry points: - Model A still shows **`High`**; Model B still shows **`Low`**. - Each model's effort is independent — changing one did not affect the other. +--- + +## TC-003: Thinking effort descriptions are fully readable in the hover card + +**Type:** `Regression` +**Priority:** `P1` + +Regression guard for +[issue #233](https://github.com/microsoft/copilot-for-eclipse/issues/233) / +[PR #234](https://github.com/microsoft/copilot-for-eclipse/pull/234). The hover +card previously clamped its width to a fixed maximum (`LONG_POPUP_WIDTH = 300`), +which truncated the longer per-level description text in the **Thinking effort** +section. The fix removes that cap so the hover shell uses its natural packed +width (only enforcing a `250` px minimum), letting every description wrap and +render in full. + +#### Preconditions +- A reasoning-capable model (e.g. Claude Sonnet 4.6) advertising multiple + effort levels, each with a descriptive sub-label (e.g. + `Low — Faster responses, less reasoning`, + `Medium — Balanced reasoning and speed`, + `High — Deeper reasoning, slower responses`). + +#### Steps +1. Open the Copilot Chat view and click the model picker to open the dropdown. +2. Hover over the reasoning-capable model to open its hover card. +3. Locate the **Thinking effort** section and read each effort level's + description line in full. +4. Visually compare the right edge of the longest description against the hover card border. + +#### Expected Result +- The hover card is wide enough to display the full description for every effort + level — no text is clipped, cut off, or replaced with an ellipsis at the right + edge. +- Each description ends with its natural last word (or wraps onto a second line) + rather than being truncated mid-word against the card border. +- The hover card width is never narrower than the `250` px minimum, so short + content still renders cleanly. + +#### 📸 Key Screenshots +- [ ] **Hover card — full descriptions** — Thinking effort section showing each + level's complete, untruncated description text. +- [ ] **Right-edge close-up** — the longest description's final word fully + visible inside the card border (no clipping/ellipsis). + From fb60576e210676196a7d28ca82ca8f0eff514284 Mon Sep 17 00:00:00 2001 From: Sheng Chen Date: Thu, 25 Jun 2026 09:26:32 +0800 Subject: [PATCH 06/14] =?UTF-8?q?docs:=20Add=20test=20plan=20for=20Run=20I?= =?UTF-8?q?n=20Terminal=20tool=20with=20multi-line=20PowerShe=E2=80=A6=20(?= =?UTF-8?q?#309)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../run-in-terminal/run-in-terminal.md | 112 ++++++++++++++++++ 1 file changed, 112 insertions(+) create mode 100644 com.microsoft.copilot.eclipse.swtbot.test/test-plans/run-in-terminal/run-in-terminal.md diff --git a/com.microsoft.copilot.eclipse.swtbot.test/test-plans/run-in-terminal/run-in-terminal.md b/com.microsoft.copilot.eclipse.swtbot.test/test-plans/run-in-terminal/run-in-terminal.md new file mode 100644 index 00000000..fb6de18c --- /dev/null +++ b/com.microsoft.copilot.eclipse.swtbot.test/test-plans/run-in-terminal/run-in-terminal.md @@ -0,0 +1,112 @@ +# Run In Terminal Tool + +## Overview +Verify that Agent mode invokes `run_in_terminal` with commands that preserve multi-line shell syntax and execute +build commands from the correct imported project root. These cases guard against command flattening and wrong +working-directory selection in terminal tool calls. + +--- + +## Test Cases + +### TC-001: Multi-line PowerShell command executes correctly + +**Type:** `Regression` +**Priority:** `P0` + +#### Preconditions +- Eclipse IDE is open with the GitHub Copilot for Eclipse plugin installed and activated. +- The user is signed in to GitHub Copilot. +- Copilot Chat is open in Agent mode. +- The terminal shell used by `run_in_terminal` is PowerShell. +- Terminal tool confirmation is enabled, or the tester can inspect the generated command before allowing execution. + +#### Steps +1. Start a new Copilot Chat conversation in Agent mode. +2. Send a prompt that asks the agent to run this exact multi-line PowerShell command in the terminal and report the + output: + ```powershell + 1..3 | ForEach-Object { + Write-Output "item=$_" + } + ``` +3. When the `run_in_terminal` confirmation appears, verify that the displayed command keeps the pipeline and block + body as a multi-line command, including the opening `{`, the `Write-Output` line, and the closing `}`. +4. Allow the command to run. +5. Wait for the terminal tool call to complete and inspect the terminal output returned to the agent. + +#### Expected Result +- The command executes successfully without PowerShell parser errors caused by dropped newlines, missing braces, or + truncated block content. +- The output contains exactly these item lines, in order: + ```text + item=1 + item=2 + item=3 + ``` +- The agent response reports the same three output lines and does not claim the command failed. + +#### 📸 Key Screenshots +- [ ] **Multi-line command confirmation** — `run_in_terminal` confirmation showing the full multi-line PowerShell + command. +- [ ] **Multi-line command output** — Terminal or agent output showing `item=1`, `item=2`, and `item=3`. + +--- + +### TC-002: Maven verify command runs from imported project root + +**Type:** `Regression` +**Priority:** `P0` + +#### Preconditions +- Eclipse IDE is open with the GitHub Copilot for Eclipse plugin installed and activated. +- The user is signed in to GitHub Copilot. +- Copilot Chat is open in Agent mode. +- A Maven project is imported into the Eclipse workspace and visible in Project Explorer. +- The imported project root contains `pom.xml` and either a Maven wrapper (`mvnw` / `mvnw.cmd`) or uses a system + `mvn` installation. +- Terminal tool confirmation is enabled, or the tester can inspect the generated command before allowing execution. + +#### Steps +1. Import a Maven project into the Eclipse workspace if one is not already present. +2. Start a new Copilot Chat conversation in Agent mode. +3. Ask the agent to run Maven `verify` for the imported project. +4. When the `run_in_terminal` confirmation appears, inspect the generated command before allowing it. +5. Allow the command to run after confirming the tool will execute from the imported Maven project root. Valid evidence + includes either an explicit `cd ""` in the command or a terminal prompt/working directory + that already points at the imported project root. +6. Wait for the terminal tool call to complete or reach the normal build result for that project. + +#### Expected Result +- The terminal executes Maven `verify` from the imported Maven project root path. +- The execution directory is the project directory that contains the imported project's `pom.xml`, not the Eclipse + workspace root, a parent folder, or an unrelated project. +- The tool satisfies this either by starting the terminal with the imported project root as its working directory or by + using an equivalent explicit directory change before running `verify`, such as: + ```powershell + cd "" + .\mvnw.cmd verify + ``` + or: + ```powershell + cd "" + mvn verify + ``` +- The terminal starts Maven from that project root, so Maven resolves the expected `pom.xml`. + +#### 📸 Key Screenshots +- [ ] **Imported Maven project** — Project Explorer showing the selected/imported Maven project root with `pom.xml`. +- [ ] **Verify execution location** — `run_in_terminal` confirmation or terminal prompt showing the command will execute + from the imported Maven project root. +- [ ] **Verify command output** — Terminal output showing Maven started from the selected project root. + +--- + +## Screenshots Checklist +> Consolidated list of all key screenshot moments. + +- [ ] `TC-001` Multi-line command confirmation +- [ ] `TC-001` Multi-line command output +- [ ] `TC-002` Imported Maven project +- [ ] `TC-002` Verify execution location +- [ ] `TC-002` Verify command output From ff439e6a14d22f423659be3f7b209c86e3343d6b Mon Sep 17 00:00:00 2001 From: Dietrich Travkin <10887297+travkin79@users.noreply.github.com> Date: Thu, 25 Jun 2026 07:10:52 +0200 Subject: [PATCH 07/14] Fix #113: Resize model drop-down list for optional scroll bar (#285) Co-authored-by: Sheng Chen --- .../copilot/eclipse/ui/swt/DropdownPopup.java | 71 +++++++++++++++++-- 1 file changed, 65 insertions(+), 6 deletions(-) diff --git a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/swt/DropdownPopup.java b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/swt/DropdownPopup.java index 7b9dc2f0..344eb79c 100644 --- a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/swt/DropdownPopup.java +++ b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/swt/DropdownPopup.java @@ -28,6 +28,7 @@ import org.eclipse.swt.widgets.Shell; import org.eclipse.ui.PlatformUI; +import com.microsoft.copilot.eclipse.core.utils.PlatformUtils; import com.microsoft.copilot.eclipse.ui.utils.SwtUtils; import com.microsoft.copilot.eclipse.ui.utils.UiUtils; @@ -219,6 +220,7 @@ void open(Point location, List groups, String selectedItemId, shell.pack(); constrainHeightIfNeeded(); + reserveScrollbarSpaceIfNeeded(contentSize); adjustBounds(location, anchorHeight); scrollToFocusedItem(); shell.setVisible(true); @@ -234,14 +236,71 @@ private void constrainHeightIfNeeded() { Rectangle lastBounds = lastVisible.getBounds(); int maxContentHeight = lastBounds.y + lastBounds.height; - int scrollBarWidth = 0; - if (scrolledComposite.getVerticalBar() != null) { - scrollBarWidth = scrolledComposite.getVerticalBar().getSize().x; - } - Point shellSize = shell.getSize(); int newHeight = maxContentHeight + 2 * POPUP_MARGIN; - shell.setSize(shellSize.x + scrollBarWidth, newHeight); + shell.setSize(shellSize.x, newHeight); + } + + /** + * Ensures the vertical scrollbar never hides item content, which manifests differently across platforms. + * + *
    + *
  • Classic (space-reserving) scrollbars (typical on Windows and many GTK themes) shrink the + * {@link ScrolledComposite} client area. With {@link ScrolledComposite#setExpandHorizontal(boolean)} the + * content is stretched to that reduced width, so the shell must be widened by the reserved trough width to + * restore the full natural content width.
    • + *
  • Overlay scrollbars (the default on modern GNOME, e.g. RHEL 9) do not reduce the client + * area; the scrollbar is painted on top of the right edge of the content, covering the right-aligned suffix + * labels. Widening the shell alone does not help because the suffix stays pinned to the (new) right edge, + * still under the overlay. Instead we stop horizontal expansion, keep the content at its natural width, and + * widen the shell so an empty scrollbar-width strip is left on the right for the overlay to paint over.
    • + *
+ * + * @param contentSize the natural (unconstrained) size of the item container + */ + private void reserveScrollbarSpaceIfNeeded(Point contentSize) { + if (items.size() <= MAX_VISIBLE_ITEMS || items.isEmpty()) { + return; + } + + if (scrolledComposite == null || scrolledComposite.isDisposed() || scrolledComposite.getVerticalBar() == null) { + return; + } + + // Fix #113: We adapt the drop-down dialog's size only for Linux (where the issue appeared), + // in order to avoid potential regressions, + // see https://github.com/microsoft/copilot-for-eclipse/pull/285#issuecomment-4686907980 + if (PlatformUtils.isLinux()) { + // Force a layout so the client area reflects the constrained height before we measure it. + // The layout operation doesn't lead to flickering, since it is not user-visible at this point. + shell.layout(true, true); + int scrollBarWidth = scrolledComposite.getVerticalBar().getSize().x; + if (scrollBarWidth <= 0) { + return; + } + Rectangle clientArea = scrolledComposite.getClientArea(); + Point shellSize = shell.getSize(); + if (clientArea.width < contentSize.x) { + // Classic scrollbar: the trough shrank the client area. Widen the shell to restore full content width. + int deficit = contentSize.x - clientArea.width; + shell.setSize(shellSize.x + deficit, shellSize.y); + } else { + // Overlay scrollbar: client area was not reduced, so the bar paints over the content's right edge. Pin the + // content to its natural width and reserve just enough room on the right for the overlay. + scrolledComposite.setExpandHorizontal(false); + Control content = scrolledComposite.getContent(); + if (content != null && !content.isDisposed()) { + content.setSize(contentSize); + } + int extraWidth = Math.max(0, scrollBarWidth - ITEM_H_PADDING); + shell.setSize(shellSize.x + extraWidth, shellSize.y); + } + } else { + // Behavior before fixing #113 + int scrollBarWidth = scrolledComposite.getVerticalBar().getSize().x; + Point shellSize = shell.getSize(); + shell.setSize(shellSize.x + scrollBarWidth, shellSize.y); + } } private void adjustBounds(Point location, int anchorHeight) { From 710528eac01966e6f48b7eb1534254ffaa13e33e Mon Sep 17 00:00:00 2001 From: Sheng Chen Date: Thu, 25 Jun 2026 14:55:40 +0800 Subject: [PATCH 08/14] feat: Add profile to skip tests during UI probe execution (#290) * feat: Add profile to skip tests during UI probe execution * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- .github/skills/ui-action/REFERENCE.md | 169 ++++++++++ .github/skills/ui-action/SKILL.md | 315 +++--------------- .../pom.xml | 25 +- com.microsoft.copilot.eclipse.ui.test/pom.xml | 23 ++ 4 files changed, 267 insertions(+), 265 deletions(-) create mode 100644 .github/skills/ui-action/REFERENCE.md diff --git a/.github/skills/ui-action/REFERENCE.md b/.github/skills/ui-action/REFERENCE.md new file mode 100644 index 00000000..65d4fb3e --- /dev/null +++ b/.github/skills/ui-action/REFERENCE.md @@ -0,0 +1,169 @@ +# SWTBot Probe Reference + +## Runner behavior + +`ProbeRunner` is a JUnit 4 test launched by Tycho under a real Eclipse workbench +with `useUIHarness=true` and `useUIThread=false`. It loads the JSON array from +`-Dprobe.script`, executes each `ProbeStep` through `StepExecutor`, writes result +artifacts, and fails the Maven build if any failed step has `failFast` enabled. + +Before the bot starts, the runner pre-populates configuration-scope preferences +so Quick Start, What's New, Welcome, and "Terminal Support Unavailable" dialogs +do not block normal probes. + +`screenshot` and failure screenshots capture the active workbench shell with +`java.awt.Robot`, then fall back to SWTBot's full-display capture if needed. +Screenshots are diagnostic artifacts only; JSON assertions drive pass/fail. + +## Running probes + +Default command from the repository root: + + # macOS / Linux + ./mvnw clean verify -Dprobe.script=probe-scripts/.json + # Windows (PowerShell) + .\mvnw.cmd clean verify -Dprobe.script=probe-scripts/.json + +The narrower module command is useful only after a green root build: + +```bash +./mvnw -pl com.microsoft.copilot.eclipse.swtbot.test -am -Dprobe.script=probe-scripts/.json verify +``` + +The module-only shortcut can fail dependency resolution or reuse stale bundles, +especially after source edits. If widget markers or other code changes appear to +be ignored, run the root `clean verify` command. + +Platform notes: + +- Linux headless: prefix the command with `xvfb-run -a`. +- macOS: keep `${swtbot.platformArgLine}` in + `com.microsoft.copilot.eclipse.swtbot.test/pom.xml` so the `swtbot-osx` + profile can inject `-XstartOnFirstThread`. +- macOS screenshots: the JVM needs Screen Recording permission. Blank or + wallpaper-only PNGs usually mean permission is missing. +- Do not set `-Djava.awt.headless=true`; `Robot` cannot capture screenshots in + headless AWT mode. +- Windows HiDPI: workbench shell bounds are DIP coordinates while `Robot` + expects raw pixels, so captures can clip at scaling above 100%. + +Windows stale cache recovery: + +```powershell +Remove-Item -Recurse -Force $env:USERPROFILE\.m2\repository\com\microsoft\copilot\eclipse +``` + +## Results + +Artifacts are written under +`com.microsoft.copilot.eclipse.swtbot.test/target/probe-results/`: + +```text +results.json # pass/fail summary +workspace.log # sandbox Eclipse .metadata/.log +screenshots/ + .png # from screenshot steps + FAILED-stepNN-.png # auto-captured on step failure +ui-dumps/.xml # from dumpUi steps +``` + +Quick pass/fail checks: + +```bash +jq '{passed, failed, failed_steps: [.steps[] | select(.status=="failed") | {index, action, message}]}' \ + com.microsoft.copilot.eclipse.swtbot.test/target/probe-results/results.json +``` + +```powershell +Get-Content com.microsoft.copilot.eclipse.swtbot.test/target/probe-results/results.json | + ConvertFrom-Json | Select-Object passed, failed, durationMs +``` + +Maven exit code `0` and `results.json` `.failed == 0` means pass. Otherwise, +open the failed step message, `FAILED-stepNN-*.png`, nearest `ui-dumps/*.xml`, +and `workspace.log`. Many "widget not found" failures are downstream of a +Copilot language server startup or authentication failure. + +## Authentication + +The Copilot JS agent reads its GitHub token from the host's standard Copilot +store (`%USERPROFILE%\AppData\Local\github-copilot\apps.json` on Windows; +`~/.config/github-copilot/apps.json` elsewhere). The Tycho JVM inherits it. + +- Signed-in host required for probes that exercise chat completion. +- To probe unauthenticated state, assert the "Sign in to GitHub" UI instead of + polling internal status managers. +- CI-side auth bootstrap is out of scope for this skill. + +## Actions + +Set `"failFast": false` on a step to record a failure without aborting the probe. + +| `action` | Required | Optional | Notes | +|---|---|---|---| +| `screenshot` | - | `id` | PNG of the active workbench window written to `screenshots/`. | +| `sleep` | - | `timeoutSec` default `1` | Plain `Thread.sleep`. Prefer waits over sleeps. | +| `waitForIdle` | - | - | Flushes the SWT event queue. | +| `pressKey` | `key` | `locator` | Supports `ENTER`, `RETURN`, `CR`, `ESC`, `ESCAPE`, `TAB`, `SPACE`, `BS`, `BACKSPACE`, `DELETE`. Uses SWTBot `pressShortcut`; with a text/styled-text locator it targets that widget, otherwise the active shell. | +| `showView` | `idRef` | - | Opens an Eclipse view via `IWorkbenchPage#showView`. | +| `closeView` | `idRef` | - | Hides the view if present. | +| `invokeCommand` | `idRef` | - | Runs a registered Eclipse command via `IHandlerService#executeCommand`. | +| `click` | `locator` | - | Focuses text/styled-text widgets; otherwise reflectively invokes `click()` on the matched widget. | +| `typeIn` | `locator`, `text` | - | Sets text on text/styled-text widgets. | +| `clearElement` | `locator` | - | Clears text/styled-text widgets. | +| `waitFor` | `locator` | `timeoutSec` default `30` | Polls until the locator resolves. | +| `waitForMethod` | `locator`, `method` | `timeoutSec` default `30`, `expectedValue` | Polls a no-arg getter on the located widget until it returns non-null/non-empty, or equals `expectedValue`. | +| `assertExists` | `locator` | `shouldExist` default `true` | Asserts presence or absence. | +| `dumpUi` | - | `id` | Writes shell widget hierarchy XML with class names and SWTBot widget IDs. | +| `newSession` | - | - | Copilot-specific shortcut for the `newChatSession` command. | + +## Locators + +Locators are JSON objects with a `by` discriminator. SWTBot is not XPath-based; +extend `Locator` and `StepExecutor` when the existing vocabulary is not enough. + +| `by` | Other fields | What it finds | +|---|---|---| +| `viewId` | `id` | Eclipse view by ID (`bot.viewById`). | +| `label` | `text` | First label with the given text. | +| `button` | `text` | First button with the given text. | +| `buttonWithTooltip` | `tooltip` | First button whose tooltip matches; useful for icon-only buttons like Send. | +| `text` | `index` default `0` | Nth text field. | +| `styledText` | - | First `StyledText`. | +| `tree` | `labels` array | Tree path under the active tree. | +| `cssId` | `value` | Widget whose `CssConstants.CSS_ID_KEY` data equals `value`. | +| `cssClass` | `value` | Widget whose `CssConstants.CSS_CLASS_NAME_KEY` contains `value` as a whitespace-separated token. | +| `widgetId` | `value` | Widget tagged with `setData("org.eclipse.swtbot.widget.key", value)`. Preferred for widgets you own. | +| `widgetClass` | `value` | First widget whose `getClass().getSimpleName()` equals `value`. Use only when you cannot tag the widget. | + +Stable IDs and signals currently used by probes: + +- `widgetId`: `user-turn`, `copilot-turn`, `model-picker`. +- `cssClass`: `model-info-label` appears only after a Copilot turn has completed. +- `cssId`: `chat-container`, `chat-content-wrapper`, `chat-content-viewer`, + `chat-action-bar-wrapper`, `chat-action-bar`, `chat-history-viewer`. + +## Troubleshooting + +| Symptom | Likely cause / fix | +|---|---| +| `IllegalArgumentException: Missing required field: ...` | Step JSON is missing a field required by that action. Check the action table. | +| `AssertionError: waitFor timed out: locator ...` | Widget did not appear. Add `dumpUi` before the failing step, confirm class/id, or raise `timeoutSec`. | +| `assertExists failed: ... shouldExist=true` | Locator did not match. Confirm the `by` type matches the widget family. | +| `click not supported on ` | Wrapper has no `click()` method. Use a more specific locator, such as `button` instead of `label`. | +| Tests skipped: "No probe script specified" | Pass `-Dprobe.script=probe-scripts/.json`. | +| `model-info-label` wait times out | Usually authentication or language-server startup. Inspect `workspace.log`. | +| All screenshots are blank or tiny identical PNGs | `Robot` capture failed. On macOS, grant Screen Recording permission to the JVM and re-run. | +| Widget-id markers missing from `dumpUi` | Stale bundle cache or stale module jar. Run root `./mvnw clean verify`. | + +## Extending the vocabulary + +Probe support lives in `com.microsoft.copilot.eclipse.swtbot.test/src/.../probe/`: + +- `ProbeStep.java` and `Locator.java`: JSON shape. +- `StepExecutor.java`: action dispatch, locator resolution, screenshots, dumps. +- `ProbeRunner.java`: runner loop, reporting, and workbench setup. + +Add UI-level primitives that mimic user behavior. Do not add actions that call +private production methods, reach into OSGi services, or mutate app state behind +the UI; those hide real UX bugs and break on internal refactors. diff --git a/.github/skills/ui-action/SKILL.md b/.github/skills/ui-action/SKILL.md index a586d100..5b4740cc 100644 --- a/.github/skills/ui-action/SKILL.md +++ b/.github/skills/ui-action/SKILL.md @@ -1,19 +1,18 @@ --- name: ui-action -description: Write or update a SWTBot JSON probe script (e.g. from a test plan), then validate it locally by running the `ProbeRunner` Tycho test against the Copilot for Eclipse plugin. Use this whenever you need end-to-end UI validation against a real Eclipse workbench. +description: Authors and validates SWTBot JSON probe scripts for GitHub Copilot for Eclipse UI flows against a real Eclipse workbench. Use when creating or updating probe-scripts, converting test plans to UI probes, validating end-to-end Eclipse UI behavior, or troubleshooting ProbeRunner failures. --- -# Authoring and Validating SWTBot Probe Scripts (Eclipse) +# SWTBot Probe Scripts -Use this skill to write or update a JSON probe script and validate it by -invoking the `com.microsoft.copilot.eclipse.swtbot.test` Tycho bundle locally. -Probe scripts are JSON, so no Java is compiled per test case. +Use this skill to create or update JSON probes for `com.microsoft.copilot.eclipse.swtbot.test`. +Probes drive the workbench through SWTBot actions and run as Tycho tests; no Java is compiled per test case. ## Quick start -1. Drop a probe at `com.microsoft.copilot.eclipse.swtbot.test/probe-scripts/-.json` - (e.g. `chat-send-receive-001.json`). Start every probe with this preamble that - settles the workbench and opens the Copilot Chat view: +1. Create one focused JSON script at + `com.microsoft.copilot.eclipse.swtbot.test/probe-scripts/-.json`. +2. Start by settling the workbench and opening the view under test: ```json [ @@ -25,275 +24,63 @@ Probe scripts are JSON, so no Java is compiled per test case. ] ``` -2. Run it (cross-platform command; use `./mvnw` on macOS/Linux): +3. Add user-level steps only, such as `click`, `typeIn`, `clearElement`, `pressKey`, + `invokeCommand`, `waitFor`, `waitForMethod`, `assertExists`, `screenshot`, or + `dumpUi`. Prefer `widgetId` locators for widgets the plugin owns. +4. Run from the repository root: - ```powershell + ```bash ./mvnw clean verify -Dprobe.script=probe-scripts/.json ``` - Root `clean verify` is the recommended default. Tycho prefers each - bundle's freshly-packaged jar in `/target/` and silently falls - back to the stale Maven cache if the jar is missing — making `setData` - markers and other source edits appear to be ignored. + Use root `clean verify` as the default. Tycho can reuse stale bundle jars + when only the SWTBot module is run, making source edits appear ignored. Use + `xvfb-run -a` on Linux headless. +5. Read `com.microsoft.copilot.eclipse.swtbot.test/target/probe-results/results.json`, + `workspace.log`, `screenshots/`, and `ui-dumps/`. - The narrower `./mvnw -pl com.microsoft.copilot.eclipse.swtbot.test -am - -Dprobe.script=... verify` shortcut is **experimental**: in practice - it can fail dependency resolution (observed: `Missing requirement: - com.microsoft.copilot.eclipse.core 0.0.0`, with Tycho choosing the - wrong `osgi.arch` on aarch64). Use only as a follow-up, after a green - root `clean verify`. +If `-Dprobe.script` is unset, `ProbeRunner` skips itself so ordinary `./mvnw verify` runs are unaffected. -3. Read results under `com.microsoft.copilot.eclipse.swtbot.test/target/probe-results/`: +## Authoring workflow - ``` - results.json # pass/fail summary - workspace.log # sandbox Eclipse .metadata/.log - screenshots/ - .png # from `screenshot` steps - FAILED-stepNN-.png # auto-captured on step failure - ui-dumps/.xml # from `dumpUi` steps - ``` - -If `-Dprobe.script` is unset the test is skipped, so ordinary `./mvnw verify` -runs are unaffected. - -> **Linux headless:** prefix with `xvfb-run -a`. **macOS:** the swtbot test -> pom carries a `swtbot-osx` profile that injects `-XstartOnFirstThread` -> into the test JVM via the `swtbot.platformArgLine` placeholder -> interpolated into ``. If you ever edit -> `com.microsoft.copilot.eclipse.swtbot.test/pom.xml`, **keep the -> `${swtbot.platformArgLine}` token in ``** — replacing it with a -> plain literal silently drops the macOS arg and the workbench fails with -> `SWTException: Invalid thread access`. **Stale cache recovery:** -> `Remove-Item -Recurse -Force $env:USERPROFILE\.m2\repository\com\microsoft\copilot\eclipse` -> then re-run `clean verify`. - -## How the runner works - -`ProbeRunner` is a JUnit 4 test Tycho launches under a real Eclipse workbench -(`useUIHarness=true`, `useUIThread=false`). It: - -1. Loads the JSON probe pointed at by `-Dprobe.script`. -2. Walks each step via `SWTWorkbenchBot`, wrapping every step in a uniform - try/catch so one failure doesn't crash the run. -3. Writes `results.json`, screenshots (PNG — `SWTBotPreferences.SCREENSHOT_FORMAT` - is pinned), and widget-tree dumps. -4. Fails the Maven build if any `failFast` step failed. - -Before the bot starts, the runner pre-populates configuration-scope -preferences so Quick Start, What's New, Welcome, and "Terminal Support -Unavailable" dialogs don't pop during a probe — probes see a clean workbench. - -`screenshot` and `dumpUi` capture from the **on-screen workbench shell**: -`StepExecutor#captureWorkbenchShell` resolves the active workbench -`Shell`'s bounds, then snapshots those screen pixels with -`java.awt.Robot.createScreenCapture` (with SWTBot's full-display -`bot.captureScreenshot` as a final fallback). SWT's own GC-based capture -(`new GC(display).copyArea(...)` and `Shell#print(GC)`) returns blank -images on macOS Cocoa for workbench shells — Robot is the workaround. -Caveats: - -- **macOS:** the JVM running the tests needs **Screen Recording** - permission (System Settings → Privacy & Security → Screen Recording). - Without it Robot returns blank or wallpaper-only frames. -- **Headless AWT:** `-Djava.awt.headless=true` makes `new Robot()` throw, - and the SWTBot fallback is also blank on Cocoa. Don't set it. -- **Windows HiDPI:** `Shell#getBounds()` returns DIP coordinates; Robot - expects raw screen pixels. At display scaling > 100% the captured - region is undersized / clipped. (At 100% — typical CI runners — it's - a pixel-perfect match.) - -## Authoring rules - -### Drive the UI, don't reach into services - -Probes **must** interact with the workbench the way a user does — SWTBot -clicks, typing, menu navigation, keyboard shortcuts, or `invokeCommand` -against a registered Eclipse command id (the same `IHandlerService#executeCommand` -path that menus and key bindings trigger — useful when an entry point is -buried in a popup like the Copilot status-bar menu, which is awkward to -drive with SWTBot). Do not add actions that reflectively invoke private -methods, call OSGi services to flip state, or read private fields; those -hide real UX bugs and break on every internal refactor. If a scenario needs -a new primitive (e.g. a chat-mode dropdown picker), extend the -action/locator vocabulary in `StepExecutor` with a UI-level primitive, -not a service shortcut. +- Convert one scenario/test case into one probe. Split unrelated behaviors so each failure screenshot has one meaning. +- Drive the UI the way a user does. `invokeCommand` is allowed for registered Eclipse command IDs; do not call + OSGi services, private methods, or test-only shortcuts. +- Pair screenshots with machine-checked assertions. Screenshots never fail a step; `assertExists`, `waitFor`, and + `waitForMethod` decide pass/fail. +- Wait for asynchronous UI state before acting. For chat, wait for the `model-picker` selected model before Send, + then wait for `user-turn` and `model-info-label`. +- When a locator fails, insert `dumpUi` before the failure and inspect `ui-dumps/*.xml` for widget class names and + SWTBot widget IDs. +- For unauthenticated chat probes, assert the "Sign in to GitHub" UI. Do not poll internal auth managers. -### Tag widgets for `widgetId` locators - -SWTBot's blessed identification convention is -`widget.setData("org.eclipse.swtbot.widget.key", "")` at widget -construction, located from tests with the `widgetId` locator. Zero runtime -cost, far more robust than class-name or creation-order lookup. Prefer -`widgetId` over `widgetClass` whenever you control the widget's construction; -use `widgetClass` only for platform / third-party widgets you can't tag. - -Current tags: `user-turn` (`UserTurnWidget`), `copilot-turn` -(`CopilotTurnWidget`), `model-picker` (chat-model `DropdownButton`). - -### Screenshots are artifacts; assertions catch regressions - -`screenshot` never fails a step. Validation has two layers: - -- **Structural** (machine-checked): `assertExists`, `waitFor`, specific - locators. These failures land in `results.json` and drive the build exit - code — the agent judges pass/fail from here. -- **Visual** (agent-checked): after the run, open the PNGs under - `screenshots/` with your vision tool and describe what you see. No - baseline-image comparison in JSON (cross-platform font rendering makes - pixel diffs flaky). - -Pair screenshots with assertions around every non-trivial interaction: +## Chat send skeleton ```json -{ "action": "screenshot", "id": "before-send" }, -{ "action": "click", "locator": { "by": "buttonWithTooltip", "tooltip": "Send" } }, -{ "action": "waitForIdle" }, -{ "action": "assertExists", "locator": { "by": "widgetId", "value": "user-turn" } }, -{ "action": "screenshot", "id": "after-send" } +[ + { "action": "waitForIdle" }, + { "action": "showView", "idRef": "com.microsoft.copilot.eclipse.ui.chat.ChatView" }, + { "action": "waitFor", "locator": { "by": "styledText" }, "timeoutSec": 30 }, + { "action": "clearElement", "locator": { "by": "styledText" } }, + { "action": "typeIn", "locator": { "by": "styledText" }, "text": "hello" }, + { "action": "waitForMethod", "locator": { "by": "widgetId", "value": "model-picker" }, + "method": "getSelectedItemId", "timeoutSec": 60 }, + { "action": "click", "locator": { "by": "buttonWithTooltip", "tooltip": "Send" } }, + { "action": "waitFor", "locator": { "by": "widgetId", "value": "user-turn" }, "timeoutSec": 30 }, + { "action": "waitFor", "locator": { "by": "cssClass", "value": "model-info-label" }, "timeoutSec": 120 }, + { "action": "assertExists", "locator": { "by": "widgetId", "value": "copilot-turn" } }, + { "action": "screenshot", "id": "agent-response" } +] ``` -When a locator is wrong, insert a `dumpUi` step and inspect -`ui-dumps/*.xml` to find the real widget class / id. - -### Authentication prerequisite - -The Copilot JS agent reads its GitHub token from the host's standard Copilot -store (`%USERPROFILE%\AppData\Local\github-copilot\apps.json` on Windows; -`~/.config/github-copilot/apps.json` elsewhere) — the Tycho JVM inherits it. - -- **Signed-in host required** for any probe that exercises chat. Sign in via - any Copilot client (Eclipse plugin, VS Code, `gh auth login`). -- **Probing unauthed state:** assert on the "Sign in to GitHub" button that - replaces the chat input; don't poll internal status managers. -- CI-side auth bootstrap is out of scope for this skill. - -## Reading results & interpreting failures - -Maven exit code `0` **and** `results.json` `.failed == 0` → overall pass. -Otherwise open the failed step's `message`, its `FAILED-stepNN-*.png`, the -nearest `ui-dumps/*.xml`, **and** `workspace.log` — many "widget not found" -failures are downstream of a Copilot LS that never started or authenticated. -Look for `!ENTRY com.microsoft.copilot.eclipse` entries and NPEs on -`CopilotLanguageServer.*`. - -| Symptom | What's wrong / fix | -|---|---| -| `IllegalArgumentException: Missing required field: …` | Step JSON missing a field required by that action (see action table). | -| `AssertionError: waitFor timed out: locator …` | Widget not appearing. Add a `dumpUi` before the failing step, check class/id, or raise `timeoutSec`. | -| `assertExists failed: … shouldExist=true` | Locator didn't match. Confirm `by` matches the widget family. | -| `click not supported on ` | Wrapper has no `click()`; use the right `by` (e.g. `button`, not `label`). | -| Tests skipped: "No probe script specified" | Pass `-Dprobe.script=probe-scripts/.json`. | -| `model-info-label` wait times out | Usually auth; open `workspace.log`. | -| All screenshots are blank / identical ~4KB PNGs across every step | Robot capture failed. macOS: grant Screen Recording permission to the JVM and re-run. Verify by file size — real workbench frames are 50KB+; a uniform 4KB family means the on-screen capture path didn't run or returned empty pixels. | -| Widget-id markers missing from `dumpUi` | Stale Maven cache — run full `clean verify` (see Quick start). | - -Quick pass/fail check: - -```powershell -Get-Content com.microsoft.copilot.eclipse.swtbot.test/target/probe-results/results.json | - ConvertFrom-Json | Select-Object passed, failed, durationMs -``` - -```bash -jq '{passed, failed, failed_steps: [.steps[] | select(.status=="failed") | {index, action, message}]}' \ - com.microsoft.copilot.eclipse.swtbot.test/target/probe-results/results.json -``` - -## Action reference - -Set `"failFast": false` on a step to record a failure without aborting the probe. - -| `action` | Required | Optional | Notes | -|---|---|---|---| -| `screenshot` | — | `id` | PNG of the active workbench window written to `screenshots/`. | -| `sleep` | — | `timeoutSec` (default 1) | Plain `Thread.sleep`. | -| `waitForIdle` | — | — | Flushes the SWT event queue. | -| `pressKey` | `key` | `locator` | Accepts `ENTER`/`CR`, `ESC`/`ESCAPE`, `TAB`, `SPACE`, `BS`/`BACKSPACE`, `DELETE`. Without `locator`, presses on the active shell; with a `locator` targeting a text / styledText widget, sends the key directly (needed for chat input ENTER-to-send). | -| `showView` | `idRef` (Eclipse view id) | — | Opens via `IWorkbenchPage#showView`. | -| `closeView` | `idRef` | — | Hides the view if present. | -| `invokeCommand` | `idRef` (command id) | — | Runs via `IHandlerService#executeCommand`. | -| `click` | `locator` | — | Reflective `click()` on matched widget. | -| `typeIn` | `locator`, `text` | — | Works on text / styled-text widgets. | -| `clearElement` | `locator` | — | Sets text to empty. | -| `waitFor` | `locator` | `timeoutSec` (default 30) | Polls until locator resolves. | -| `waitForMethod` | `locator`, `method` | `timeoutSec` (default 30), `expectedValue` | Polls until a no-arg getter on the located widget returns non-null (and non-empty for `String`), or — if `expectedValue` is set — until `toString()` equals it. Invoked reflectively via the class hierarchy. Use for UI-exposed state not reachable via finders, e.g. `DropdownButton.getSelectedItemId()`. | -| `assertExists` | `locator` | `shouldExist` (default true) | Asserts presence / absence. | -| `dumpUi` | — | `id` | Writes widget hierarchy XML. | -| `newSession` | — | — | Copilot-specific: triggers `newChatSession` command. | - -## Locator reference - -Locators are JSON objects with a `by` discriminator. SWTBot is **not -XPath-based** — stick to this vocabulary; extend `Locator` / `StepExecutor` -if you need a new finder. - -| `by` | Other fields | What it finds | -|---|---|---| -| `viewId` | `id` | Eclipse view by id (`bot.viewById`). | -| `label` | `text` | First label with the given text. | -| `button` | `text` | First button with the given text. | -| `buttonWithTooltip` | `tooltip` | First button whose tooltip matches (use for icon-only buttons like **Send**). | -| `text` | `index` (default 0) | Nth text field. | -| `styledText` | — | First StyledText (editors / chat input). | -| `tree` | `labels` (array) | Tree path under the active tree. | -| `cssId` | `value` | Widget whose `CssConstants.CSS_ID_KEY` equals `value` (e.g. `chat-content-viewer`, `chat-action-bar`). Walks the full shell tree. | -| `cssClass` | `value` | Widget whose `CssConstants.CSS_CLASS_NAME_KEY` contains `value` as a token. `model-info-label` is only set on a **completed** Copilot turn, so waiting on it is a reliable "response received" signal. | -| `widgetId` | `value` | Widget tagged with `setData("org.eclipse.swtbot.widget.key", value)`. **Preferred** identifier for widgets you own. | -| `widgetClass` | `value` | First widget whose `getClass().getSimpleName()` equals `value` (walks the full shell tree). Fallback for widgets you can't tag. | - -## Canonical example: send a prompt and verify a response - -```json -{ "action": "clearElement", "locator": { "by": "styledText" } }, -{ "action": "typeIn", "locator": { "by": "styledText" }, "text": "your prompt" }, -{ "action": "screenshot", "id": "typed" }, - -{ "action": "waitForMethod", - "locator": { "by": "widgetId", "value": "model-picker" }, - "method": "getSelectedItemId", - "timeoutSec": 60 }, - -{ "action": "click", "locator": { "by": "buttonWithTooltip", "tooltip": "Send" } }, - -{ "action": "waitFor", "locator": { "by": "widgetId", "value": "user-turn" }, - "timeoutSec": 30 }, -{ "action": "assertExists", "locator": { "by": "widgetId", "value": "user-turn" } }, - -{ "action": "waitFor", "locator": { "by": "cssClass", "value": "model-info-label" }, - "timeoutSec": 120 }, -{ "action": "screenshot", "id": "agent-response" }, -{ "action": "assertExists", "locator": { "by": "widgetId", "value": "copilot-turn" } } -``` - -Key signals: - -- `model-picker.getSelectedItemId()` non-null — the workbench has resolved an - active chat model. Sending before this NPEs in `onSendInternal` with - "activeModel is null" because auth + model fetch complete asynchronously. -- `user-turn` — the user turn has rendered (Send button dispatched the prompt). -- `model-info-label` — only appears once a Copilot turn has **completed** - (rendered in the turn's footer at end of streaming); the reliable "response - fully received" signal. -- `copilot-turn` — the assistant turn container; good secondary assertion. - -Without a signed-in host the language server can't complete a turn, so -`model-info-label` never appears and the step times out — itself a useful -diagnostic (check `workspace.log`). - -## Extending the vocabulary - -Everything the probe understands lives in -`com.microsoft.copilot.eclipse.swtbot.test/src/.../probe/`: +## Pass/fail judgment -- `ProbeStep.java` / `Locator.java` — JSON shape. -- `StepExecutor.java` — action dispatch + locator resolution. -- `ProbeRunner.java` — runner loop, reporting, screenshots. +A probe passes only when Maven exits `0` and `results.json` has `"failed": 0`. +On failure, inspect the failed step message, `FAILED-stepNN-*.png`, nearest `ui-dumps/*.xml`, and `workspace.log`. -Add a case to the `switch` in `StepExecutor#execute` (or `resolve`) and the new -action is usable from every probe. +## Extending probes -### Keep one probe focused +The vocabulary lives in `com.microsoft.copilot.eclipse.swtbot.test/src/.../probe/`. +Add UI-level primitives in `ProbeStep`, `Locator`, and `StepExecutor`, then update [REFERENCE.md](REFERENCE.md). -Each JSON script represents one test case. Split unrelated behaviours into -separate probes so a single `FAILED-step…` screenshot tells you what broke. \ No newline at end of file +See [REFERENCE.md](REFERENCE.md) for action/locator tables, platform notes, result queries, troubleshooting, and current stable widget IDs. diff --git a/com.microsoft.copilot.eclipse.core.test/pom.xml b/com.microsoft.copilot.eclipse.core.test/pom.xml index 3231acca..dcc3fca9 100644 --- a/com.microsoft.copilot.eclipse.core.test/pom.xml +++ b/com.microsoft.copilot.eclipse.core.test/pom.xml @@ -14,4 +14,27 @@ true - \ No newline at end of file + + + + skip-tests-during-ui-probe + + + probe.script + + + + + + org.eclipse.tycho + tycho-surefire-plugin + ${tycho-version} + + true + + + + + + + diff --git a/com.microsoft.copilot.eclipse.ui.test/pom.xml b/com.microsoft.copilot.eclipse.ui.test/pom.xml index 2514c9b6..d6296186 100644 --- a/com.microsoft.copilot.eclipse.ui.test/pom.xml +++ b/com.microsoft.copilot.eclipse.ui.test/pom.xml @@ -15,6 +15,29 @@ true + + + skip-tests-during-ui-probe + + + probe.script + + + + + + org.eclipse.tycho + tycho-surefire-plugin + ${tycho-version} + + true + + + + + + + From 645bf6c5896a1e9d3132aefd50d93aab61e23ec6 Mon Sep 17 00:00:00 2001 From: xinyi-gong Date: Tue, 30 Jun 2026 12:30:20 +0800 Subject: [PATCH 09/14] fix - reduce streaming render cost with incremental layout and event batching (#321) --- .../eclipse/ui/chat/BaseTurnWidget.java | 4 +- .../eclipse/ui/chat/ChatContentViewer.java | 407 ++++++++++++------ .../copilot/eclipse/ui/chat/ChatView.java | 22 +- .../eclipse/ui/chat/ThinkingBlock.java | 3 +- .../ui/chat/services/AgentToolService.java | 2 +- 5 files changed, 290 insertions(+), 148 deletions(-) diff --git a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/BaseTurnWidget.java b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/BaseTurnWidget.java index 45557687..1218ce5a 100644 --- a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/BaseTurnWidget.java +++ b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/BaseTurnWidget.java @@ -654,8 +654,8 @@ public CompletableFuture requestToolExecuti this.confirmDialog.addDisposeListener(e -> { Composite ancestor = this.getParent(); while (ancestor != null && !ancestor.isDisposed()) { - if (ancestor instanceof ChatContentViewer) { - ((ChatContentViewer) ancestor).requestRefreshScrollerLayout(); + if (ancestor instanceof ChatContentViewer viewer) { + SwtUtils.invokeOnDisplayThreadAsync(() -> viewer.refreshLayoutFull(), viewer); break; } ancestor = ancestor.getParent(); diff --git a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/ChatContentViewer.java b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/ChatContentViewer.java index 86a0bbf7..7188df57 100644 --- a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/ChatContentViewer.java +++ b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/ChatContentViewer.java @@ -7,7 +7,10 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Queue; import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.ConcurrentLinkedQueue; +import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicReference; import java.util.regex.Pattern; @@ -23,6 +26,7 @@ import org.eclipse.swt.layout.GridData; import org.eclipse.swt.layout.GridLayout; import org.eclipse.swt.widgets.Composite; +import org.eclipse.swt.widgets.Control; import org.eclipse.swt.widgets.ScrollBar; import org.eclipse.ui.PlatformUI; @@ -70,9 +74,18 @@ public class ChatContentViewer extends ScrolledComposite { private BaseTurnWidget latestUserTurn; private CopilotTurnWidget latestCopilotTurn; private BaseTurnWidget latestTurnWidget; - // Auto-scroll state management private boolean autoScrollEnabled; + /** Streaming events queued by LSP threads and drained on the UI thread in batches. */ + private final Queue pendingEvents = new ConcurrentLinkedQueue<>(); + private final AtomicBoolean drainScheduled = new AtomicBoolean(false); + + /** Client-area width of the last layout pass; a change forces a full re-measure (text re-wraps). */ + private int lastLayoutWidth = -1; + + /** Guards against scheduling more than one pending async refresh from a burst of resize events. */ + private final AtomicBoolean refreshScheduled = new AtomicBoolean(false); + /** * Create the composite. * @@ -98,7 +111,9 @@ public ChatContentViewer(Composite parent, int style, ChatServiceManager service this.addControlListener(new ControlAdapter() { @Override public void controlResized(ControlEvent e) { - refreshScrollerLayout(); + // setMinHeight re-fires controlResized; coalesce into one async incremental pass so the + // resize handler never recurses synchronously into layout. + coalesceAsync(refreshScheduled, ChatContentViewer.this::refreshLayoutIncremental); } }); @@ -135,7 +150,7 @@ public void startNewTurn(String workDoneToken, String message) { turnWidget.appendMessage(message); turnWidget.flushMessageBuffer(); - refreshScrollerLayout(); + refreshLayoutFull(); scrollToLatestUserTurn(); // Reset auto-scroll for new conversation turn autoScrollEnabled = true; @@ -186,123 +201,140 @@ public void setConversationId(String conversationId) { } /** - * Process turn event. + * Process turn event. Events are queued and drained in batches on the UI thread so the LSP thread + * is never blocked and multiple in-flight events coalesce into a single layout pass. */ public void processTurnEvent(ChatProgressValue value) { - SwtUtils.invokeOnDisplayThread(() -> { - if (!turns.containsKey(value.getTurnId())) { - CopilotCore.LOGGER.error(new IllegalStateException("turnId not found: " + value.getTurnId())); - return; - } - BaseTurnWidget turnWidget = turns.get(value.getTurnId()); - if (turnWidget == null) { - appendMessageToTheLatestTurn(value.getReply()); - } - - ChatServiceManager chatServiceManager = CopilotUi.getPlugin().getChatServiceManager(); - - if (value.getKind() == WorkDoneProgressKind.report) { - if (turnWidget instanceof ThinkingTurnWidget thinkingTurn) { - thinkingTurn.setConversationContext(conversationId, value.getTurnId()); - thinkingTurn.appendThinking(value.getThinking()); - updateActiveThinkingBlockId(value.getTurnId(), thinkingTurn); - if (hasRenderableOutput(value)) { - // Seal before appending the reply so the spinner stops and the title is fetched. - thinkingTurn.sealThinking(); - } - } + pendingEvents.offer(value); + coalesceAsync(drainScheduled, this::drainPendingEvents); + } - if (value.getAgentRounds() != null && !value.getAgentRounds().isEmpty()) { - // Handle agent mode responses - AgentRound agentRound = value.getAgentRounds().get(0); + private void drainPendingEvents() { + if (isDisposed()) { + pendingEvents.clear(); + return; + } + ChatProgressValue event; + while ((event = pendingEvents.poll()) != null) { + doProcessTurnEvent(event); + } + refreshLayoutIncremental(); + scrollToBottomIfAutoScroll(); + // Events may have arrived while draining; schedule a follow-up drain if so. + if (!pendingEvents.isEmpty()) { + coalesceAsync(drainScheduled, this::drainPendingEvents); + } + } - if (agentRound.getReply() != null) { - turnWidget.appendMessage(agentRound.getReply()); - } + private void doProcessTurnEvent(ChatProgressValue value) { + if (!turns.containsKey(value.getTurnId())) { + CopilotCore.LOGGER.error(new IllegalStateException("turnId not found: " + value.getTurnId())); + return; + } + BaseTurnWidget turnWidget = turns.get(value.getTurnId()); + if (turnWidget == null) { + CopilotCore.LOGGER.error(new IllegalStateException("turnWidget not found: " + value.getTurnId())); + appendMessageToTheLatestTurn(value.getReply()); + return; + } - if (agentRound.getToolCalls() != null && !agentRound.getToolCalls().isEmpty()) { - AgentToolCall toolCall = agentRound.getToolCalls().get(0); - turnWidget.appendToolCallStatus(toolCall); + ChatServiceManager chatServiceManager = CopilotUi.getPlugin().getChatServiceManager(); - // Extract and process todo list from tool result details - processTodoListFromToolCall(chatServiceManager, value.getConversationId(), toolCall); - } - } else { - // Handle chat mode responses - turnWidget.appendMessage(value.getReply()); - } - } else if (value.getKind() == WorkDoneProgressKind.end) { - // Seal any in-progress thinking block before the turn ends. - if (turnWidget instanceof ThinkingTurnWidget thinkingTurn) { + if (value.getKind() == WorkDoneProgressKind.report) { + if (turnWidget instanceof ThinkingTurnWidget thinkingTurn) { + thinkingTurn.setConversationContext(conversationId, value.getTurnId()); + thinkingTurn.appendThinking(value.getThinking()); + updateActiveThinkingBlockId(value.getTurnId(), thinkingTurn); + if (hasRenderableOutput(value)) { + // Seal before appending the reply so the spinner stops and the title is fetched. thinkingTurn.sealThinking(); - updateActiveThinkingBlockId(value.getTurnId(), thinkingTurn); } - turnWidget.flushMessageBuffer(); } - refreshScrollerLayout(); - // Auto-scroll to bottom if enabled - if (shouldAutoScrollToBottom()) { - scrollToBottom(); - } + if (value.getAgentRounds() != null && !value.getAgentRounds().isEmpty()) { + // Handle agent mode responses + AgentRound agentRound = value.getAgentRounds().get(0); + + if (agentRound.getReply() != null) { + turnWidget.appendMessage(agentRound.getReply()); + } + + if (agentRound.getToolCalls() != null && !agentRound.getToolCalls().isEmpty()) { + AgentToolCall toolCall = agentRound.getToolCalls().get(0); + turnWidget.appendToolCallStatus(toolCall); - String errMsg = value.getErrorMessage(); - if (StringUtils.isNotEmpty(errMsg)) { - errMsg = REQUEST_ID_SUFFIX.matcher(errMsg).replaceAll(StringUtils.EMPTY).trim(); + // Extract and process todo list from tool result details + processTodoListFromToolCall(chatServiceManager, value.getConversationId(), toolCall); + } + } else { + // Handle chat mode responses + turnWidget.appendMessage(value.getReply()); } - String reason = value.getErrorReason(); - if (StringUtils.isNotEmpty(reason) && reason.equals("model_not_supported")) { - // TODO: add enable button for better UX. - errMsg = Messages.chat_model_unsupported_message; + } else if (value.getKind() == WorkDoneProgressKind.end) { + // Seal any in-progress thinking block before the turn ends. + if (turnWidget instanceof ThinkingTurnWidget thinkingTurn) { + thinkingTurn.sealThinking(); + updateActiveThinkingBlockId(value.getTurnId(), thinkingTurn); } - if (StringUtils.isNotEmpty(errMsg)) { - // TODO: Remove this legacy fallback after TBB is officially released. - // When the language server has not enabled token-based billing yet, fall back to the - // original main-branch 402 behavior: replace the message with a plan-driven fallback - // notice, switch to the fallback model, refresh quota, and replay the previous input. - CheckQuotaResult quotaStatus = this.serviceManager.getAuthStatusManager().getQuotaStatus(); - CopilotModel fallbackModel = null; - if (!quotaStatus.tokenBasedBillingEnabled() && value.getCode() == 402) { - CopilotPlan userPlan = quotaStatus.copilotPlan(); - fallbackModel = this.serviceManager.getModelService().getFallbackModel(); - String fallbackModelName = fallbackModel != null ? fallbackModel.getModelName() - : Messages.chat_noQuotaView_fallbackModel; - - if (MenuUtils.isCfiPlan(userPlan)) { - // Pro, Pro+ and Max message - errMsg = String.format(Messages.chat_noQuotaView_proProplusWarnMsg, fallbackModelName); - } else if (userPlan == CopilotPlan.business || userPlan == CopilotPlan.enterprise) { - // CE and CB message - errMsg = String.format(Messages.chat_noQuotaView_cbCeWarnMsg, fallbackModelName); - } + turnWidget.flushMessageBuffer(); + } + + String errMsg = value.getErrorMessage(); + if (StringUtils.isNotEmpty(errMsg)) { + errMsg = REQUEST_ID_SUFFIX.matcher(errMsg).replaceAll(StringUtils.EMPTY).trim(); + } + String reason = value.getErrorReason(); + if (StringUtils.isNotEmpty(reason) && reason.equals("model_not_supported")) { + // TODO: add enable button for better UX. + errMsg = Messages.chat_model_unsupported_message; + } + if (StringUtils.isNotEmpty(errMsg)) { + // TODO: Remove this legacy fallback after TBB is officially released. + // When the language server has not enabled token-based billing yet, fall back to the + // original main-branch 402 behavior: replace the message with a plan-driven fallback + // notice, switch to the fallback model, refresh quota, and replay the previous input. + CheckQuotaResult quotaStatus = this.serviceManager.getAuthStatusManager().getQuotaStatus(); + CopilotModel fallbackModel = null; + if (!quotaStatus.tokenBasedBillingEnabled() && value.getCode() == 402) { + CopilotPlan userPlan = quotaStatus.copilotPlan(); + fallbackModel = this.serviceManager.getModelService().getFallbackModel(); + String fallbackModelName = fallbackModel != null ? fallbackModel.getModelName() + : Messages.chat_noQuotaView_fallbackModel; + + if (MenuUtils.isCfiPlan(userPlan)) { + // Pro, Pro+ and Max message + errMsg = String.format(Messages.chat_noQuotaView_proProplusWarnMsg, fallbackModelName); + } else if (userPlan == CopilotPlan.business || userPlan == CopilotPlan.enterprise) { + // CE and CB message + errMsg = String.format(Messages.chat_noQuotaView_cbCeWarnMsg, fallbackModelName); } + } - renderWarnMessageWithUpgradePlanButton(errMsg, value.getCode(), value.getErrorModelProviderName()); - - // TODO: Remove this legacy fallback after TBB is officially released. - // Only replay the previous input when a fallback model is actually available; otherwise - // setFallBackModelAsActiveModel() is a no-op and re-posting the input with the same - // active model would just trigger the same 402 again. - if (!quotaStatus.tokenBasedBillingEnabled() && value.getCode() == 402 - && quotaStatus.copilotPlan() != CopilotPlan.free - && fallbackModel != null) { - // Detach the failed turn so the replayed response creates a new Copilot turn below the - // warning, instead of streaming into the same turn that just rendered the warn widget. - this.latestTurnWidget = null; - this.latestCopilotTurn = null; - - this.serviceManager.getModelService().setFallBackModelAsActiveModel(); - this.serviceManager.getAuthStatusManager().checkQuota(); - - String previousInput = this.serviceManager.getUserPreferenceService().getPreviousInput(StringUtils.EMPTY); - if (StringUtils.isNotEmpty(previousInput)) { - IEventBroker eventBroker = PlatformUI.getWorkbench().getService(IEventBroker.class); - Map properties = Map.of("previousInput", previousInput, "needCreateUserTurn", false); - eventBroker.post(CopilotEventConstants.TOPIC_CHAT_ON_SEND, properties); - } + renderWarnMessageWithUpgradePlanButton(errMsg, value.getCode(), value.getErrorModelProviderName()); + + // TODO: Remove this legacy fallback after TBB is officially released. + // Only replay the previous input when a fallback model is actually available; otherwise + // setFallBackModelAsActiveModel() is a no-op and re-posting the input with the same + // active model would just trigger the same 402 again. + if (!quotaStatus.tokenBasedBillingEnabled() && value.getCode() == 402 + && quotaStatus.copilotPlan() != CopilotPlan.free + && fallbackModel != null) { + // Detach the failed turn so the replayed response creates a new Copilot turn below the + // warning, instead of streaming into the same turn that just rendered the warn widget. + this.latestTurnWidget = null; + this.latestCopilotTurn = null; + + this.serviceManager.getModelService().setFallBackModelAsActiveModel(); + this.serviceManager.getAuthStatusManager().checkQuota(); + + String previousInput = this.serviceManager.getUserPreferenceService().getPreviousInput(StringUtils.EMPTY); + if (StringUtils.isNotEmpty(previousInput)) { + IEventBroker eventBroker = PlatformUI.getWorkbench().getService(IEventBroker.class); + Map properties = Map.of("previousInput", previousInput, "needCreateUserTurn", false); + eventBroker.post(CopilotEventConstants.TOPIC_CHAT_ON_SEND, properties); } } - }, this); + } } /** Returns the active thinking block ID last observed while processing this turn's progress. */ @@ -393,7 +425,8 @@ public void showCompactingStatusOnLatestCopilotTurn() { // the next round's reply and produce a single garbled line. latestCopilotTurn.flushMessageBuffer(); latestCopilotTurn.showCompactingStatus(); - refreshScrollerLayout(); + refreshLayoutFull(); + scrollToBottomIfAutoScroll(); } /** @@ -408,7 +441,8 @@ public void hideCompactingStatusOnLatestCopilotTurn() { // in case a cancel path did not receive an end progress event to flush it. latestCopilotTurn.flushMessageBuffer(); latestCopilotTurn.hideCompactingStatus(); - refreshScrollerLayout(); + refreshLayoutFull(); + scrollToBottomIfAutoScroll(); } /** @@ -420,7 +454,7 @@ public BaseTurnWidget getTurnWidget(String turnId) { private void renderWarnMessageWithUpgradePlanButton(String errorMessage, int code, String modelProviderName) { latestTurnWidget.createWarnDialog(errorMessage, code, modelProviderName); - refreshScrollerLayout(); + refreshLayoutFull(); scrollToLatestUserTurn(); } @@ -432,28 +466,78 @@ public void renderErrorMessage(String errorMessage) { this.errorWidget.dispose(); } this.errorWidget = new ErrorWidget(cmpContent, SWT.BOTTOM, errorMessage); - refreshScrollerLayout(); + refreshLayoutFull(); scrollToLatestUserTurn(); } /** - * Schedules a single async {@link #refreshScrollerLayout()} call so that multiple dispose/layout - * events that arrive in the same event-loop tick are coalesced into one pass. + * Coalesces a burst of calls into a single async pass on the UI thread. While a pass is already + * scheduled, further calls are dropped; {@code scheduled} is cleared right before {@code task} runs + * so work arriving during the task re-schedules a follow-up pass. This breaks synchronous + * re-entrancy (e.g. a layout pass writing the scroller min size re-fires {@code controlResized}) + * without a re-entrancy guard, while idempotent writes still converge to a fixed point. + * + * @param scheduled the per-task latch guarding against duplicate scheduling + * @param task the work to run once on the next UI-thread turn + */ + private void coalesceAsync(AtomicBoolean scheduled, Runnable task) { + if (scheduled.compareAndSet(false, true)) { + SwtUtils.invokeOnDisplayThreadAsync(() -> { + scheduled.set(false); + task.run(); + }, this); + } + } + + /** + * Full re-measure entry point for external callers. Layout only; scrolling is a separate concern + * handled by callers via {@link #scrollToBottomIfAutoScroll()}. + */ + public void refreshLayoutFull() { + refreshLayout(MeasureMode.FULL); + } + + /** + * Incremental re-measure of just the trailing (streaming) turns. Layout only; scrolling is handled + * separately by callers via {@link #scrollToBottomIfAutoScroll()}. */ - public void requestRefreshScrollerLayout() { - SwtUtils.invokeOnDisplayThreadAsync(() -> refreshScrollerLayout(), this); + private void refreshLayoutIncremental() { + refreshLayout(MeasureMode.INCREMENTAL); } /** - * Update the size of scrolled composite when there are content updates. + * Selects how many turns {@link #refreshLayout(MeasureMode)} re-measures. + */ + private enum MeasureMode { + /** Recursively re-measure every turn. O(n) in the number of turns. */ + FULL, + /** Only flush the trailing (mutating) turns; sealed turns keep cached sizes. O(1). */ + INCREMENTAL + } + + /** + * Re-measure the scroller and update its min size. + * + * @param mode {@link MeasureMode#FULL} recursively re-measures every turn; {@link + * MeasureMode#INCREMENTAL} only flushes the trailing (mutating) turns while sealed turns keep + * cached sizes, keeping the pass O(1). A width change always upgrades to a full measure because + * text re-wraps. */ - public void refreshScrollerLayout() { + private void refreshLayout(MeasureMode mode) { if (this.isDisposed()) { return; } - Rectangle clientArea = this.getClientArea(); - Point containerSize = cmpContent.computeSize(clientArea.width, SWT.DEFAULT); + int width = clientArea.width; + boolean fullMeasure = mode == MeasureMode.FULL || width != lastLayoutWidth; + lastLayoutWidth = width; + + if (!fullMeasure) { + // Only the trailing turns can grow/change during streaming. + flushTrailingTurnCaches(); + } + + Point containerSize = cmpContent.computeSize(width, SWT.DEFAULT, fullMeasure); // Use the default size as a fallback if (latestUserTurn == null) { @@ -461,22 +545,68 @@ public void refreshScrollerLayout() { return; } - Point userTurnSize = latestUserTurn.computeSize(SWT.DEFAULT, SWT.DEFAULT); - Point copilotTurnSize = latestCopilotTurn == null ? new Point(0, 0) - : latestCopilotTurn.computeSize(SWT.DEFAULT, SWT.DEFAULT); + // Measure at the actual column width, not SWT.DEFAULT: unconstrained width collapses + // soft-wrapped text to a single line and under-estimates the height. roundedHeight must match + // the laid-out height shouldAutoScrollToBottom() reads via getBounds(), or the padding branch + // below reserves phantom whitespace while auto-scroll fires into it (issue #259 flicker). + int userTurnHeight = latestUserTurn.computeSize(width, SWT.DEFAULT).y; + int copilotTurnHeight = latestCopilotTurn == null || latestCopilotTurn.isDisposed() ? 0 + : latestCopilotTurn.computeSize(width, SWT.DEFAULT).y; // Calculate the content height, so that the latest user turn is able to be put at the top of the client area. int contentHeight = 0; - int roundedHeight = userTurnSize.y + copilotTurnSize.y; + int roundedHeight = userTurnHeight + copilotTurnHeight; if (roundedHeight < clientArea.height) { contentHeight = clientArea.height + containerSize.y - roundedHeight; } else { contentHeight = containerSize.y; } - this.setMinHeight(contentHeight); - this.setMinWidth(containerSize.x); - this.layout(true, true); + // Only write min size when it changes: setMin* re-fires controlResized, so skipping no-op writes + // lets the coalesced async refresh converge to a fixed point. + if (this.getMinHeight() != contentHeight) { + this.setMinHeight(contentHeight); + } + if (this.getMinWidth() != containerSize.x) { + this.setMinWidth(containerSize.x); + } + // Incremental layout: only re-position the latest (growing) copilot turn instead of recursing + // into all past turns as conversations grow longer. + if (latestCopilotTurn != null && !latestCopilotTurn.isDisposed()) { + cmpContent.layout(new Control[] {latestCopilotTurn}); + } else { + cmpContent.layout(true, false); + } + this.layout(true, false); + } + + /** + * Scrolls the viewport to the bottom when auto-scroll is currently enabled. Kept separate from the + * layout pass so refresh and scroll stay independent concerns; callers invoke this after a refresh + * when they want the latest content to stay in view. + */ + public void scrollToBottomIfAutoScroll() { + if (shouldAutoScrollToBottom()) { + scrollToBottom(); + } + } + + /** + * Flushes the cached layout sizes of the trailing (mutating) turns so the next + * {@code computeSize(width, DEFAULT, false)} re-measures them while sealed historical turns stay + * cached, keeping the layout pass O(1) in the number of historical turns. + */ + private void flushTrailingTurnCaches() { + List dirty = new ArrayList<>(2); + if (latestUserTurn != null && !latestUserTurn.isDisposed()) { + dirty.add(latestUserTurn); + } + if (latestCopilotTurn != null && !latestCopilotTurn.isDisposed()) { + dirty.add(latestCopilotTurn); + } + if (!dirty.isEmpty()) { + cmpContent.layout(dirty.toArray(new Control[0])); + } } /** @@ -493,16 +623,34 @@ private boolean shouldAutoScrollToBottom() { } Rectangle clientArea = this.getClientArea(); - Point userTurnSize = latestUserTurn.computeSize(SWT.DEFAULT, SWT.DEFAULT); - Point copilotTurnSize = latestCopilotTurn == null ? new Point(0, 0) - : latestCopilotTurn.computeSize(SWT.DEFAULT, SWT.DEFAULT); - - int roundedHeight = userTurnSize.y + copilotTurnSize.y; + // Use the freshly laid-out bounds rather than computeSize(): the incremental streaming pass + // repositions the trailing turns but does not flush their computeSize cache, so computeSize + // would return a stale height and the auto-scroll trigger would fire seconds too late. + // getBounds() reflects the just-applied layout. + int roundedHeight = currentTurnLaidOutHeight(); // Only auto-scroll when content height exceeds the visible area return roundedHeight >= clientArea.height; } + /** + * Returns the height of the latest (streaming) turn from its applied layout bounds. Falls back to + * {@code computeSize} only when bounds are not yet available (before the first layout pass). + */ + private int currentTurnLaidOutHeight() { + int height = 0; + if (latestUserTurn != null && !latestUserTurn.isDisposed()) { + int userHeight = latestUserTurn.getBounds().height; + height += userHeight > 0 ? userHeight : latestUserTurn.computeSize(SWT.DEFAULT, SWT.DEFAULT).y; + } + if (latestCopilotTurn != null && !latestCopilotTurn.isDisposed()) { + int copilotHeight = latestCopilotTurn.getBounds().height; + height += copilotHeight > 0 ? copilotHeight + : latestCopilotTurn.computeSize(SWT.DEFAULT, SWT.DEFAULT).y; + } + return height; + } + /** * Scroll to the bottom. */ @@ -523,9 +671,7 @@ private void scrollToLatestUserTurn() { return; } - // Use async execution to ensure layout is computed before reading positions. - // Using sync execution would read positions before the layout is complete, - // resulting in incorrect scroll position (always scrolling to 0). + // Async so layout is computed before reading positions; reading synchronously scrolls to 0. SwtUtils.invokeOnDisplayThreadAsync(() -> { if (this.isDisposed() || latestUserTurn.isDisposed()) { return; @@ -537,6 +683,7 @@ private void scrollToLatestUserTurn() { @Override public void dispose() { + pendingEvents.clear(); super.dispose(); for (BaseTurnWidget turn : turns.values()) { turn.dispose(); diff --git a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/ChatView.java b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/ChatView.java index 89b350df..3cf6ff19 100644 --- a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/ChatView.java +++ b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/ChatView.java @@ -32,7 +32,6 @@ import org.eclipse.swt.widgets.Composite; import org.eclipse.swt.widgets.Control; import org.eclipse.swt.widgets.Display; -import org.eclipse.swt.widgets.ScrollBar; import org.eclipse.ui.IPartListener2; import org.eclipse.ui.IWorkbenchPartReference; import org.eclipse.ui.PlatformUI; @@ -1819,16 +1818,10 @@ public void scrollContentToBottom() { return; } - chatContentViewer.getDisplay().asyncExec(() -> { - if (chatContentViewer.isDisposed()) { - return; - } - chatContentViewer.refreshScrollerLayout(); - ScrollBar verticalBar = chatContentViewer.getVerticalBar(); - if (verticalBar != null && !verticalBar.isDisposed()) { - chatContentViewer.setOrigin(0, verticalBar.getMaximum()); - } - }); + SwtUtils.invokeOnDisplayThreadAsync(() -> { + chatContentViewer.refreshLayoutFull(); + chatContentViewer.scrollToBottomIfAutoScroll(); + }, chatContentViewer); } /** @@ -1847,8 +1840,11 @@ private void renderModelInfoInTurnWidget(String turnId, String modelName, double if (turnWidget instanceof CopilotTurnWidget copilotWidget) { copilotWidget.renderModelInfo(modelName, billingMultiplier, reasoningEffort); - // Refresh the scroller layout to ensure the footer is visible - SwtUtils.invokeOnDisplayThreadAsync(() -> this.chatContentViewer.refreshScrollerLayout(), this.chatContentViewer); + // Refresh the scroller layout to ensure the footer is visible. + SwtUtils.invokeOnDisplayThreadAsync(() -> { + this.chatContentViewer.refreshLayoutFull(); + this.chatContentViewer.scrollToBottomIfAutoScroll(); + }, this.chatContentViewer); } } diff --git a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/ThinkingBlock.java b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/ThinkingBlock.java index 8e974b9e..e273986f 100644 --- a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/ThinkingBlock.java +++ b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/ThinkingBlock.java @@ -315,7 +315,6 @@ private void refreshBody() { body.requestLayout(); updateScrollerDuringStreaming(); - refreshEnclosingScroller(); } /** Resize the scroller to fit content (up to max height) and auto-scroll to bottom if enabled. */ @@ -499,7 +498,7 @@ private void refreshEnclosingScroller() { Composite p = getParent(); while (p != null && !p.isDisposed()) { if (p instanceof ChatContentViewer) { - ((ChatContentViewer) p).refreshScrollerLayout(); + ((ChatContentViewer) p).refreshLayoutFull(); return; } p = p.getParent(); diff --git a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/services/AgentToolService.java b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/services/AgentToolService.java index be236d9c..c7a013c2 100644 --- a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/services/AgentToolService.java +++ b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/services/AgentToolService.java @@ -294,7 +294,7 @@ public CompletableFuture onToolConfirmation SwtUtils.invokeOnDisplayThread(() -> { ref.set(activeTurnWidget.requestToolExecutionConfirmation( content, params.getInput())); - boundChatView.getChatContentViewer().refreshScrollerLayout(); + boundChatView.getChatContentViewer().refreshLayoutFull(); }); CompletableFuture future = ref.get(); From 73e72b75c4fe2bd1227d430f09f2ad5786083264 Mon Sep 17 00:00:00 2001 From: xinyi-gong Date: Thu, 2 Jul 2026 11:20:58 +0800 Subject: [PATCH 10/14] feat: Auto-approve reads of Copilot customization files (#331) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat: Auto-approve reads of Copilot customization files Reading a Copilot customization file (skill, instruction, prompt, or agent) is normal agent operation, but these reads previously triggered a confirmation dialog — including workspace instructions caught by the default deny rule and user-global files outside the workspace. Recognize these reads precisely from the language server, which is the source of truth for where the files live (including user-configured and global locations). New copilot/customSkill|customPrompt|customInstruction| customAgent/list requests report each file's on-disk URI; a read is auto-approved when it exactly matches a reported file or sits inside a reported skill folder (covering SKILL.md and its helper files). Editing is never auto-approved — only reads. - Add CustomizationFileService, refreshed per type on the matching custom*/didChange notification, exposing an immutable path snapshot. - Hook FileOperationConfirmationHandler to auto-approve FILE_READ of a recognized customization file before the outside-workspace check. - Add FileUtils.isPathWithin for skill-folder containment. Tests: FileUtilsTests (isPathWithin), FileOperationConfirmationHandlerTests (isCustomizationRead), and TC-010/TC-011 in the auto-approve test plan. * rename refresh functions and merge workspaceFoldersParams * refactor: Decouple customization file service via event bus * rename refreshAll to refreshAllAsync --- .../service/CustomizationFileService.java | 144 ++++++++++++++++++ .../chat/service/IChatServiceManager.java | 5 + .../service/ICustomizationFileService.java | 39 +++++ .../core/events/CopilotEventConstants.java | 4 +- .../core/lsp/CopilotLanguageClient.java | 31 +++- .../core/lsp/CopilotLanguageServer.java | 37 ++++- .../lsp/CopilotLanguageServerConnection.java | 45 +++++- .../protocol/ConversationTemplatesParams.java | 21 --- .../lsp/protocol/CustomizationFileInfo.java | 16 ++ .../lsp/protocol/WorkspaceFoldersParams.java | 20 +++ .../file-operation-auto-approve.md | 73 +++++++++ ...FileOperationConfirmationHandlerTests.java | 90 +++++++++++ .../FileOperationConfirmationHandler.java | 47 ++++++ .../chat/services/ChatCompletionService.java | 9 +- .../ui/chat/services/ChatServiceManager.java | 10 ++ 15 files changed, 557 insertions(+), 34 deletions(-) create mode 100644 com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/chat/service/CustomizationFileService.java create mode 100644 com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/chat/service/ICustomizationFileService.java delete mode 100644 com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/lsp/protocol/ConversationTemplatesParams.java create mode 100644 com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/lsp/protocol/CustomizationFileInfo.java create mode 100644 com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/lsp/protocol/WorkspaceFoldersParams.java diff --git a/com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/chat/service/CustomizationFileService.java b/com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/chat/service/CustomizationFileService.java new file mode 100644 index 00000000..c94c14f7 --- /dev/null +++ b/com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/chat/service/CustomizationFileService.java @@ -0,0 +1,144 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + +package com.microsoft.copilot.eclipse.core.chat.service; + +import java.nio.file.Path; +import java.util.ArrayList; +import java.util.HashSet; +import java.util.List; +import java.util.Set; +import java.util.concurrent.CompletableFuture; + +import org.eclipse.e4.core.contexts.EclipseContextFactory; +import org.eclipse.e4.core.services.events.IEventBroker; +import org.eclipse.lsp4j.WorkspaceFolder; +import org.osgi.framework.FrameworkUtil; +import org.osgi.service.event.EventHandler; + +import com.microsoft.copilot.eclipse.core.CopilotCore; +import com.microsoft.copilot.eclipse.core.events.CopilotEventConstants; +import com.microsoft.copilot.eclipse.core.lsp.CopilotLanguageServerConnection; +import com.microsoft.copilot.eclipse.core.lsp.protocol.CustomizationFileInfo; +import com.microsoft.copilot.eclipse.core.utils.FileUtils; +import com.microsoft.copilot.eclipse.core.utils.WorkspaceUtils; + +/** + * Default {@link ICustomizationFileService} backed by the language server's {@code copilot/custom*} + * list requests. Each customization type is fetched and snapshotted independently. + */ +public class CustomizationFileService implements ICustomizationFileService { + + private final CopilotLanguageServerConnection lsConnection; + private final IEventBroker eventBroker; + private final EventHandler customizationFilesChangedHandler; + + private volatile Set promptFiles = Set.of(); + private volatile Set instructionFiles = Set.of(); + private volatile Set agentFiles = Set.of(); + private volatile Set skillFolders = Set.of(); + + private volatile Set customizationFiles = Set.of(); + + /** + * Creates the service and subscribes to customization-file change events. + * + * @param lsConnection the language server connection used to issue the list requests + */ + public CustomizationFileService(CopilotLanguageServerConnection lsConnection) { + this.lsConnection = lsConnection; + this.eventBroker = EclipseContextFactory + .getServiceContext(FrameworkUtil.getBundle(getClass()).getBundleContext()).get(IEventBroker.class); + this.customizationFilesChangedHandler = event -> { + if (event.getProperty(IEventBroker.DATA) instanceof CustomizationType type) { + CompletableFuture.runAsync(() -> refreshType(type, WorkspaceUtils.listWorkspaceFolders())); + } + }; + if (eventBroker != null) { + eventBroker.subscribe(CopilotEventConstants.TOPIC_CHAT_DID_CHANGE_CUSTOMIZATION_FILES, + customizationFilesChangedHandler); + } + } + + /** Unsubscribes from customization-file change events. */ + public void dispose() { + if (eventBroker != null) { + eventBroker.unsubscribe(customizationFilesChangedHandler); + } + } + + @Override + public Set getCustomizationFiles() { + return customizationFiles; + } + + @Override + public Set getSkillFolders() { + return skillFolders; + } + + @Override + public void refreshAllAsync() { + CompletableFuture.runAsync(() -> { + List workspaceFolders = WorkspaceUtils.listWorkspaceFolders(); + for (CustomizationType type : CustomizationType.values()) { + refreshType(type, workspaceFolders); + } + }); + } + + private void refreshType(CustomizationType type, List workspaceFolders) { + switch (type) { + case SKILL -> toPaths(lsConnection.listCustomSkills(workspaceFolders)).thenAccept(paths -> { + Set folders = new HashSet<>(); + for (Path skillFile : paths) { + Path parent = skillFile.getParent(); + if (parent != null) { + folders.add(parent); + } + } + this.skillFolders = Set.copyOf(folders); + }); + case PROMPT -> toPaths(lsConnection.listCustomPrompts(workspaceFolders)).thenAccept(paths -> { + this.promptFiles = Set.copyOf(paths); + rebuildCustomizationFiles(); + }); + case INSTRUCTION -> toPaths(lsConnection.listCustomInstructions(workspaceFolders)).thenAccept(paths -> { + this.instructionFiles = Set.copyOf(paths); + rebuildCustomizationFiles(); + }); + case AGENT -> toPaths(lsConnection.listCustomAgents(workspaceFolders)).thenAccept(paths -> { + this.agentFiles = Set.copyOf(paths); + rebuildCustomizationFiles(); + }); + default -> { + // No other customization types. + } + } + } + + private synchronized void rebuildCustomizationFiles() { + Set all = new HashSet<>(promptFiles); + all.addAll(instructionFiles); + all.addAll(agentFiles); + this.customizationFiles = Set.copyOf(all); + } + + private CompletableFuture> toPaths(CompletableFuture future) { + return future.thenApply(infos -> { + List paths = new ArrayList<>(); + if (infos != null) { + for (CustomizationFileInfo info : infos) { + Path path = FileUtils.getLocalFilePath(info.uri()); + if (path != null) { + paths.add(path.toAbsolutePath().normalize()); + } + } + } + return paths; + }).exceptionally(ex -> { + CopilotCore.LOGGER.error("Failed to list customization files", ex); + return List.of(); + }); + } +} diff --git a/com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/chat/service/IChatServiceManager.java b/com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/chat/service/IChatServiceManager.java index 65aa911e..d6b0784f 100644 --- a/com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/chat/service/IChatServiceManager.java +++ b/com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/chat/service/IChatServiceManager.java @@ -17,4 +17,9 @@ public interface IChatServiceManager { * Get the MCP config service. */ IMcpConfigService getMcpConfigService(); + + /** + * Get the customization file service tracking skill/prompt/instruction/agent file locations. + */ + ICustomizationFileService getCustomizationFileService(); } diff --git a/com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/chat/service/ICustomizationFileService.java b/com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/chat/service/ICustomizationFileService.java new file mode 100644 index 00000000..736df418 --- /dev/null +++ b/com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/chat/service/ICustomizationFileService.java @@ -0,0 +1,39 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + +package com.microsoft.copilot.eclipse.core.chat.service; + +import java.nio.file.Path; +import java.util.Set; + +/** + * Maintains an up-to-date view of the Copilot customization files (skills, prompts, instructions, + * and agents) reported by the language server, keyed by their on-disk location. + * + */ +public interface ICustomizationFileService { + + /** The customization kinds, each backed by a distinct language-server list request. */ + enum CustomizationType { + SKILL, PROMPT, INSTRUCTION, AGENT + } + + /** + * Returns the absolute paths of single-file customization files (prompts, instructions, agents). + * The returned set is an immutable snapshot. + */ + Set getCustomizationFiles(); + + /** + * Returns the absolute paths of skill folders (the directory containing each {@code SKILL.md}). + * A read of any file within one of these folders is a skill read. The returned set is an immutable + * snapshot. + */ + Set getSkillFolders(); + + /** + * Refreshes every customization type from the language server. Used for the initial load. + * Fire-and-forget; the work completes asynchronously off the caller's thread. + */ + void refreshAllAsync(); +} diff --git a/com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/events/CopilotEventConstants.java b/com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/events/CopilotEventConstants.java index 61637b76..1a2d3c9e 100644 --- a/com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/events/CopilotEventConstants.java +++ b/com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/events/CopilotEventConstants.java @@ -183,8 +183,8 @@ public class CopilotEventConstants { public static final String TOPIC_QUOTA_WARNING = TOPIC_QUOTA + "WARNING"; /** - * Event when custom prompts, skills, agents, or instructions change on the language server. Clients should re-fetch - * conversation templates on receipt. + * Event when custom prompts, skills, agents, or instructions change on the language server. The event data is the + * {@link com.microsoft.copilot.eclipse.core.chat.service.ICustomizationFileService.CustomizationType} that changed. */ public static final String TOPIC_CHAT_DID_CHANGE_CUSTOMIZATION_FILES = TOPIC_CHAT + "DID_CHANGE_CUSTOMIZATION_FILES"; diff --git a/com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/lsp/CopilotLanguageClient.java b/com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/lsp/CopilotLanguageClient.java index da7aaee9..cf0e81ad 100644 --- a/com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/lsp/CopilotLanguageClient.java +++ b/com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/lsp/CopilotLanguageClient.java @@ -37,6 +37,7 @@ import com.microsoft.copilot.eclipse.core.CopilotCore; import com.microsoft.copilot.eclipse.core.FeatureFlags; import com.microsoft.copilot.eclipse.core.chat.service.IChatServiceManager; +import com.microsoft.copilot.eclipse.core.chat.service.ICustomizationFileService.CustomizationType; import com.microsoft.copilot.eclipse.core.chat.service.IMcpConfigService; import com.microsoft.copilot.eclipse.core.chat.service.IReferencedFileService; import com.microsoft.copilot.eclipse.core.events.CopilotEventConstants; @@ -264,24 +265,42 @@ public void onRateLimitWarning(RateLimitWarningParams params) { } /** - * Notify when custom skills change (global or workspace). Signal-only; clients re-fetch templates. + * Notify when custom skills change (global or workspace). */ @JsonNotification("copilot/customSkill/didChange") public void onDidChangeCustomSkill(Object params) { - notifyCustomizationFilesChanged(); + postCustomizationFilesChanged(CustomizationType.SKILL); } /** - * Notify when custom prompts change (global or workspace). Signal-only; clients re-fetch templates. + * Notify when custom prompts change (global or workspace). */ @JsonNotification("copilot/customPrompt/didChange") public void onDidChangeCustomPrompt(Object params) { - notifyCustomizationFilesChanged(); + postCustomizationFilesChanged(CustomizationType.PROMPT); } - private void notifyCustomizationFilesChanged() { + /** + * Notify when custom instructions change (global or workspace). + */ + @JsonNotification("copilot/customInstruction/didChange") + public void onDidChangeCustomInstruction(Object params) { + postCustomizationFilesChanged(CustomizationType.INSTRUCTION); + } + + /** + * Notify when custom agents change (global or workspace). + */ + @JsonNotification("copilot/customAgent/didChange") + public void onDidChangeCustomAgent(Object params) { + postCustomizationFilesChanged(CustomizationType.AGENT); + } + + // Broadcasts the change on the event bus; subscribers (the customization-file service and the + // slash-command service) react without this class depending on them directly. + private void postCustomizationFilesChanged(CustomizationType type) { if (eventBroker != null) { - eventBroker.post(CopilotEventConstants.TOPIC_CHAT_DID_CHANGE_CUSTOMIZATION_FILES, null); + eventBroker.post(CopilotEventConstants.TOPIC_CHAT_DID_CHANGE_CUSTOMIZATION_FILES, type); } } 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 8f74d5dd..f238d933 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 @@ -29,10 +29,10 @@ import com.microsoft.copilot.eclipse.core.lsp.protocol.ConversationMode; import com.microsoft.copilot.eclipse.core.lsp.protocol.ConversationModesParams; import com.microsoft.copilot.eclipse.core.lsp.protocol.ConversationTemplate; -import com.microsoft.copilot.eclipse.core.lsp.protocol.ConversationTemplatesParams; import com.microsoft.copilot.eclipse.core.lsp.protocol.ConversationTurnParams; import com.microsoft.copilot.eclipse.core.lsp.protocol.CopilotModel; import com.microsoft.copilot.eclipse.core.lsp.protocol.CopilotStatusResult; +import com.microsoft.copilot.eclipse.core.lsp.protocol.CustomizationFileInfo; import com.microsoft.copilot.eclipse.core.lsp.protocol.DidShowInlineEditParams; import com.microsoft.copilot.eclipse.core.lsp.protocol.GenerateThinkingTitleParams; import com.microsoft.copilot.eclipse.core.lsp.protocol.GenerateThinkingTitleResponse; @@ -51,6 +51,7 @@ import com.microsoft.copilot.eclipse.core.lsp.protocol.TelemetryExceptionParams; import com.microsoft.copilot.eclipse.core.lsp.protocol.UpdateConversationToolsStatusParams; 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.ByokListApiKeyResponse; import com.microsoft.copilot.eclipse.core.lsp.protocol.byok.ByokListModelParams; @@ -146,7 +147,39 @@ public interface CopilotLanguageServer extends LanguageServer { * @param params includes workspace folders for discovering workspace-specific prompt files and skills */ @JsonRequest("conversation/templates") - CompletableFuture listTemplates(ConversationTemplatesParams params); + CompletableFuture listTemplates(WorkspaceFoldersParams params); + + /** + * List custom skill files (each carries its on-disk {@code uri}). + * + * @param params includes the workspace folders to scan + */ + @JsonRequest("copilot/customSkill/list") + CompletableFuture listCustomSkills(WorkspaceFoldersParams params); + + /** + * List custom prompt files (each carries its on-disk {@code uri}). + * + * @param params includes the workspace folders to scan + */ + @JsonRequest("copilot/customPrompt/list") + CompletableFuture listCustomPrompts(WorkspaceFoldersParams params); + + /** + * List custom instruction files (each carries its on-disk {@code uri}). + * + * @param params includes the workspace folders to scan + */ + @JsonRequest("copilot/customInstruction/list") + CompletableFuture listCustomInstructions(WorkspaceFoldersParams params); + + /** + * List custom agent files (each carries its on-disk {@code uri}). + * + * @param params includes the workspace folders to scan + */ + @JsonRequest("copilot/customAgent/list") + CompletableFuture listCustomAgents(WorkspaceFoldersParams params); /** * List conversation modes. 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 de116abb..1c853da5 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 @@ -46,10 +46,10 @@ import com.microsoft.copilot.eclipse.core.lsp.protocol.ConversationMode; import com.microsoft.copilot.eclipse.core.lsp.protocol.ConversationModesParams; import com.microsoft.copilot.eclipse.core.lsp.protocol.ConversationTemplate; -import com.microsoft.copilot.eclipse.core.lsp.protocol.ConversationTemplatesParams; import com.microsoft.copilot.eclipse.core.lsp.protocol.ConversationTurnParams; import com.microsoft.copilot.eclipse.core.lsp.protocol.CopilotModel; import com.microsoft.copilot.eclipse.core.lsp.protocol.CopilotStatusResult; +import com.microsoft.copilot.eclipse.core.lsp.protocol.CustomizationFileInfo; import com.microsoft.copilot.eclipse.core.lsp.protocol.DidChangeCopilotWatchedFilesParams; import com.microsoft.copilot.eclipse.core.lsp.protocol.DidShowInlineEditParams; import com.microsoft.copilot.eclipse.core.lsp.protocol.GenerateThinkingTitleParams; @@ -72,6 +72,7 @@ import com.microsoft.copilot.eclipse.core.lsp.protocol.Turn; import com.microsoft.copilot.eclipse.core.lsp.protocol.UpdateConversationToolsStatusParams; 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.ByokListApiKeyResponse; import com.microsoft.copilot.eclipse.core.lsp.protocol.byok.ByokListModelParams; @@ -374,11 +375,51 @@ public CompletableFuture addConversationTurn(String workDoneToke */ public CompletableFuture listConversationTemplates(List workspaceFolders) { Function> fn = server -> { - return ((CopilotLanguageServer) server).listTemplates(new ConversationTemplatesParams(workspaceFolders)); + return ((CopilotLanguageServer) server).listTemplates(new WorkspaceFoldersParams(workspaceFolders)); }; return this.languageServerWrapper.execute(fn); } + /** + * List custom skill files, each carrying its on-disk {@code uri}. + * + * @param workspaceFolders the workspace folders to scan + */ + public CompletableFuture listCustomSkills(List workspaceFolders) { + return this.languageServerWrapper.execute(server -> + ((CopilotLanguageServer) server).listCustomSkills(new WorkspaceFoldersParams(workspaceFolders))); + } + + /** + * List custom prompt files, each carrying its on-disk {@code uri}. + * + * @param workspaceFolders the workspace folders to scan + */ + public CompletableFuture listCustomPrompts(List workspaceFolders) { + return this.languageServerWrapper.execute(server -> + ((CopilotLanguageServer) server).listCustomPrompts(new WorkspaceFoldersParams(workspaceFolders))); + } + + /** + * List custom instruction files, each carrying its on-disk {@code uri}. + * + * @param workspaceFolders the workspace folders to scan + */ + public CompletableFuture listCustomInstructions(List workspaceFolders) { + return this.languageServerWrapper.execute(server -> + ((CopilotLanguageServer) server).listCustomInstructions(new WorkspaceFoldersParams(workspaceFolders))); + } + + /** + * List custom agent files, each carrying its on-disk {@code uri}. + * + * @param workspaceFolders the workspace folders to scan + */ + public CompletableFuture listCustomAgents(List workspaceFolders) { + return this.languageServerWrapper.execute(server -> + ((CopilotLanguageServer) server).listCustomAgents(new WorkspaceFoldersParams(workspaceFolders))); + } + /** * List the conversation modes. */ diff --git a/com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/lsp/protocol/ConversationTemplatesParams.java b/com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/lsp/protocol/ConversationTemplatesParams.java deleted file mode 100644 index 6b384b94..00000000 --- a/com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/lsp/protocol/ConversationTemplatesParams.java +++ /dev/null @@ -1,21 +0,0 @@ -// Copyright (c) Microsoft Corporation. -// Licensed under the MIT license. - -package com.microsoft.copilot.eclipse.core.lsp.protocol; - -import java.util.Collections; -import java.util.List; - -import org.eclipse.lsp4j.WorkspaceFolder; - -/** - * Parameters for the {@code conversation/templates} request. - * - * @param workspaceFolders the workspace folders used to discover workspace-specific prompt files and skills - */ -public record ConversationTemplatesParams(List workspaceFolders) { - /** Compact constructor that defaults {@code null} workspace folders to an empty list. */ - public ConversationTemplatesParams { - workspaceFolders = workspaceFolders != null ? workspaceFolders : Collections.emptyList(); - } -} diff --git a/com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/lsp/protocol/CustomizationFileInfo.java b/com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/lsp/protocol/CustomizationFileInfo.java new file mode 100644 index 00000000..c164b1dd --- /dev/null +++ b/com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/lsp/protocol/CustomizationFileInfo.java @@ -0,0 +1,16 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + +package com.microsoft.copilot.eclipse.core.lsp.protocol; + +/** + * Metadata for a Copilot customization file (skill, prompt, instruction, or agent) as returned by + * the language server's customization list requests. + * + * @param id the stable identifier, when provided by the language server + * @param name the user-facing name + * @param uri the on-disk file URI, or {@code null} for entries without a backing file + * @param storage the storage scope reported by the language server (for example {@code local} or {@code user}) + */ +public record CustomizationFileInfo(String id, String name, String uri, String storage) { +} diff --git a/com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/lsp/protocol/WorkspaceFoldersParams.java b/com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/lsp/protocol/WorkspaceFoldersParams.java new file mode 100644 index 00000000..5f3e3574 --- /dev/null +++ b/com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/lsp/protocol/WorkspaceFoldersParams.java @@ -0,0 +1,20 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + +package com.microsoft.copilot.eclipse.core.lsp.protocol; + +import java.util.List; + +import org.eclipse.lsp4j.WorkspaceFolder; + +/** + * Generic parameters for language-server requests scoped to a set of workspace folders. + * + * @param workspaceFolders the workspace folders to scan + */ +public record WorkspaceFoldersParams(List workspaceFolders) { + /** Compact constructor that defensively copies the folders, defaulting {@code null} to empty. */ + public WorkspaceFoldersParams { + workspaceFolders = workspaceFolders != null ? List.copyOf(workspaceFolders) : List.of(); + } +} diff --git a/com.microsoft.copilot.eclipse.swtbot.test/test-plans/file-operation-auto-approve/file-operation-auto-approve.md b/com.microsoft.copilot.eclipse.swtbot.test/test-plans/file-operation-auto-approve/file-operation-auto-approve.md index 2a76f5f9..e5ddf4e8 100644 --- a/com.microsoft.copilot.eclipse.swtbot.test/test-plans/file-operation-auto-approve/file-operation-auto-approve.md +++ b/com.microsoft.copilot.eclipse.swtbot.test/test-plans/file-operation-auto-approve/file-operation-auto-approve.md @@ -340,3 +340,76 @@ approval #### 📸 Key Screenshots - [ ] Preference page after reset — only defaults. - [ ] `.java` file shows confirmation dialog post-reset. + +--- + +## 9. Customization file reads (skills, instructions, prompts, agents) + +Reading a Copilot customization file is always auto-approved, even with +"Auto approve file operations not covered by rules" **unchecked** and even +when a deny rule would otherwise match (e.g. the default `.github/instructions/*` +rule). The exemption is read-only and covers both workspace and user-global +(`~/.copilot`, `~/.claude`, `~/.agents`) locations. Recognized files: +`//**` (the whole skill folder, including helper files), +`*.instructions.md`, `*.prompt.md`, `*.agent.md`, and the fixed names +`copilot-instructions.md`, `git-commit-instructions.md`, `AGENTS.md`, `CLAUDE.md`. + +### TC-010: Customization reads auto-approve; edits and ordinary files still prompt + +**Type:** `Happy Path` +**Priority:** `P0` + +#### Preconditions +- "Auto approve file operations not covered by rules" is **unchecked**. +- The project contains `.github/skills/demo/SKILL.md` (whose body also tells + Copilot to read a sibling `notes.md`), `.github/instructions/coding.instructions.md`, + and `.github/copilot-instructions.md`. + +#### Steps +1. In Agent Mode, invoke the demo skill (or ask Copilot to read + `.github/skills/demo/SKILL.md`). +2. Observe **no confirmation** for `SKILL.md` **or** the referenced `notes.md`. +3. Type: `read the project instructions and summarize them`. +4. Observe **no confirmation** for `coding.instructions.md` — even though the + default `.github/instructions/*` deny rule matches. +5. Type: `read .github/copilot-instructions.md`. +6. Observe **no confirmation**. +7. Type: `add a comment line to .github/skills/demo/SKILL.md`. +8. Observe **confirmation dialog appears** — editing is not exempt. +9. Type: `read src/demo/App.java`. +10. Observe **confirmation dialog appears** — ordinary files are unaffected. + +#### Expected Result +- Reads of skill files (and their helpers), instruction/prompt/agent files, and + the fixed well-known files auto-approve with no dialog. +- Editing a customization file still prompts. +- Ordinary (non-customization) reads still prompt. + +#### 📸 Key Screenshots +- [ ] Skill `SKILL.md` + `notes.md` read with no confirmation. +- [ ] Instruction file read with no confirmation despite the deny rule. +- [ ] Edit to `SKILL.md` shows a confirmation dialog. + +--- + +### TC-011: User-global customization files auto-approve + +**Type:** `Edge Case` +**Priority:** `P1` + +#### Preconditions +- "Auto approve file operations not covered by rules" is **unchecked**. +- A user-global skill exists, e.g. `~/.copilot/skills/demo-global/SKILL.md`. + +#### Steps +1. In Agent Mode, invoke the global demo skill (or ask Copilot to read its + `SKILL.md`). +2. Observe **no confirmation** — global customization files are exempt even + though they live outside the workspace (an ordinary outside-workspace file + would still prompt, per TC-006). + +#### Expected Result +- User-global customization reads auto-approve without a dialog. + +#### 📸 Key Screenshots +- [ ] Global skill read with no confirmation. diff --git a/com.microsoft.copilot.eclipse.ui.test/src/com/microsoft/copilot/eclipse/ui/chat/confirmation/FileOperationConfirmationHandlerTests.java b/com.microsoft.copilot.eclipse.ui.test/src/com/microsoft/copilot/eclipse/ui/chat/confirmation/FileOperationConfirmationHandlerTests.java index 5d9b9573..ba60a62e 100644 --- a/com.microsoft.copilot.eclipse.ui.test/src/com/microsoft/copilot/eclipse/ui/chat/confirmation/FileOperationConfirmationHandlerTests.java +++ b/com.microsoft.copilot.eclipse.ui.test/src/com/microsoft/copilot/eclipse/ui/chat/confirmation/FileOperationConfirmationHandlerTests.java @@ -9,15 +9,18 @@ import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.Mockito.when; +import java.nio.file.Path; import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Set; import com.google.gson.Gson; import org.eclipse.jface.preference.IPreferenceStore; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; +import org.junit.jupiter.api.io.TempDir; import org.mockito.Mock; import org.mockito.junit.jupiter.MockitoExtension; import org.mockito.junit.jupiter.MockitoSettings; @@ -46,6 +49,9 @@ class FileOperationConfirmationHandlerTests { private AttachedFileRegistry attachedFileRegistry; private FileOperationConfirmationHandler handler; + @TempDir + private Path customizationBase; + @BeforeEach void setUp() { attachedFileRegistry = new AttachedFileRegistry(); @@ -677,6 +683,90 @@ void evaluate_priorityOrder_sessionFolderBeatsOutsideWorkspace() { assertTrue(evaluate(params, CONV_ID).isAutoApproved()); } + // --- customization file read recognition (isCustomizationRead) --- + + private Set customizationFiles() { + return Set.of( + customizationBase.resolve(".github/prompts/example.prompt.md"), + customizationBase.resolve(".github/instructions/coding.instructions.md"), + customizationBase.resolve(".github/agents/my.agent.md")); + } + + private Set skillFolders() { + return Set.of(customizationBase.resolve(".github/skills/demo")); + } + + @Test + void isCustomizationRead_promptFileExactMatch_isTrue() { + Path prompt = customizationBase.resolve(".github/prompts/example.prompt.md"); + assertTrue(FileOperationConfirmationHandler.isCustomizationRead( + prompt, customizationFiles(), skillFolders())); + } + + @Test + void isCustomizationRead_instructionFileExactMatch_isTrue() { + Path instruction = customizationBase.resolve(".github/instructions/coding.instructions.md"); + assertTrue(FileOperationConfirmationHandler.isCustomizationRead( + instruction, customizationFiles(), skillFolders())); + } + + @Test + void isCustomizationRead_agentFileExactMatch_isTrue() { + Path agent = customizationBase.resolve(".github/agents/my.agent.md"); + assertTrue(FileOperationConfirmationHandler.isCustomizationRead( + agent, customizationFiles(), skillFolders())); + } + + @Test + void isCustomizationRead_skillMarkdownInsideFolder_isTrue() { + Path skill = customizationBase.resolve(".github/skills/demo/SKILL.md"); + assertTrue(FileOperationConfirmationHandler.isCustomizationRead( + skill, customizationFiles(), skillFolders())); + } + + @Test + void isCustomizationRead_skillHelperFileInsideFolder_isTrue() { + Path helper = customizationBase.resolve(".github/skills/demo/notes.md"); + assertTrue(FileOperationConfirmationHandler.isCustomizationRead( + helper, customizationFiles(), skillFolders())); + } + + @Test + void isCustomizationRead_nestedSkillHelperFile_isTrue() { + Path helper = customizationBase.resolve(".github/skills/demo/scripts/run.py"); + assertTrue(FileOperationConfirmationHandler.isCustomizationRead( + helper, customizationFiles(), skillFolders())); + } + + @Test + void isCustomizationRead_unrelatedFile_isFalse() { + Path other = customizationBase.resolve("src/Main.java"); + assertFalse(FileOperationConfirmationHandler.isCustomizationRead( + other, customizationFiles(), skillFolders())); + } + + @Test + void isCustomizationRead_promptOutsideReportedSet_isFalse() { + // A *.prompt.md the language server did not report must not be auto-approved. + Path loose = customizationBase.resolve("docs/other.prompt.md"); + assertFalse(FileOperationConfirmationHandler.isCustomizationRead( + loose, customizationFiles(), skillFolders())); + } + + @Test + void isCustomizationRead_fileInUnreportedSkillFolder_isFalse() { + Path other = customizationBase.resolve(".github/skills/other/SKILL.md"); + assertFalse(FileOperationConfirmationHandler.isCustomizationRead( + other, customizationFiles(), skillFolders())); + } + + @Test + void isCustomizationRead_emptyInputs_isFalse() { + Path prompt = customizationBase.resolve(".github/prompts/example.prompt.md"); + assertFalse(FileOperationConfirmationHandler.isCustomizationRead( + prompt, Set.of(), Set.of())); + } + // --- Helpers --- private void stubRules(List rules) { diff --git a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/confirmation/FileOperationConfirmationHandler.java b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/confirmation/FileOperationConfirmationHandler.java index 313c56c2..ba861ed4 100644 --- a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/confirmation/FileOperationConfirmationHandler.java +++ b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/confirmation/FileOperationConfirmationHandler.java @@ -28,8 +28,11 @@ import com.microsoft.copilot.eclipse.core.chat.ConfirmationContent; import com.microsoft.copilot.eclipse.core.chat.ConfirmationResult; import com.microsoft.copilot.eclipse.core.chat.FileOperationAutoApproveRule; +import com.microsoft.copilot.eclipse.core.chat.service.IChatServiceManager; +import com.microsoft.copilot.eclipse.core.chat.service.ICustomizationFileService; import com.microsoft.copilot.eclipse.core.lsp.protocol.InvokeClientToolConfirmationParams; import com.microsoft.copilot.eclipse.core.lsp.protocol.ToolMetadata; +import com.microsoft.copilot.eclipse.core.utils.FileUtils; import com.microsoft.copilot.eclipse.ui.chat.Messages; /** @@ -171,6 +174,17 @@ private ConfirmationResult evaluateAutoApprovalEnabled( } } + // Auto-approve reads of Copilot customization files (skills, instructions, prompts, agents) + // discovered by the language server. Reading them is normal agent operation; edits still prompt. + if (isFileRead(params)) { + Path localPath = FileUtils.getLocalFilePath(filePath); + ICustomizationFileService service = getCustomizationFileService(); + if (localPath != null && service != null && isCustomizationRead( + localPath, service.getCustomizationFiles(), service.getSkillFolders())) { + return ConfirmationResult.AUTO_APPROVED; + } + } + // Files outside workspace always require confirmation if (isOutsideWorkspace(params)) { return ConfirmationResult.needsConfirmation(buildContent(params)); @@ -338,6 +352,39 @@ private boolean isOutsideWorkspace(InvokeClientToolConfirmationParams params) { return false; } + private static boolean isFileRead(InvokeClientToolConfirmationParams params) { + return FileToolType.fromValue(ConfirmationHandler.extractToolType(params)) + == FileToolType.FILE_READ; + } + + private static ICustomizationFileService getCustomizationFileService() { + IChatServiceManager chatServiceManager = CopilotCore.getPlugin().getChatServiceManager(); + return chatServiceManager == null ? null : chatServiceManager.getCustomizationFileService(); + } + + /** + * Returns whether reading {@code file} is a customization-file read that is safe to auto-approve. + * A read matches when the file exactly equals a language-server-reported customization file, or + * sits inside a reported skill folder (covering {@code SKILL.md} and the helper files it uses). + * + * @param file the absolute path being read + * @param customizationFiles reported single-file customizations (prompts, instructions, agents) + * @param skillFolders reported skill folders (each directory containing a {@code SKILL.md}) + * @return {@code true} when the read is a recognized customization read + */ + public static boolean isCustomizationRead(Path file, Set customizationFiles, Set skillFolders) { + Path normalized = file.toAbsolutePath().normalize(); + if (customizationFiles.contains(normalized)) { + return true; + } + for (Path skillFolder : skillFolders) { + if (normalized.startsWith(skillFolder)) { + return true; + } + } + return false; + } + /** Normalizes a file path for case-insensitive, separator-agnostic comparison. */ private static String normalizePath(String path) { return ConfirmationHandler.normalizePath(path); diff --git a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/services/ChatCompletionService.java b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/services/ChatCompletionService.java index cff99834..1f0953cd 100644 --- a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/services/ChatCompletionService.java +++ b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/services/ChatCompletionService.java @@ -30,6 +30,7 @@ import com.microsoft.copilot.eclipse.core.CopilotAuthStatusListener; import com.microsoft.copilot.eclipse.core.CopilotCore; import com.microsoft.copilot.eclipse.core.FeatureFlags; +import com.microsoft.copilot.eclipse.core.chat.service.ICustomizationFileService.CustomizationType; import com.microsoft.copilot.eclipse.core.events.CopilotEventConstants; import com.microsoft.copilot.eclipse.core.lsp.CopilotLanguageServerConnection; import com.microsoft.copilot.eclipse.core.lsp.protocol.ChatMode; @@ -76,7 +77,13 @@ public ChatCompletionService(CopilotLanguageServerConnection lsConnection, AuthS ResourcesPlugin.getWorkspace().addResourceChangeListener(skillFileListener, IResourceChangeEvent.POST_CHANGE); this.eventBroker = PlatformUI.getWorkbench().getService(IEventBroker.class); if (this.eventBroker != null) { - this.customPromptsChangedHandler = event -> fetchAsync(); + // Templates only surface skills and prompts, so ignore instruction/agent changes. + this.customPromptsChangedHandler = event -> { + Object type = event.getProperty(IEventBroker.DATA); + if (type == CustomizationType.SKILL || type == CustomizationType.PROMPT) { + fetchAsync(); + } + }; this.eventBroker.subscribe(CopilotEventConstants.TOPIC_CHAT_DID_CHANGE_CUSTOMIZATION_FILES, customPromptsChangedHandler); } diff --git a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/services/ChatServiceManager.java b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/services/ChatServiceManager.java index 0883074f..62e6c4a6 100644 --- a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/services/ChatServiceManager.java +++ b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/services/ChatServiceManager.java @@ -5,6 +5,7 @@ import com.microsoft.copilot.eclipse.core.AuthStatusManager; import com.microsoft.copilot.eclipse.core.CopilotCore; +import com.microsoft.copilot.eclipse.core.chat.service.CustomizationFileService; import com.microsoft.copilot.eclipse.core.chat.service.IChatServiceManager; import com.microsoft.copilot.eclipse.core.lsp.CopilotLanguageServerConnection; import com.microsoft.copilot.eclipse.core.persistence.ConversationPersistenceManager; @@ -29,6 +30,7 @@ public class ChatServiceManager implements IChatServiceManager { private ReferencedFileService referencedFileService; private McpConfigService mcpConfigService; private McpExtensionPointManager mcpExtensionPointManager; + private CustomizationFileService customizationFileService; private McpRuntimeLogger mcpRuntimeLogger; private ConversationPersistenceManager persistenceManager; @@ -51,6 +53,8 @@ public ChatServiceManager() { referencedFileService = new ReferencedFileService(); mcpConfigService = new McpConfigService(); mcpExtensionPointManager = new McpExtensionPointManager(mcpConfigService); + customizationFileService = new CustomizationFileService(this.lsConnection); + customizationFileService.refreshAllAsync(); mcpRuntimeLogger = new McpRuntimeLogger(); persistenceManager = new ConversationPersistenceManager(this.authStatusManager); chatFontService = new ChatFontService(); @@ -140,6 +144,11 @@ public McpConfigService getMcpConfigService() { return mcpConfigService; } + @Override + public CustomizationFileService getCustomizationFileService() { + return customizationFileService; + } + /** * Get the persistence manager. * @@ -202,6 +211,7 @@ public void dispose() { this.agentToolService.dispose(); this.referencedFileService.dispose(); this.mcpConfigService.dispose(); + this.customizationFileService.dispose(); this.contextWindowService.dispose(); if (this.byokService != null) { this.byokService.dispose(); From 22ab0375d7b623aa155932d4dbeaf38d014eb22b Mon Sep 17 00:00:00 2001 From: xinyi-gong Date: Fri, 3 Jul 2026 13:04:14 +0800 Subject: [PATCH 11/14] feat: Surface organization-enabled custom (BYOK) models (#333) --- .../core/lsp/protocol/CopilotModelTests.java | 88 +++++++++++++++++++ .../copilot-agent/package-lock.json | 66 +++++++------- .../copilot-agent/package.json | 12 +-- .../lsp/CopilotLanguageServerConnection.java | 16 ++-- .../core/lsp/protocol/CopilotModel.java | 38 +++++++- .../eclipse/core/lsp/protocol/ModelInfo.java | 8 +- .../eclipse/ui/utils/ModelUtilsTests.java | 22 +++++ .../ui/chat/ModelPickerGroupsBuilder.java | 2 +- .../copilot/eclipse/ui/i18n/Messages.java | 1 + .../eclipse/ui/i18n/messages.properties | 1 + .../ui/swt/ModelHoverContentProvider.java | 33 +++++++ .../copilot/eclipse/ui/utils/ModelUtils.java | 5 ++ 12 files changed, 236 insertions(+), 56 deletions(-) create mode 100644 com.microsoft.copilot.eclipse.core.test/src/com/microsoft/copilot/eclipse/core/lsp/protocol/CopilotModelTests.java diff --git a/com.microsoft.copilot.eclipse.core.test/src/com/microsoft/copilot/eclipse/core/lsp/protocol/CopilotModelTests.java b/com.microsoft.copilot.eclipse.core.test/src/com/microsoft/copilot/eclipse/core/lsp/protocol/CopilotModelTests.java new file mode 100644 index 00000000..37f7b85d --- /dev/null +++ b/com.microsoft.copilot.eclipse.core.test/src/com/microsoft/copilot/eclipse/core/lsp/protocol/CopilotModelTests.java @@ -0,0 +1,88 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + +package com.microsoft.copilot.eclipse.core.lsp.protocol; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; + +import org.junit.jupiter.api.Test; + +import com.google.gson.Gson; +import com.microsoft.copilot.eclipse.core.lsp.protocol.CopilotModel.CopilotModelCustomModel; + +/** + * Tests for {@link CopilotModel}, focusing on the {@code customModel} metadata that carries organization- and + * enterprise-contributed custom (BYOK) models exposed through {@code copilot/models}. + */ +class CopilotModelTests { + + private final Gson gson = new Gson(); + + @Test + void testDeserialize_populatesCustomModelMetadata() { + String json = """ + { + "modelFamily": "custom", + "modelName": "Sonnet (Org)", + "id": "claude-sonnet-org", + "scopes": ["chat-panel", "agent-panel"], + "customModel": { + "keyName": "Contoso Azure Key", + "ownerName": "Contoso", + "ownerType": "organization", + "provider": "azure" + } + } + """; + + CopilotModel model = gson.fromJson(json, CopilotModel.class); + + assertNotNull(model.getCustomModel()); + assertEquals("Contoso Azure Key", model.getCustomModel().keyName()); + assertEquals("Contoso", model.getCustomModel().ownerName()); + assertEquals("organization", model.getCustomModel().ownerType()); + assertEquals("azure", model.getCustomModel().provider()); + } + + @Test + void testDeserialize_customModelAbsentIsNull() { + String json = """ + { + "modelFamily": "gpt-4o", + "modelName": "GPT-4o", + "id": "gpt-4o", + "scopes": ["chat-panel"] + } + """; + + CopilotModel model = gson.fromJson(json, CopilotModel.class); + + assertNull(model.getCustomModel()); + } + + @Test + void testEqualsAndHashCode_accountForCustomModel() { + CopilotModel base = new CopilotModel(); + base.setId("claude-sonnet-org"); + base.setModelName("Sonnet (Org)"); + base.setCustomModel(new CopilotModelCustomModel("Contoso Azure Key", "Contoso", "organization", "azure")); + + CopilotModel same = new CopilotModel(); + same.setId("claude-sonnet-org"); + same.setModelName("Sonnet (Org)"); + same.setCustomModel(new CopilotModelCustomModel("Contoso Azure Key", "Contoso", "organization", "azure")); + + CopilotModel differentOwner = new CopilotModel(); + differentOwner.setId("claude-sonnet-org"); + differentOwner.setModelName("Sonnet (Org)"); + differentOwner.setCustomModel(new CopilotModelCustomModel("Contoso Azure Key", "Fabrikam", "organization", + "azure")); + + assertEquals(base, same); + assertEquals(base.hashCode(), same.hashCode()); + assertNotEquals(base, differentOwner); + } +} diff --git a/com.microsoft.copilot.eclipse.core/copilot-agent/package-lock.json b/com.microsoft.copilot.eclipse.core/copilot-agent/package-lock.json index f389abb6..eda43152 100644 --- a/com.microsoft.copilot.eclipse.core/copilot-agent/package-lock.json +++ b/com.microsoft.copilot.eclipse.core/copilot-agent/package-lock.json @@ -9,18 +9,18 @@ "version": "0.0.1", "hasInstallScript": true, "dependencies": { - "@github/copilot-language-server": "1.502.5", - "@github/copilot-language-server-darwin-arm64": "1.502.5", - "@github/copilot-language-server-darwin-x64": "1.502.5", - "@github/copilot-language-server-linux-arm64": "1.502.5", - "@github/copilot-language-server-linux-x64": "1.502.5", - "@github/copilot-language-server-win32-x64": "1.502.5" + "@github/copilot-language-server": "1.509.5", + "@github/copilot-language-server-darwin-arm64": "1.509.5", + "@github/copilot-language-server-darwin-x64": "1.509.5", + "@github/copilot-language-server-linux-arm64": "1.509.5", + "@github/copilot-language-server-linux-x64": "1.509.5", + "@github/copilot-language-server-win32-x64": "1.509.5" } }, "node_modules/@github/copilot-language-server": { - "version": "1.502.5", - "resolved": "https://registry.npmjs.org/@github/copilot-language-server/-/copilot-language-server-1.502.5.tgz", - "integrity": "sha512-vIFbb145TwhDD1KTdJjySAokBcJla6NWsfvqjotM2AsDu3lLtQjPFbt6P90vnMVbQ6Ixc/IbDqsjToaWmXqqIA==", + "version": "1.509.5", + "resolved": "https://registry.npmjs.org/@github/copilot-language-server/-/copilot-language-server-1.509.5.tgz", + "integrity": "sha512-cAZVbN5UHrm+ILqroO2NY2gbI7mYojwmiOClCWj8yPD0svXJKSVmmmDonkVrvNZ1VxW3tIbCbE1LRTL6HVN/9g==", "license": "MIT", "dependencies": { "vscode-languageserver-protocol": "^3.17.5" @@ -29,18 +29,18 @@ "copilot-language-server": "dist/language-server.js" }, "optionalDependencies": { - "@github/copilot-language-server-darwin-arm64": "1.502.5", - "@github/copilot-language-server-darwin-x64": "1.502.5", - "@github/copilot-language-server-linux-arm64": "1.502.5", - "@github/copilot-language-server-linux-x64": "1.502.5", - "@github/copilot-language-server-win32-arm64": "1.502.5", - "@github/copilot-language-server-win32-x64": "1.502.5" + "@github/copilot-language-server-darwin-arm64": "1.509.5", + "@github/copilot-language-server-darwin-x64": "1.509.5", + "@github/copilot-language-server-linux-arm64": "1.509.5", + "@github/copilot-language-server-linux-x64": "1.509.5", + "@github/copilot-language-server-win32-arm64": "1.509.5", + "@github/copilot-language-server-win32-x64": "1.509.5" } }, "node_modules/@github/copilot-language-server-darwin-arm64": { - "version": "1.502.5", - "resolved": "https://registry.npmjs.org/@github/copilot-language-server-darwin-arm64/-/copilot-language-server-darwin-arm64-1.502.5.tgz", - "integrity": "sha512-3QTwdHjX2qpOwRhKIcXj3PU+wr5FodMcHPRnXK2qMv4xiRtibuuqicFVPF7ckO7Xym2o2FfJHfS8e58NNl278w==", + "version": "1.509.5", + "resolved": "https://registry.npmjs.org/@github/copilot-language-server-darwin-arm64/-/copilot-language-server-darwin-arm64-1.509.5.tgz", + "integrity": "sha512-eGb5bHN1OVUEnQYCop1+Hd4z6CR2fZK0iqFRu0pdGeQ3/y370trt2KR+omJv354ibsCUmpDUIqZXb6hOiIZk/Q==", "cpu": [ "arm64" ], @@ -50,9 +50,9 @@ ] }, "node_modules/@github/copilot-language-server-darwin-x64": { - "version": "1.502.5", - "resolved": "https://registry.npmjs.org/@github/copilot-language-server-darwin-x64/-/copilot-language-server-darwin-x64-1.502.5.tgz", - "integrity": "sha512-qz3aGl2KtAA/pM65esyXrsv4wBDFoKwUrP4hBAm2XplIPVF931HOluCXGxL0GDU2OMsEAxgFy5xTGODMg1U7kQ==", + "version": "1.509.5", + "resolved": "https://registry.npmjs.org/@github/copilot-language-server-darwin-x64/-/copilot-language-server-darwin-x64-1.509.5.tgz", + "integrity": "sha512-EonG1QHCx7qmiPc0EsVYSk9oBeWtwHXw0h3fMFOWKN37HOKImMiDeC7JBjv1kG/q9XzHGGhkYjkvReebMKZoOg==", "cpu": [ "x64" ], @@ -62,9 +62,9 @@ ] }, "node_modules/@github/copilot-language-server-linux-arm64": { - "version": "1.502.5", - "resolved": "https://registry.npmjs.org/@github/copilot-language-server-linux-arm64/-/copilot-language-server-linux-arm64-1.502.5.tgz", - "integrity": "sha512-8xabGJgGzpa4KVDQc1ZYH3elTzyzVRUWJLlBBQbK4vSpNtsnmGOk4TkY7uJZ7uXtaX1/xDD1venbPWWNOABCig==", + "version": "1.509.5", + "resolved": "https://registry.npmjs.org/@github/copilot-language-server-linux-arm64/-/copilot-language-server-linux-arm64-1.509.5.tgz", + "integrity": "sha512-f1SYaxh8drXeZ2Sf5A/gjFC5gg0qnUdwYUWXA2vpJrMZ2bMbylacGS9hwhkwk9jDaHkeOmjOWWh+g2csdLgVTw==", "cpu": [ "arm64" ], @@ -74,9 +74,9 @@ ] }, "node_modules/@github/copilot-language-server-linux-x64": { - "version": "1.502.5", - "resolved": "https://registry.npmjs.org/@github/copilot-language-server-linux-x64/-/copilot-language-server-linux-x64-1.502.5.tgz", - "integrity": "sha512-7JcmuKj4yOoDjU3kx4JlVPzC2jDES6nFFqUoNAEnKPFPUjIwf4RJKI2heonIUtFnFXZ9JWT09vqe477oO0F80w==", + "version": "1.509.5", + "resolved": "https://registry.npmjs.org/@github/copilot-language-server-linux-x64/-/copilot-language-server-linux-x64-1.509.5.tgz", + "integrity": "sha512-T0ihY6w2H7sdKMaF0k7pHBQV3wTj4Uzq38kx8pGKAFTupUIgivMUHOWysiLdsBgQ9rafByIMzHR3slnpYcH1SQ==", "cpu": [ "x64" ], @@ -86,9 +86,9 @@ ] }, "node_modules/@github/copilot-language-server-win32-arm64": { - "version": "1.502.5", - "resolved": "https://registry.npmjs.org/@github/copilot-language-server-win32-arm64/-/copilot-language-server-win32-arm64-1.502.5.tgz", - "integrity": "sha512-7/QHmdWSIOKvbr5s5tH1u8/nQ+AfvqqGbPlpZeokv/hTVwCZZ69ntMVGkoSQVKcSAYVL2h97M5+0zNbofYitow==", + "version": "1.509.5", + "resolved": "https://registry.npmjs.org/@github/copilot-language-server-win32-arm64/-/copilot-language-server-win32-arm64-1.509.5.tgz", + "integrity": "sha512-T4fyhZIJCU+ETyp3vcIw0Y+sIvcsnq8C59s2l7RR9jX6lwHacwsU2PUcRYQqETfWOkM5YX7wzKiThLxvsOAc9g==", "cpu": [ "arm64" ], @@ -99,9 +99,9 @@ ] }, "node_modules/@github/copilot-language-server-win32-x64": { - "version": "1.502.5", - "resolved": "https://registry.npmjs.org/@github/copilot-language-server-win32-x64/-/copilot-language-server-win32-x64-1.502.5.tgz", - "integrity": "sha512-9jdvyPZnu2muS9Jo6LtmP33NkpTht/2Gc9pWRrkQizuNabWy7jq2KuJdYf1/vwiyEioMiZznGBv+hscrJ9z5LA==", + "version": "1.509.5", + "resolved": "https://registry.npmjs.org/@github/copilot-language-server-win32-x64/-/copilot-language-server-win32-x64-1.509.5.tgz", + "integrity": "sha512-tv12/MayELMseV7V+ghdC/AY5GgKh/lyncAU/A7eEpPt8ACNEErBZ51mriAT1F/EqSfB1iEUKVtgZ4/+eTe9Fw==", "cpu": [ "x64" ], diff --git a/com.microsoft.copilot.eclipse.core/copilot-agent/package.json b/com.microsoft.copilot.eclipse.core/copilot-agent/package.json index 7702b184..d6c12fb7 100644 --- a/com.microsoft.copilot.eclipse.core/copilot-agent/package.json +++ b/com.microsoft.copilot.eclipse.core/copilot-agent/package.json @@ -12,11 +12,11 @@ "postinstall": "node copy-binaries.js" }, "dependencies": { - "@github/copilot-language-server": "1.502.5", - "@github/copilot-language-server-win32-x64": "1.502.5", - "@github/copilot-language-server-darwin-x64": "1.502.5", - "@github/copilot-language-server-darwin-arm64": "1.502.5", - "@github/copilot-language-server-linux-x64": "1.502.5", - "@github/copilot-language-server-linux-arm64": "1.502.5" + "@github/copilot-language-server": "1.509.5", + "@github/copilot-language-server-win32-x64": "1.509.5", + "@github/copilot-language-server-darwin-x64": "1.509.5", + "@github/copilot-language-server-darwin-arm64": "1.509.5", + "@github/copilot-language-server-linux-x64": "1.509.5", + "@github/copilot-language-server-linux-arm64": "1.509.5" } } 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 1c853da5..5844c4f7 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 @@ -749,19 +749,15 @@ private String getModelName(CopilotModel activeModel) { } /** - * Builds the {@link ModelInfo} payload to forward with chat requests. Returns {@code null} when no reasoning effort - * is available so we do not send redundant {@code id}/{@code providerName} fields ahead of the future migration - * away from the legacy {@code model}/{@code modelProviderName} fields. Today the language server only consumes - * {@code modelInfo.reasoningEffort}, so suppressing the payload when there is nothing meaningful to forward keeps - * the protocol surface minimal and avoids implicit behaviour changes if the server starts honouring id/providerName - * before the client migration lands. + * Builds the {@link ModelInfo} payload for chat requests. Always sends the concrete model id (which the server + * prefers over the legacy {@code model} family field), since the family is not unique across models. Returns + * {@code null} when no model id is available. */ private static ModelInfo buildModelInfo(CopilotModel activeModel, String reasoningEffort) { - if (StringUtils.isBlank(reasoningEffort)) { + if (activeModel == null || StringUtils.isBlank(activeModel.getId())) { return null; } - String id = activeModel != null ? activeModel.getId() : null; - String providerName = activeModel != null ? activeModel.getProviderName() : null; - return new ModelInfo(id, providerName, reasoningEffort, null); + String effort = StringUtils.isBlank(reasoningEffort) ? null : reasoningEffort; + return new ModelInfo(activeModel.getId(), activeModel.getProviderName(), effort, null); } } diff --git a/com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/lsp/protocol/CopilotModel.java b/com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/lsp/protocol/CopilotModel.java index dfc8bd3c..b766f49c 100644 --- a/com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/lsp/protocol/CopilotModel.java +++ b/com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/lsp/protocol/CopilotModel.java @@ -25,6 +25,7 @@ public class CopilotModel { private boolean isChatFallback; private CopilotModelCapabilities capabilities; private CopilotModelBilling billing; + private CopilotModelCustomModel customModel; private String degradationReason; private String providerName; private String modelPickerCategory; @@ -158,6 +159,28 @@ public String toString() { } } + /** + * Metadata describing a custom (BYOK) model that is contributed to the user by an organization or enterprise. When + * present, the model is served through a key that the owner configured, so it is surfaced in the model picker + * automatically without the user having to register their own API key. + * + * @param keyName the display name of the API key the model is served through + * @param ownerName the name of the organization, enterprise, or user that contributed the model + * @param ownerType the type of the owner (e.g. {@code organization}, {@code enterprise}, {@code user}) + * @param provider the underlying model provider (e.g. {@code azure}, {@code openai}) + */ + public record CopilotModelCustomModel(String keyName, String ownerName, String ownerType, String provider) { + @Override + public String toString() { + ToStringBuilder builder = new ToStringBuilder(this); + builder.append("keyName", keyName); + builder.append("ownerName", ownerName); + builder.append("ownerType", ownerType); + builder.append("provider", provider); + return builder.toString(); + } + } + public String getModelFamily() { return modelFamily; } @@ -246,6 +269,14 @@ public void setBilling(CopilotModelBilling billing) { this.billing = billing; } + public CopilotModelCustomModel getCustomModel() { + return customModel; + } + + public void setCustomModel(CopilotModelCustomModel customModel) { + this.customModel = customModel; + } + public String getDegradationReason() { return degradationReason; } @@ -300,6 +331,7 @@ public boolean equals(Object obj) { } CopilotModel other = (CopilotModel) obj; return Objects.equals(billing, other.billing) && Objects.equals(capabilities, other.capabilities) + && Objects.equals(customModel, other.customModel) && Objects.equals(degradationReason, other.degradationReason) && Objects.equals(id, other.id) && isChatDefault == other.isChatDefault && isChatFallback == other.isChatFallback && Objects.equals(modelFamily, other.modelFamily) && Objects.equals(modelName, other.modelName) @@ -312,8 +344,9 @@ public boolean equals(Object obj) { @Override public int hashCode() { - return Objects.hash(billing, capabilities, degradationReason, id, isChatDefault, isChatFallback, modelFamily, - modelName, modelPickerCategory, modelPickerPriceCategory, modelPolicy, preview, providerName, scopes, vendor); + return Objects.hash(billing, capabilities, customModel, degradationReason, id, isChatDefault, isChatFallback, + modelFamily, modelName, modelPickerCategory, modelPickerPriceCategory, modelPolicy, preview, providerName, + scopes, vendor); } @Override @@ -330,6 +363,7 @@ public String toString() { builder.append("isChatFallback", isChatFallback); builder.append("capabilities", capabilities); builder.append("billing", billing); + builder.append("customModel", customModel); builder.append("degradationReason", degradationReason); builder.append("providerName", providerName); builder.append("modelPickerCategory", modelPickerCategory); diff --git a/com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/lsp/protocol/ModelInfo.java b/com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/lsp/protocol/ModelInfo.java index de61df10..e7ff3e84 100644 --- a/com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/lsp/protocol/ModelInfo.java +++ b/com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/lsp/protocol/ModelInfo.java @@ -7,10 +7,10 @@ * Optional model metadata associated with a conversation turn. Mirrors the {@code modelInfo} field on the * {@code conversation/create} and {@code conversation/turn} LSP requests and responses. * - *

The {@code id} and {@code providerName} fields are reserved for a future migration away from the legacy - * {@code model} / {@code modelProviderName} fields and should not be relied upon today. The {@code reasoningEffort} - * field carries the user-selected reasoning effort level (e.g. {@code low}, {@code medium}, {@code high}) when the - * model surfaces selectable effort levels. + *

The language server resolves the turn's model from {@code id} (and {@code providerName} for BYOK models) in + * preference to the legacy {@code model} / {@code modelProviderName} request fields, so {@code id} should carry the + * concrete model id of the user-selected model. The {@code reasoningEffort} field carries the user-selected reasoning + * effort level (e.g. {@code low}, {@code medium}, {@code high}) when the model surfaces selectable effort levels. * * @param id model identifier (optional) * @param providerName provider name (optional) diff --git a/com.microsoft.copilot.eclipse.ui.test/src/com/microsoft/copilot/eclipse/ui/utils/ModelUtilsTests.java b/com.microsoft.copilot.eclipse.ui.test/src/com/microsoft/copilot/eclipse/ui/utils/ModelUtilsTests.java index e8448ea6..e2185223 100644 --- a/com.microsoft.copilot.eclipse.ui.test/src/com/microsoft/copilot/eclipse/ui/utils/ModelUtilsTests.java +++ b/com.microsoft.copilot.eclipse.ui.test/src/com/microsoft/copilot/eclipse/ui/utils/ModelUtilsTests.java @@ -17,6 +17,7 @@ import com.microsoft.copilot.eclipse.core.lsp.protocol.CopilotModel.CopilotModelCapabilities; import com.microsoft.copilot.eclipse.core.lsp.protocol.CopilotModel.CopilotModelCapabilitiesLimits; import com.microsoft.copilot.eclipse.core.lsp.protocol.CopilotModel.CopilotModelCapabilitiesSupports; +import com.microsoft.copilot.eclipse.core.lsp.protocol.CopilotModel.CopilotModelCustomModel; import com.microsoft.copilot.eclipse.core.lsp.protocol.CopilotScope; import com.microsoft.copilot.eclipse.core.lsp.protocol.byok.ByokModel; import com.microsoft.copilot.eclipse.core.lsp.protocol.byok.ByokModelCapabilities; @@ -150,6 +151,27 @@ void testSupportsReasoningEffortLevel_falseWhenCapabilitiesMissing() { assertFalse(ModelUtils.supportsReasoningEffortLevel(null)); } + @Test + void testGetModelSuffix_customModelUsesProvider() { + // Organization-contributed custom models arrive without a providerName but carry their provider in the + // custom-model metadata; the suffix should surface that provider like a BYOK model. + CopilotModel model = new CopilotModel(); + model.setModelName("Sonnet (Org)"); + model.setCustomModel(new CopilotModelCustomModel("Contoso Azure Key", "Contoso", "organization", "Azure")); + + assertEquals("Azure", ModelUtils.getModelSuffix(model, null)); + } + + @Test + void testGetModelSuffix_providerNameTakesPrecedenceOverCustomModel() { + CopilotModel model = new CopilotModel(); + model.setModelName("GPT-4o"); + model.setProviderName("OpenAI"); + model.setCustomModel(new CopilotModelCustomModel("Key", "Contoso", "organization", "Azure")); + + assertEquals("OpenAI", ModelUtils.getModelSuffix(model, null)); + } + @Test void testIsAutoModel() { CopilotModel auto = new CopilotModel(); diff --git a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/ModelPickerGroupsBuilder.java b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/ModelPickerGroupsBuilder.java index fb838528..46a245ee 100644 --- a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/ModelPickerGroupsBuilder.java +++ b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/ModelPickerGroupsBuilder.java @@ -66,7 +66,7 @@ public static List build(Map modelMap, List customModels = new ArrayList<>(); for (CopilotModel model : modelMap.values()) { - if (model.getProviderName() != null) { + if (model.getProviderName() != null || model.getCustomModel() != null) { customModels.add(model); } else if (model.getBilling() != null) { if (model.getBilling().isPremium()) { diff --git a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/i18n/Messages.java b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/i18n/Messages.java index cb3fc2dd..642b71b1 100644 --- a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/i18n/Messages.java +++ b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/i18n/Messages.java @@ -203,6 +203,7 @@ public final class Messages extends NLS { public static String model_hover_cost; public static String model_hover_thinkingEffort; public static String model_hover_thinkingEffort_default_suffix; + public static String model_hover_customModelInfo; public static String model_reasoningEffort_none; public static String model_reasoningEffort_low; public static String model_reasoningEffort_medium; diff --git a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/i18n/messages.properties b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/i18n/messages.properties index 3974ed60..bb6b4400 100644 --- a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/i18n/messages.properties +++ b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/i18n/messages.properties @@ -197,6 +197,7 @@ model_hover_contextWindow=Context Window: model_hover_cost=Cost: model_hover_thinkingEffort=Thinking Effort model_hover_thinkingEffort_default_suffix={0} (default) +model_hover_customModelInfo={0} is contributed by {1} using {2} model_reasoningEffort_none=None model_reasoningEffort_low=Low model_reasoningEffort_medium=Medium diff --git a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/swt/ModelHoverContentProvider.java b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/swt/ModelHoverContentProvider.java index 1c963f89..425a6b54 100644 --- a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/swt/ModelHoverContentProvider.java +++ b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/swt/ModelHoverContentProvider.java @@ -27,6 +27,7 @@ import com.microsoft.copilot.eclipse.core.lsp.protocol.CopilotModel; import com.microsoft.copilot.eclipse.core.lsp.protocol.CopilotModel.CopilotModelCapabilitiesSupports; +import com.microsoft.copilot.eclipse.core.lsp.protocol.CopilotModel.CopilotModelCustomModel; import com.microsoft.copilot.eclipse.ui.CopilotUi; import com.microsoft.copilot.eclipse.ui.chat.services.ModelService; import com.microsoft.copilot.eclipse.ui.i18n.Messages; @@ -76,6 +77,8 @@ public void configureHover(Composite parent, DropdownItem item, Runnable closeRe addWarningRow(parent, model.getDegradationReason()); } + addCustomModelInfoSection(parent, item.getLabel()); + addContextWindowSection(parent); addPricingSection(parent, model.getModelPickerPriceCategory()); addThinkingEffortSection(parent, closeRequest); @@ -89,6 +92,36 @@ private void renderHeader(Composite parent, DropdownItem item) { titleLabel.setLayoutData(headerGd); } + /** + * Renders the "contributed by" line for organization/enterprise-contributed custom (BYOK) models. Mirrors the + * IntelliJ model tooltip: the row only appears when the model carries custom-model metadata with a non-blank owner + * and key name, communicating that the model was provided by an owner through a specific key. + * + * @param parent the hover composite to render into + * @param displayedName the model name as shown in the hover header + */ + private void addCustomModelInfoSection(Composite parent, String displayedName) { + CopilotModelCustomModel customModel = model.getCustomModel(); + if (customModel == null) { + return; + } + String ownerName = StringUtils.trimToNull(customModel.ownerName()); + String keyName = StringUtils.trimToNull(customModel.keyName()); + if (ownerName == null || keyName == null) { + return; + } + + String modelLabel = StringUtils.defaultIfBlank(displayedName, model.getModelName()); + String infoText = NLS.bind(Messages.model_hover_customModelInfo, + new Object[] { modelLabel, ownerName, keyName }); + Label infoLabel = new Label(parent, SWT.WRAP); + infoLabel.setText(infoText); + setCssClass(infoLabel, POPUP_SECONDARY_TEXT_CLASS); + GridData gd = new GridData(SWT.FILL, SWT.NONE, true, false); + gd.verticalIndent = SECTION_SPACING; + infoLabel.setLayoutData(gd); + } + private void addContextWindowSection(Composite parent) { String contextWindowText = ModelUtils.getContextWindowText(model); if (StringUtils.isBlank(contextWindowText)) { diff --git a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/utils/ModelUtils.java b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/utils/ModelUtils.java index f806727f..5919742d 100644 --- a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/utils/ModelUtils.java +++ b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/utils/ModelUtils.java @@ -101,6 +101,11 @@ public static String getModelSuffix(CopilotModel model, String reasoningEffort) if (model.getProviderName() != null) { return model.getProviderName(); } + // Organization/enterprise-contributed custom models arrive through copilot/models without a providerName, but + // still carry their underlying provider in the custom-model metadata. Surface it so they read like BYOK models. + if (model.getCustomModel() != null && StringUtils.isNotBlank(model.getCustomModel().provider())) { + return model.getCustomModel().provider(); + } if (isAutoModel(model)) { return Messages.model_billing_multiplier_variable; } From 073b82a3ec5d3b6cc2257ec18fd96079f16a4d04 Mon Sep 17 00:00:00 2001 From: xinyi-gong Date: Fri, 3 Jul 2026 13:17:12 +0800 Subject: [PATCH 12/14] fix - chat not scrolling to the newest messages in long conversations (#334) --- .../ui/chat/ChatContentViewerTest.java | 115 ++++++ .../eclipse/ui/chat/ChatContentViewer.java | 372 +++++++++++------- 2 files changed, 346 insertions(+), 141 deletions(-) diff --git a/com.microsoft.copilot.eclipse.ui.test/src/com/microsoft/copilot/eclipse/ui/chat/ChatContentViewerTest.java b/com.microsoft.copilot.eclipse.ui.test/src/com/microsoft/copilot/eclipse/ui/chat/ChatContentViewerTest.java index b181bd9a..16595f17 100644 --- a/com.microsoft.copilot.eclipse.ui.test/src/com/microsoft/copilot/eclipse/ui/chat/ChatContentViewerTest.java +++ b/com.microsoft.copilot.eclipse.ui.test/src/com/microsoft/copilot/eclipse/ui/chat/ChatContentViewerTest.java @@ -9,14 +9,17 @@ import static org.mockito.Mockito.when; import java.lang.reflect.Field; +import java.lang.reflect.Method; import java.util.List; import java.util.Map; import org.eclipse.lsp4j.WorkDoneProgressKind; import org.eclipse.swt.SWT; import org.eclipse.swt.widgets.Display; +import org.eclipse.swt.widgets.ScrollBar; import org.eclipse.swt.widgets.Shell; import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; @@ -104,6 +107,118 @@ private Map getTurnsMap(ChatContentViewer viewer) { return (Map) getFieldValue(viewer, "turns"); } + @Test + void testClampOffset_clampsToScrollableRange() { + SwtUtils.invokeOnDisplayThread(() -> { + sizeViewer(400, 300); + setFieldValue(viewer, "totalHeight", 1000); + int maxOffset = invokeIntMethod(viewer, "maxOffset"); + + Assertions.assertTrue(maxOffset > 0, "content taller than the viewport should be scrollable"); + Assertions.assertEquals(0, invokeIntMethod(viewer, "clampOffset", -50), + "negative offset should clamp to 0"); + Assertions.assertEquals(maxOffset, invokeIntMethod(viewer, "clampOffset", 5000), + "offset past the end should clamp to maxOffset"); + Assertions.assertEquals(200, invokeIntMethod(viewer, "clampOffset", 200), + "in-range offset should be left unchanged"); + }); + } + + @Test + void testMaxOffset_isZeroWhenContentFitsViewport() { + SwtUtils.invokeOnDisplayThread(() -> { + sizeViewer(400, 300); + setFieldValue(viewer, "totalHeight", 200); + + Assertions.assertEquals(0, invokeIntMethod(viewer, "maxOffset"), + "content shorter than the viewport should not be scrollable"); + }); + } + + @Test + void testUpdateScrollBar_rangeMatchesTotalHeight() { + SwtUtils.invokeOnDisplayThread(() -> { + sizeViewer(400, 300); + setFieldValue(viewer, "totalHeight", 1000); + setFieldValue(viewer, "scrollOffset", 120); + + invokeVoidMethod(viewer, "updateScrollBar", 300); + + ScrollBar bar = viewer.getVerticalBar(); + Assertions.assertTrue(bar.getEnabled(), "scrollbar should be enabled when content overflows"); + Assertions.assertEquals(1000, bar.getMaximum(), "scrollbar maximum should equal totalHeight"); + Assertions.assertEquals(300, bar.getThumb(), "scrollbar thumb should equal the viewport height"); + Assertions.assertEquals(120, bar.getSelection(), "scrollbar selection should equal scrollOffset"); + }); + } + + @Test + void testUpdateScrollBar_disabledWhenContentFits() { + SwtUtils.invokeOnDisplayThread(() -> { + sizeViewer(400, 300); + setFieldValue(viewer, "totalHeight", 200); + + invokeVoidMethod(viewer, "updateScrollBar", 300); + + Assertions.assertFalse(viewer.getVerticalBar().getEnabled(), + "scrollbar should be disabled when all content fits"); + }); + } + + @Test + void testIsViewportAtBottom_togglesWhenLeavingAndReturning() { + SwtUtils.invokeOnDisplayThread(() -> { + sizeViewer(400, 300); + setFieldValue(viewer, "totalHeight", 1000); + int maxOffset = invokeIntMethod(viewer, "maxOffset"); + + setFieldValue(viewer, "scrollOffset", maxOffset); + Assertions.assertTrue(invokeBooleanMethod(viewer, "isViewportAtBottom"), + "pinned to the very bottom should report at-bottom"); + + setFieldValue(viewer, "scrollOffset", 0); + Assertions.assertFalse(invokeBooleanMethod(viewer, "isViewportAtBottom"), + "scrolled up to the top should report not-at-bottom"); + + // Just inside the bottom threshold (SCROLL_THRESHOLD = 100) should count as at-bottom again. + setFieldValue(viewer, "scrollOffset", maxOffset - 50); + Assertions.assertTrue(invokeBooleanMethod(viewer, "isViewportAtBottom"), + "back within the bottom threshold should report at-bottom again"); + }); + } + + /** Gives the viewer (and its content child) a deterministic client area for scroll-model math. */ + private void sizeViewer(int width, int height) { + viewer.setSize(width, height); + viewer.layout(true, true); + } + + private int invokeIntMethod(Object target, String name, int arg) { + return (int) invokeMethod(target, name, new Class[] {int.class}, arg); + } + + private int invokeIntMethod(Object target, String name) { + return (int) invokeMethod(target, name, new Class[] {}); + } + + private boolean invokeBooleanMethod(Object target, String name) { + return (boolean) invokeMethod(target, name, new Class[] {}); + } + + private void invokeVoidMethod(Object target, String name, int arg) { + invokeMethod(target, name, new Class[] {int.class}, arg); + } + + private Object invokeMethod(Object target, String name, Class[] paramTypes, Object... args) { + try { + Method method = target.getClass().getDeclaredMethod(name, paramTypes); + method.setAccessible(true); + return method.invoke(target, args); + } catch (Exception e) { + throw new RuntimeException("Failed to invoke method " + name, e); + } + } + private Object getFieldValue(Object target, String fieldName) { try { Field field = target.getClass().getDeclaredField(fieldName); diff --git a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/ChatContentViewer.java b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/ChatContentViewer.java index 7188df57..83e197e7 100644 --- a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/ChatContentViewer.java +++ b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/ChatContentViewer.java @@ -5,6 +5,7 @@ import java.util.ArrayList; import java.util.HashMap; +import java.util.IdentityHashMap; import java.util.List; import java.util.Map; import java.util.Queue; @@ -18,13 +19,10 @@ import org.eclipse.e4.core.services.events.IEventBroker; import org.eclipse.lsp4j.WorkDoneProgressKind; import org.eclipse.swt.SWT; -import org.eclipse.swt.custom.ScrolledComposite; -import org.eclipse.swt.events.ControlAdapter; -import org.eclipse.swt.events.ControlEvent; -import org.eclipse.swt.graphics.Point; +import org.eclipse.swt.graphics.Font; +import org.eclipse.swt.graphics.GC; import org.eclipse.swt.graphics.Rectangle; import org.eclipse.swt.layout.GridData; -import org.eclipse.swt.layout.GridLayout; import org.eclipse.swt.widgets.Composite; import org.eclipse.swt.widgets.Control; import org.eclipse.swt.widgets.ScrollBar; @@ -50,8 +48,14 @@ /** * Widget to display chat content. + * + *

A self-managed, windowed vertical scroller (not a {@link org.eclipse.swt.custom.ScrolledComposite}): + * on Windows a classic scroller cannot move very tall content far enough to reveal the newest messages. + * Here {@code cmpContent} is pinned to the viewport and each turn is positioned in + * viewport-local coordinates; off-window turns are parked with {@code setVisible(false)}, so no child is + * ever given an out-of-range native coordinate.

*/ -public class ChatContentViewer extends ScrolledComposite { +public class ChatContentViewer extends Composite { private static final int SCROLL_THRESHOLD = 100; @@ -86,6 +90,18 @@ public class ChatContentViewer extends ScrolledComposite { /** Guards against scheduling more than one pending async refresh from a burst of resize events. */ private final AtomicBoolean refreshScheduled = new AtomicBoolean(false); + /** Current logical scroll position (top of the viewport in content coordinates). */ + private int scrollOffset; + + /** Full logical content height in pixels; may far exceed any native coordinate limit. */ + private int totalHeight; + + /** Cached measured heights keyed by row control identity; invalidated on width change. */ + private final Map heightCache = new IdentityHashMap<>(); + + /** Cached font line height (px), the unit for one scroll line; recomputed when the font changes. */ + private int cachedLineHeight = -1; + /** * Create the composite. * @@ -93,47 +109,46 @@ public class ChatContentViewer extends ScrolledComposite { * @param style the style */ public ChatContentViewer(Composite parent, int style, ChatServiceManager serviceManager) { - super(parent, style | SWT.V_SCROLL); - this.setExpandHorizontal(true); - this.setExpandVertical(true); - this.setLayout(new GridLayout(1, true)); + super(parent, style | SWT.V_SCROLL | SWT.DOUBLE_BUFFERED); + // Null layout: children are positioned manually by relayoutWindow() so SWT never stacks them into + // one oversized composite. + this.setLayout(null); this.setLayoutData(new GridData(SWT.FILL, SWT.FILL, true, true)); this.setData(CssConstants.CSS_ID_KEY, "chat-content-viewer"); this.cmpContent = new Composite(this, SWT.NONE); - GridLayout gl = new GridLayout(1, true); - gl.marginHeight = 0; - gl.marginWidth = 0; - this.cmpContent.setLayout(gl); - this.cmpContent.setLayoutData(new GridData(SWT.FILL, SWT.FILL, true, true)); - this.setContent(this.cmpContent); - - this.addControlListener(new ControlAdapter() { - @Override - public void controlResized(ControlEvent e) { - // setMinHeight re-fires controlResized; coalesce into one async incremental pass so the - // resize handler never recurses synchronously into layout. - coalesceAsync(refreshScheduled, ChatContentViewer.this::refreshLayoutIncremental); - } + this.cmpContent.setLayout(null); + + this.addListener(SWT.Resize, e -> { + layoutContentArea(); + coalesceAsync(refreshScheduled, this::refreshLayoutIncremental); }); - // Listen for user scroll events to manage auto-scroll behavior ScrollBar verticalBar = this.getVerticalBar(); if (verticalBar != null) { verticalBar.addListener(SWT.Selection, event -> { - int selection = verticalBar.getSelection(); - int maximum = verticalBar.getMaximum(); - int thumb = verticalBar.getThumb(); - - // If scrolled to bottom, keep auto-scroll enabled - // Otherwise disable it (user is viewing history) - int threshold = SCROLL_THRESHOLD; - int maxScrollPosition = maximum - thumb; - boolean isAtBottom = selection >= (maxScrollPosition - threshold); - autoScrollEnabled = isAtBottom; + scrollOffset = verticalBar.getSelection(); + autoScrollEnabled = isViewportAtBottom(); + relayoutWindow(); }); } + // The wheel does not move the scrollbar on a manually-laid-out composite, so handle it explicitly. + this.addListener(SWT.MouseWheel, event -> { + if (totalHeight <= getClientArea().height) { + return; + } + // event.count is the OS-provided number of lines to scroll for this notch; a font line height + // turns it into pixels, so one notch moves whole text lines like a native scrollable. + int newOffset = clampOffset(scrollOffset - event.count * lineHeight()); + if (newOffset != scrollOffset) { + scrollOffset = newOffset; + autoScrollEnabled = isViewportAtBottom(); + relayoutWindow(); + } + event.doit = false; + }); + this.turns = new HashMap<>(); this.activeThinkingBlockIds = new ConcurrentHashMap<>(); @@ -471,11 +486,9 @@ public void renderErrorMessage(String errorMessage) { } /** - * Coalesces a burst of calls into a single async pass on the UI thread. While a pass is already - * scheduled, further calls are dropped; {@code scheduled} is cleared right before {@code task} runs - * so work arriving during the task re-schedules a follow-up pass. This breaks synchronous - * re-entrancy (e.g. a layout pass writing the scroller min size re-fires {@code controlResized}) - * without a re-entrancy guard, while idempotent writes still converge to a fixed point. + * Coalesces a burst of calls into a single async pass on the UI thread, clearing {@code scheduled} + * right before {@code task} runs so work arriving during the task re-schedules a follow-up pass. + * Breaks synchronous re-entrancy without a re-entrancy guard. * * @param scheduled the per-task latch guarding against duplicate scheduling * @param task the work to run once on the next UI-thread turn @@ -509,19 +522,15 @@ private void refreshLayoutIncremental() { * Selects how many turns {@link #refreshLayout(MeasureMode)} re-measures. */ private enum MeasureMode { - /** Recursively re-measure every turn. O(n) in the number of turns. */ + /** Re-measure every turn. */ FULL, - /** Only flush the trailing (mutating) turns; sealed turns keep cached sizes. O(1). */ + /** Only re-measure the trailing (mutating) turns; sealed turns keep cached sizes. */ INCREMENTAL } /** - * Re-measure the scroller and update its min size. - * - * @param mode {@link MeasureMode#FULL} recursively re-measures every turn; {@link - * MeasureMode#INCREMENTAL} only flushes the trailing (mutating) turns while sealed turns keep - * cached sizes, keeping the pass O(1). A width change always upgrades to a full measure because - * text re-wraps. + * Re-measures turns and re-runs the windowing pass. {@link MeasureMode#INCREMENTAL} keeps sealed + * turns' cached heights; a width change forces a full re-measure because text re-wraps. */ private void refreshLayout(MeasureMode mode) { if (this.isDisposed()) { @@ -532,133 +541,200 @@ private void refreshLayout(MeasureMode mode) { boolean fullMeasure = mode == MeasureMode.FULL || width != lastLayoutWidth; lastLayoutWidth = width; - if (!fullMeasure) { - // Only the trailing turns can grow/change during streaming. - flushTrailingTurnCaches(); - } - - Point containerSize = cmpContent.computeSize(width, SWT.DEFAULT, fullMeasure); - - // Use the default size as a fallback - if (latestUserTurn == null) { - this.setMinSize(containerSize); - return; - } - - // Measure at the actual column width, not SWT.DEFAULT: unconstrained width collapses - // soft-wrapped text to a single line and under-estimates the height. roundedHeight must match - // the laid-out height shouldAutoScrollToBottom() reads via getBounds(), or the padding branch - // below reserves phantom whitespace while auto-scroll fires into it (issue #259 flicker). - int userTurnHeight = latestUserTurn.computeSize(width, SWT.DEFAULT).y; - int copilotTurnHeight = latestCopilotTurn == null || latestCopilotTurn.isDisposed() ? 0 - : latestCopilotTurn.computeSize(width, SWT.DEFAULT).y; - - // Calculate the content height, so that the latest user turn is able to be put at the top of the client area. - int contentHeight = 0; - int roundedHeight = userTurnHeight + copilotTurnHeight; - if (roundedHeight < clientArea.height) { - contentHeight = clientArea.height + containerSize.y - roundedHeight; + if (fullMeasure) { + heightCache.clear(); } else { - contentHeight = containerSize.y; + invalidateTrailingTurnHeights(); } - // Only write min size when it changes: setMin* re-fires controlResized, so skipping no-op writes - // lets the coalesced async refresh converge to a fixed point. - if (this.getMinHeight() != contentHeight) { - this.setMinHeight(contentHeight); - } - if (this.getMinWidth() != containerSize.x) { - this.setMinWidth(containerSize.x); - } - // Incremental layout: only re-position the latest (growing) copilot turn instead of recursing - // into all past turns as conversations grow longer. - if (latestCopilotTurn != null && !latestCopilotTurn.isDisposed()) { - cmpContent.layout(new Control[] {latestCopilotTurn}); - } else { - cmpContent.layout(true, false); - } - this.layout(true, false); + layoutContentArea(); + relayoutWindow(); } /** - * Scrolls the viewport to the bottom when auto-scroll is currently enabled. Kept separate from the - * layout pass so refresh and scroll stay independent concerns; callers invoke this after a refresh - * when they want the latest content to stay in view. + * Scrolls to the bottom when auto-scroll is enabled. The bottom padding reserved by {@link + * #relayoutWindow()} makes this pin the latest user turn to the top while the round is short, then + * follow the real bottom once it grows past the viewport. */ public void scrollToBottomIfAutoScroll() { - if (shouldAutoScrollToBottom()) { - scrollToBottom(); + if (this.isDisposed() || latestUserTurn == null || latestUserTurn.isDisposed()) { + return; } + if (!autoScrollEnabled) { + return; + } + scrollOffset = Integer.MAX_VALUE; + relayoutWindow(); } /** - * Flushes the cached layout sizes of the trailing (mutating) turns so the next - * {@code computeSize(width, DEFAULT, false)} re-measures them while sealed historical turns stay - * cached, keeping the layout pass O(1) in the number of historical turns. + * Drops the cached heights of the trailing (mutating) turns so they are re-measured next pass, while + * sealed historical turns keep their cached size. */ - private void flushTrailingTurnCaches() { - List dirty = new ArrayList<>(2); + private void invalidateTrailingTurnHeights() { if (latestUserTurn != null && !latestUserTurn.isDisposed()) { - dirty.add(latestUserTurn); + heightCache.remove(latestUserTurn); } if (latestCopilotTurn != null && !latestCopilotTurn.isDisposed()) { - dirty.add(latestCopilotTurn); + heightCache.remove(latestCopilotTurn); } - if (!dirty.isEmpty()) { - cmpContent.layout(dirty.toArray(new Control[0])); + if (errorWidget != null && !errorWidget.isDisposed()) { + heightCache.remove(errorWidget); + } + } + + /** Pins {@code cmpContent} to the current viewport rectangle so it is never grown or moved. */ + private void layoutContentArea() { + if (cmpContent == null || cmpContent.isDisposed()) { + return; } + Rectangle clientArea = this.getClientArea(); + cmpContent.setBounds(0, 0, Math.max(0, clientArea.width), Math.max(0, clientArea.height)); } /** - * Check if auto-scroll to bottom is needed. Only scroll when auto-scroll is enabled (user hasn't manually scrolled - * during response). + * The core windowing pass: measures every turn (cached), positions the ones intersecting the + * viewport in viewport-local coordinates, and parks the rest with {@code setVisible(false)} so no + * native child ever gets an out-of-range coordinate. */ - private boolean shouldAutoScrollToBottom() { - if (this.isDisposed() || latestUserTurn == null) { - return false; + private void relayoutWindow() { + if (this.isDisposed() || cmpContent == null || cmpContent.isDisposed()) { + return; + } + Rectangle clientArea = this.getClientArea(); + int width = clientArea.width; + int viewport = clientArea.height; + if (width <= 0 || viewport <= 0) { + return; } - if (!autoScrollEnabled) { - return false; + Control[] children = cmpContent.getChildren(); + int[] tops = new int[children.length]; + int[] heights = new int[children.length]; + boolean[] remeasured = new boolean[children.length]; + int running = 0; + int latestUserTop = -1; + for (int i = 0; i < children.length; i++) { + if (children[i] == latestUserTurn) { + latestUserTop = running; + } + boolean wasCached = heightCache.containsKey(children[i]); + int height = measuredHeight(children[i], width); + tops[i] = running; + heights[i] = height; + // A cache miss means the turn was (re)measured this pass: its width changed or its content + // mutated, so its internal GridLayout must be re-run. + remeasured[i] = !wasCached; + running += height; + } + int rawHeight = running; + + // Bottom padding (virtual, no widget): when the last round is shorter than the viewport, reserve + // whitespace below it so the latest user turn can pin to the top instead of floating mid-screen. + // Without it maxOffset is too small, so the new message cannot reach the top and "scroll to + // bottom" misaligns with the real maximum, breaking auto-scroll. + int bottomPadding = 0; + if (latestUserTop >= 0) { + int lastRoundHeight = rawHeight - latestUserTop; + if (lastRoundHeight < viewport) { + bottomPadding = viewport - lastRoundHeight; + } } + totalHeight = rawHeight + bottomPadding; + scrollOffset = clampOffset(scrollOffset); - Rectangle clientArea = this.getClientArea(); - // Use the freshly laid-out bounds rather than computeSize(): the incremental streaming pass - // repositions the trailing turns but does not flush their computeSize cache, so computeSize - // would return a stale height and the auto-scroll trigger would fire seconds too late. - // getBounds() reflects the just-applied layout. - int roundedHeight = currentTurnLaidOutHeight(); + for (int i = 0; i < children.length; i++) { + Control child = children[i]; + if (child.isDisposed()) { + continue; + } + int y = tops[i] - scrollOffset; + if (y + heights[i] > 0 && y < viewport) { + child.setBounds(0, y, width, heights[i]); + if (!child.getVisible()) { + child.setVisible(true); + } + // Run the turn's own layout so its GridLayout children (wrapped text, code blocks, footers) + // reflow. + if (remeasured[i] && child instanceof Composite composite) { + composite.layout(); + } + } else if (child.getVisible()) { + child.setVisible(false); + } + } - // Only auto-scroll when content height exceeds the visible area - return roundedHeight >= clientArea.height; + updateScrollBar(viewport); } - /** - * Returns the height of the latest (streaming) turn from its applied layout bounds. Falls back to - * {@code computeSize} only when bounds are not yet available (before the first layout pass). - */ - private int currentTurnLaidOutHeight() { - int height = 0; - if (latestUserTurn != null && !latestUserTurn.isDisposed()) { - int userHeight = latestUserTurn.getBounds().height; - height += userHeight > 0 ? userHeight : latestUserTurn.computeSize(SWT.DEFAULT, SWT.DEFAULT).y; + /** Returns the measured height of a row, using the identity cache when available. */ + private int measuredHeight(Control child, int width) { + if (child == null || child.isDisposed()) { + return 0; } - if (latestCopilotTurn != null && !latestCopilotTurn.isDisposed()) { - int copilotHeight = latestCopilotTurn.getBounds().height; - height += copilotHeight > 0 ? copilotHeight - : latestCopilotTurn.computeSize(SWT.DEFAULT, SWT.DEFAULT).y; + Integer cached = heightCache.get(child); + if (cached != null) { + return cached; } + int height = child.computeSize(width, SWT.DEFAULT, true).y; + heightCache.put(child, height); return height; } + private void updateScrollBar(int viewport) { + ScrollBar verticalBar = this.getVerticalBar(); + if (verticalBar == null) { + return; + } + if (totalHeight <= viewport) { + int safeViewport = Math.max(1, viewport); + verticalBar.setValues(0, 0, safeViewport, safeViewport, lineHeight(), safeViewport); + verticalBar.setEnabled(false); + return; + } + verticalBar.setEnabled(true); + verticalBar.setValues(scrollOffset, 0, totalHeight, viewport, lineHeight(), viewport); + } + + private int maxOffset() { + return Math.max(0, totalHeight - getClientArea().height); + } + + private int clampOffset(int offset) { + return Math.max(0, Math.min(offset, maxOffset())); + } + + private boolean isViewportAtBottom() { + return scrollOffset >= maxOffset() - SCROLL_THRESHOLD; + } + + /** One scroll "line" in pixels: the current font's line height. Cached until the font changes. */ + private int lineHeight() { + if (cachedLineHeight < 0) { + GC gc = new GC(this); + try { + gc.setFont(getFont()); + cachedLineHeight = Math.max(1, gc.getFontMetrics().getHeight()); + } finally { + gc.dispose(); + } + } + return cachedLineHeight; + } + + @Override + public void setFont(Font font) { + super.setFont(font); + cachedLineHeight = -1; + } + /** * Scroll to the bottom. */ private void scrollToBottom() { - ScrollBar verticalBar = this.getVerticalBar(); - if (verticalBar != null) { - this.setOrigin(0, verticalBar.getMaximum()); - } + autoScrollEnabled = true; + scrollOffset = Integer.MAX_VALUE; + relayoutWindow(); } /** @@ -666,24 +742,38 @@ private void scrollToBottom() { */ private void scrollToLatestUserTurn() { // Scroll to the bottom as a fallback. - if (latestUserTurn == null) { + if (latestUserTurn == null || latestUserTurn.isDisposed()) { scrollToBottom(); return; } - // Async so layout is computed before reading positions; reading synchronously scrolls to 0. + // Async so heights are measured before reading positions. SwtUtils.invokeOnDisplayThreadAsync(() -> { if (this.isDisposed() || latestUserTurn.isDisposed()) { return; } - Point turnLocation = latestUserTurn.getLocation(); - this.setOrigin(0, turnLocation.y); + scrollOffset = clampOffset(topOf(latestUserTurn)); + relayoutWindow(); }, this); } + /** Returns the cumulative top offset (in content coordinates) of the given row control. */ + private int topOf(Control target) { + int width = this.getClientArea().width; + int running = 0; + for (Control child : cmpContent.getChildren()) { + if (child == target) { + break; + } + running += measuredHeight(child, width); + } + return running; + } + @Override public void dispose() { pendingEvents.clear(); + heightCache.clear(); super.dispose(); for (BaseTurnWidget turn : turns.values()) { turn.dispose(); From de828cc7ac60f026d862ae86ef73727995a1424b Mon Sep 17 00:00:00 2001 From: Sheng Chen Date: Tue, 7 Jul 2026 15:49:46 +0800 Subject: [PATCH 13/14] fix: consolidate control validation into layoutNow (#340) * fix: consolidate control validation into layoutNow requestLayout and flushPendingLayoutRequests each filtered out null/ disposed controls via a shared validControls helper before delegating to layoutNow, duplicating the same defensive check at every call site. layoutNow now performs its own null/disposed filtering right where it dereferences the controls (getShell(), shell.layout(...)), so the invariant is enforced in exactly one place instead of trusted by two separate callers. requestLayout becomes a pure dispatcher: it no longer pre-filters, it just forwards the raw controls to pendingLayoutRequests or layoutNow. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * fix: repair syntax broken by autofix commit in requestLayout The prior "Potential fix for pull request finding" commit (04e76827) badly spliced a null-check into requestLayout, leaving a dangling duplicate `layoutNow(changed); }` outside the method body and dedenting the whole method. This left invalid Java that failed the Tycho/ECJ compile in CI. Restores correct syntax/indentation while keeping the null-guard around Collections.addAll(pendingLayoutRequests, changed), which is the same defensive check layoutNow already applies to its own `controls` parameter. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- .../copilot/eclipse/ui/chat/ChatView.java | 78 ++++++++++++++++++- .../eclipse/ui/chat/ReferencedFile.java | 9 ++- 2 files changed, 82 insertions(+), 5 deletions(-) diff --git a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/ChatView.java b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/ChatView.java index 3cf6ff19..c5aa62c6 100644 --- a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/ChatView.java +++ b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/ChatView.java @@ -4,7 +4,9 @@ package com.microsoft.copilot.eclipse.ui.chat; import java.util.ArrayList; +import java.util.Collections; import java.util.HashSet; +import java.util.LinkedHashSet; import java.util.List; import java.util.Map; import java.util.Set; @@ -32,6 +34,7 @@ import org.eclipse.swt.widgets.Composite; import org.eclipse.swt.widgets.Control; import org.eclipse.swt.widgets.Display; +import org.eclipse.swt.widgets.Shell; import org.eclipse.ui.IPartListener2; import org.eclipse.ui.IWorkbenchPartReference; import org.eclipse.ui.PlatformUI; @@ -164,6 +167,9 @@ public class ChatView extends ViewPart implements ChatProgressListener, MessageL private IContextActivation chatViewContextActivation; private IPartListener2 partListener; + /** Controls whose {@link #requestLayout(Control...)} call was suppressed while this view was hidden. */ + private final Set pendingLayoutRequests = new LinkedHashSet<>(); + @Override public void createPartControl(Composite parent) { this.parent = parent; @@ -517,7 +523,12 @@ public void partOpened(IWorkbenchPartReference partRef) { @Override public void partVisible(IWorkbenchPartReference partRef) { - // No action needed + // Replay layout requests skipped while this view was hidden behind another part in the same + // stack, targeted at just the specific control(s) that changed -- avoids a blind full-tree + // relayout of the (possibly long) chat conversation. + if (partRef.getPart(false) == ChatView.this) { + flushPendingLayoutRequests(); + } } }; @@ -1702,16 +1713,75 @@ public void dispose() { getSite().getPage().removePartListener(this.partListener); this.partListener = null; } + pendingLayoutRequests.clear(); deactivateChatViewContext(); super.dispose(); } /** - * Layout the view. + * Request a layout of the given control(s), which must be part of this view's widget tree. + * + *

Passing every control whose content actually changed (rather than a blind {@code + * parent.layout(true, true)}) lets SWT flush exactly those controls' cached sizes -- and only the + * ancestor chains leading to them -- so it never touches unrelated subtrees like the chat + * conversation history. Passing only a subset of what changed (e.g. a text label but not a + * sibling icon label that also changed) will leave the omitted control's cached size stale. + * + *

Skipped while the view isn't visible (e.g. stacked behind another part) to avoid layout cost + * on every editor switch; {@code changed} is instead remembered and replayed by {@link + * #flushPendingLayoutRequests()} once the view becomes visible again. + * + * @param changed every control whose content/layout data just changed + */ + public void requestLayout(Control... changed) { + if (getSite() == null || getSite().getPage() == null || !getSite().getPage().isPartVisible(this)) { + if (changed != null) { + Collections.addAll(pendingLayoutRequests, changed); + } + return; + } + layoutNow(changed); + } + + /** + * Replays layout requests that were suppressed by {@link #requestLayout(Control...)} while this + * view was hidden, targeted at just the specific control(s) that changed. */ - public void layout(boolean changed, boolean all) { - parent.layout(changed, all); + private void flushPendingLayoutRequests() { + if (pendingLayoutRequests.isEmpty()) { + return; + } + // Snapshot and clear before replaying: a control's requestLayout() could in principle + // synchronously trigger a new call back into requestLayout(Control...), which would otherwise + // mutate pendingLayoutRequests while it's being iterated. + Set toFlush = new LinkedHashSet<>(pendingLayoutRequests); + pendingLayoutRequests.clear(); + layoutNow(toFlush.toArray(new Control[0])); + } + + /** + * Flushes cached sizes for exactly the given controls (and the ancestor chains leading to them) + * and lays them out, in a single batched pass. Silently ignores {@code null} or disposed + * controls. + */ + private void layoutNow(Control... controls) { + List valid = new ArrayList<>(); + if (controls != null) { + for (Control control : controls) { + if (control != null && !control.isDisposed()) { + valid.add(control); + } + } + } + if (valid.isEmpty()) { + return; + } + Shell shell = valid.get(0).getShell(); + if (shell == null || shell.isDisposed()) { + return; + } + shell.layout(valid.toArray(new Control[0]), SWT.DEFER); } /** diff --git a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/ReferencedFile.java b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/ReferencedFile.java index db63a981..e1bf9b25 100644 --- a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/ReferencedFile.java +++ b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/ReferencedFile.java @@ -170,7 +170,14 @@ protected void setFile(@Nullable IResource file) { setLayoutData(layoutData); ChatView chatView = UiUtils.getView(Constants.CHAT_VIEW_ID, ChatView.class); if (chatView != null) { - chatView.layout(true, true); + // Pass every child whose content setFile()/setupXDisplay() can touch (icon image, file name + // text/CSS class, close button image) -- not just this composite or a single child. SWT's + // targeted requestLayout() only flushes cached sizes for the exact controls it's given (plus + // their ancestor chains up to cmpFileRef and beyond); passing a subset silently leaves the + // others' cached sizes stale (previously: a clipped icon, and before that a chip that didn't + // shrink). Since all three are descendants of this composite, their ancestor walk-up already + // covers this chip's own RowData/visibility change too -- no need to pass `this` separately. + chatView.requestLayout(lblfileIcon, lblFileName, lblClose); } } From 63b1303eb611aeddde2ed05958c44c60d563fd22 Mon Sep 17 00:00:00 2001 From: Raghunandana Date: Tue, 7 Jul 2026 13:34:22 +0200 Subject: [PATCH 14/14] In Agent mode: Auto scroll to prompts like 'Continue'. (#26) * In Agent mode: Auto scroll to prompts like 'Continue'. User must be made aware some action is needed from them to continue the work by agent. Usually this happens when "Too many requests" or "Command line run prompt" etc. see https://github.com/microsoft/copilot-eclipse-feedback/issues/184 * Fix Checkstyle violations in BaseTurnWidget - Wrap the WarnWidget construction line that exceeded 120 characters - Fix indentation of the return statement to match the enclosing block These were causing the CI build (Checkstyle) to fail on PR #26. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Height adjustment review fix. We can use the target control height directly. No need to calculate by hint. --------- Co-authored-by: Sheng Chen Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../eclipse/ui/chat/BaseTurnWidget.java | 25 ++++++-- .../eclipse/ui/chat/ChatContentViewer.java | 60 ++++++++++++++++++- .../copilot/eclipse/ui/utils/SwtUtils.java | 22 +++++++ 3 files changed, 102 insertions(+), 5 deletions(-) diff --git a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/BaseTurnWidget.java b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/BaseTurnWidget.java index 1218ce5a..9f099223 100644 --- a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/BaseTurnWidget.java +++ b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/BaseTurnWidget.java @@ -23,6 +23,7 @@ import org.eclipse.ui.PlatformUI; import org.osgi.service.event.EventHandler; +import com.microsoft.copilot.eclipse.core.Constants; import com.microsoft.copilot.eclipse.core.CopilotCore; import com.microsoft.copilot.eclipse.core.chat.ConfirmationContent; import com.microsoft.copilot.eclipse.core.events.CopilotEventConstants; @@ -36,6 +37,7 @@ import com.microsoft.copilot.eclipse.core.persistence.CopilotTurnData.EditAgentRoundData; import com.microsoft.copilot.eclipse.core.persistence.CopilotTurnData.ReplyData; import com.microsoft.copilot.eclipse.core.persistence.CopilotTurnData.ToolCallData; +import com.microsoft.copilot.eclipse.ui.CopilotUi; import com.microsoft.copilot.eclipse.ui.chat.services.AvatarService; import com.microsoft.copilot.eclipse.ui.chat.services.ChatServiceManager; import com.microsoft.copilot.eclipse.ui.utils.SwtUtils; @@ -604,16 +606,16 @@ protected void ensureFooterAtBottom() { * @param code the server error code * @param modelProviderName the BYOK model-provider name, or {@code null} for built-in models */ - protected void createWarnDialog(String message, int code, String modelProviderName) { + protected Composite createWarnDialog(String message, int code, String modelProviderName) { // TODO: Remove this legacy fallback after TBB is officially released. // When the language server has not enabled token-based billing yet, restore the original // main-branch warning behavior (no plan-driven actions; single upgrade button on the legacy // 30-day free trial message). if (!this.serviceManager.getAuthStatusManager().getQuotaStatus().tokenBasedBillingEnabled()) { - new WarnWidget(this, SWT.BOTTOM, message, code); + WarnWidget warnWidget = new WarnWidget(this, SWT.BOTTOM, message, code); ensureFooterAtBottom(); requestLayout(); - return; + return warnWidget; } boolean byokQuotaExceeded = QuotaActions.isByokQuotaExceeded(code, modelProviderName); String displayMessage = byokQuotaExceeded ? Messages.chat_warnWidget_byokQuotaUsageMessage : message; @@ -627,9 +629,11 @@ protected void createWarnDialog(String message, int code, String modelProviderNa && quotaStatus.premiumInteractions().overagePermitted(); canUpgradePlan = quotaStatus.canUpgradePlan(); } - new WarnWidget(this, SWT.NONE, displayMessage, planForActions, overageEnabled, canUpgradePlan); + WarnWidget warnWidget = + new WarnWidget(this, SWT.NONE, displayMessage, planForActions, overageEnabled, canUpgradePlan); ensureFooterAtBottom(); requestLayout(); + return warnWidget; } /** @@ -666,6 +670,19 @@ public CompletableFuture requestToolExecuti this.getParent().requestLayout(); + // Ensure the chat content viewer scrolls to show the newly created confirmation + // dialog. Walk up the composite hierarchy to find a ChatContentViewer + // and request scrolling. Use async exec because layout needs to complete first. + SwtUtils.invokeOnDisplayThreadAsync(() -> { + ChatContentViewer viewer = SwtUtils.findParentOfType(this.getParent(), ChatContentViewer.class); + if (viewer != null) { + if (this.confirmDialog != null && !this.confirmDialog.isDisposed()) { + viewer.showControl(this.confirmDialog); + } + } + + }, this.getParent()); + return toolConfirmationFuture; } diff --git a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/ChatContentViewer.java b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/ChatContentViewer.java index 83e197e7..b3db64b4 100644 --- a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/ChatContentViewer.java +++ b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/ChatContentViewer.java @@ -468,9 +468,17 @@ public BaseTurnWidget getTurnWidget(String turnId) { } private void renderWarnMessageWithUpgradePlanButton(String errorMessage, int code, String modelProviderName) { - latestTurnWidget.createWarnDialog(errorMessage, code, modelProviderName); + Composite warnWidget = latestTurnWidget.createWarnDialog(errorMessage, code, modelProviderName); refreshLayoutFull(); scrollToLatestUserTurn(); + // Ensure the chat content viewer scrolls to show the newly created warning banner. Walk up the composite hierarchy + // to find a ChatContentViewer and request scrolling. Use async exec because layout needs to complete first. + SwtUtils.invokeOnDisplayThreadAsync(() -> { + if (warnWidget != null && !warnWidget.isDisposed()) { + showControl(warnWidget); + } + + }, this.getParent()); } /** @@ -483,6 +491,12 @@ public void renderErrorMessage(String errorMessage) { this.errorWidget = new ErrorWidget(cmpContent, SWT.BOTTOM, errorMessage); refreshLayoutFull(); scrollToLatestUserTurn(); + // Ensure the chat content viewer scrolls to show the newly created error banner. + SwtUtils.invokeOnDisplayThreadAsync(() -> { + if (this.errorWidget != null && !this.errorWidget.isDisposed()) { + this.showControl(this.errorWidget); + } + }, this.getParent()); } /** @@ -770,6 +784,50 @@ private int topOf(Control target) { return running; } + /** + * Scrolls the viewport to make {@code target} visible, equivalent to + * {@link org.eclipse.swt.custom.ScrolledComposite#showControl(Control)}. + * + *

Walks up the widget tree to find the direct child of {@code cmpContent} that contains + * {@code target}, computes the content-coordinate position of {@code target} by summing + * the turn's {@link #topOf} value with the local y offsets down to {@code target}, then + * adjusts {@link #scrollOffset} by the minimum amount needed to bring {@code target} fully + * into the viewport.

+ */ + public void showControl(Composite target) { + if (target == null || target.isDisposed()) { + return; + } + // Walk up to find the direct child of cmpContent that is the ancestor of target. + Control ancestor = target; + while (ancestor != null && ancestor.getParent() != cmpContent) { + ancestor = ancestor.getParent(); + } + if (ancestor == null || ancestor.getParent() != cmpContent) { + return; + } + // Content-coordinate top of the enclosing turn widget. + int ancestorTop = topOf(ancestor); + // Accumulate the local y offset by walking from target up to (but not including) ancestor. + int localY = 0; + for (Control c = target; c != ancestor; c = c.getParent()) { + localY += c.getLocation().y; + } + int targetTop = ancestorTop + localY; + int targetBottom = targetTop + target.getSize().y; + int viewport = getClientArea().height; + // Scroll the minimum amount: down if target is below the visible area, up if above. + int newOffset = scrollOffset; + if (targetBottom > scrollOffset + viewport) { + newOffset = targetBottom - viewport; + } + if (targetTop < newOffset) { + newOffset = targetTop; + } + scrollOffset = clampOffset(newOffset); + relayoutWindow(); + } + @Override public void dispose() { pendingEvents.clear(); diff --git a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/utils/SwtUtils.java b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/utils/SwtUtils.java index 0a054490..73180b8c 100644 --- a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/utils/SwtUtils.java +++ b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/utils/SwtUtils.java @@ -48,8 +48,30 @@ private SwtUtils() { } private static final String INLINE_ANNOTATION_COLOR_KEY = "org.eclipse.ui.editors.inlineAnnotationColor"; + private static final int DEFAULT_GHOST_TEXT_SCALE = 128; + /** + * Walks up the parent chain of the given control and returns the first ancestor that is an instance of the specified + * type, or {@code null} if none is found. + * + * @param the target type + * @param control the starting control (may be {@code null}) + * @param type the class to search for + * @return the first matching ancestor, or {@code null} + */ + @Nullable + public static T findParentOfType(Control control, Class type) { + Control current = control; + while (current != null) { + if (type.isInstance(current)) { + return type.cast(current); + } + current = current.getParent(); + } + return null; + } + /** * Invokes the given runnable on the display thread. */