From 229ebd5f7daa51a7e0fdbfe3dcc2e8d8ba40df6e Mon Sep 17 00:00:00 2001 From: xinyi-gong Date: Mon, 11 May 2026 14:16:06 +0800 Subject: [PATCH 1/8] terminal auto approve --- .../copilot/eclipse/core/Constants.java | 4 + .../eclipse/core/chat/ConfirmationAction.java | 110 ++++ .../core/chat/ConfirmationActionScope.java | 16 + .../core/chat/ConfirmationContent.java | 77 +++ .../eclipse/core/chat/ConfirmationResult.java | 71 ++- .../core/chat/TerminalAutoApproveRule.java | 77 +++ .../lsp/protocol/CopilotAgentSettings.java | 61 ++- .../terminal-auto-approve.md | 365 +++++++++++++ .../TerminalConfirmationHandlerTests.java | 478 ++++++++++++++++++ .../META-INF/MANIFEST.MF | 1 + com.microsoft.copilot.eclipse.ui/css/dark.css | 2 +- .../css/light.css | 2 +- .../plugin.properties | 3 +- com.microsoft.copilot.eclipse.ui/plugin.xml | 14 +- .../eclipse/ui/chat/BaseTurnWidget.java | 18 +- .../ui/chat/InvokeToolConfirmationDialog.java | 352 +++++++------ .../copilot/eclipse/ui/chat/Messages.java | 16 + .../confirmation/ConfirmationHandler.java | 42 ++ .../confirmation/ConfirmationService.java | 122 ++++- .../TerminalConfirmationHandler.java | 418 +++++++++++++++ .../eclipse/ui/chat/messages.properties | 16 + .../ui/chat/services/AgentToolService.java | 31 +- .../eclipse/ui/dialogs/mcp/McpServerItem.java | 9 +- .../ui/preferences/AddTerminalRuleDialog.java | 103 ++++ .../AutoApprovePreferencePage.java | 55 ++ .../CopilotPreferenceInitializer.java | 7 + .../preferences/CopilotPreferencesPage.java | 2 + .../LanguageServerSettingManager.java | 41 ++ .../eclipse/ui/preferences/Messages.java | 19 + .../TerminalAutoApproveSection.java | 273 ++++++++++ .../ui/preferences/messages.properties | 20 + .../SplitDropdownButton.java} | 37 +- .../eclipse/ui/utils/PreferencesUtils.java | 3 +- 33 files changed, 2666 insertions(+), 199 deletions(-) create mode 100644 com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/chat/ConfirmationAction.java create mode 100644 com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/chat/ConfirmationActionScope.java create mode 100644 com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/chat/ConfirmationContent.java create mode 100644 com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/chat/TerminalAutoApproveRule.java create mode 100644 com.microsoft.copilot.eclipse.swtbot.test/test-plans/terminal-auto-approve/terminal-auto-approve.md create mode 100644 com.microsoft.copilot.eclipse.ui.test/src/com/microsoft/copilot/eclipse/ui/chat/confirmation/TerminalConfirmationHandlerTests.java create mode 100644 com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/confirmation/ConfirmationHandler.java create mode 100644 com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/confirmation/TerminalConfirmationHandler.java create mode 100644 com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/preferences/AddTerminalRuleDialog.java create mode 100644 com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/preferences/AutoApprovePreferencePage.java create mode 100644 com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/preferences/TerminalAutoApproveSection.java rename com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/{dialogs/mcp/DropDownButton.java => swt/SplitDropdownButton.java} (88%) diff --git a/com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/Constants.java b/com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/Constants.java index 7e2ae6ca..188f6bad 100644 --- a/com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/Constants.java +++ b/com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/Constants.java @@ -49,6 +49,10 @@ private Constants() { public static final String GITHUB_JOBS_VIEW_ID = "com.microsoft.copilot.eclipse.ui.jobs.JobsView"; public static final String SUPPRESS_TERMINAL_DEPENDENCY_DIALOG = "suppressTerminalDependencyDialog"; + // Auto-Approve settings + public static final String AUTO_APPROVE_TERMINAL_RULES = "autoApproveTerminalRules"; + public static final String AUTO_APPROVE_UNMATCHED_TERMINAL = "autoApproveUnmatchedTerminal"; + // Base excluded file types shared by both // Copied from InelliJ, excluded file extension list // https://github.com/microsoft/copilot-intellij/blob/main/core/src/main/kotlin/com/github/copilot/chat/references/FileSearchService.kt diff --git a/com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/chat/ConfirmationAction.java b/com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/chat/ConfirmationAction.java new file mode 100644 index 00000000..07f10d92 --- /dev/null +++ b/com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/chat/ConfirmationAction.java @@ -0,0 +1,110 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + +package com.microsoft.copilot.eclipse.core.chat; + +import java.util.Map; +import java.util.Objects; + +import org.apache.commons.lang3.builder.ToStringBuilder; + +/** + * Represents a button action in the confirmation UI. Each action has a label, + * a decision (accept or dismiss), a persistence scope, and optional metadata + * for the handler to know what to persist. + */ +public class ConfirmationAction { + + /** Metadata key for the action type enum name. */ + public static final String META_ACTION = "action"; + + private final String label; + private final boolean accept; + private final ConfirmationActionScope scope; + private final Map metadata; + private final boolean primary; + + /** + * Creates a new confirmation action. + * + * @param label the button label + * @param accept true for accept, false for dismiss + * @param scope the persistence scope (null for dismiss actions) + * @param metadata extra data for the handler (e.g., command names, server name) + * @param primary whether this is the primary/default button + */ + public ConfirmationAction(String label, boolean accept, + ConfirmationActionScope scope, Map metadata, + boolean primary) { + this.label = label; + this.accept = accept; + this.scope = scope; + this.metadata = metadata != null ? metadata : Map.of(); + this.primary = primary; + } + + public String getLabel() { + return label; + } + + public boolean isAccept() { + return accept; + } + + public ConfirmationActionScope getScope() { + return scope; + } + + public Map getMetadata() { + return metadata; + } + + public boolean isPrimary() { + return primary; + } + + /** Creates a primary accept action (scope = ONCE). */ + public static ConfirmationAction allowOnce(String label) { + return new ConfirmationAction(label, true, + ConfirmationActionScope.ONCE, null, true); + } + + /** Creates a dismiss action. */ + public static ConfirmationAction skip(String label) { + return new ConfirmationAction(label, false, null, null, false); + } + + @Override + public int hashCode() { + return Objects.hash(accept, label, metadata, primary, scope); + } + + @Override + public boolean equals(Object obj) { + if (this == obj) { + return true; + } + if (obj == null) { + return false; + } + if (getClass() != obj.getClass()) { + return false; + } + ConfirmationAction other = (ConfirmationAction) obj; + return accept == other.accept + && Objects.equals(label, other.label) + && Objects.equals(metadata, other.metadata) + && primary == other.primary && scope == other.scope; + } + + @Override + public String toString() { + return new ToStringBuilder(this) + .append("label", label) + .append("accept", accept) + .append("scope", scope) + .append("metadata", metadata) + .append("primary", primary) + .toString(); + } +} diff --git a/com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/chat/ConfirmationActionScope.java b/com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/chat/ConfirmationActionScope.java new file mode 100644 index 00000000..37fc6d88 --- /dev/null +++ b/com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/chat/ConfirmationActionScope.java @@ -0,0 +1,16 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + +package com.microsoft.copilot.eclipse.core.chat; + +/** + * Scope of how a confirmation decision should be persisted. + */ +public enum ConfirmationActionScope { + /** One-time acceptance for this single invocation. */ + ONCE, + /** Remember for the current conversation session (in-memory). */ + SESSION, + /** Remember globally (application-level persistent, synced to CLS). */ + GLOBAL +} diff --git a/com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/chat/ConfirmationContent.java b/com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/chat/ConfirmationContent.java new file mode 100644 index 00000000..abc588fc --- /dev/null +++ b/com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/chat/ConfirmationContent.java @@ -0,0 +1,77 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + +package com.microsoft.copilot.eclipse.core.chat; + +import java.util.List; +import java.util.Objects; + +import org.apache.commons.lang3.builder.ToStringBuilder; + +/** + * Complete content for rendering a confirmation UI. Returned by handlers when a tool call + * needs user confirmation. Contains the display text and the list of action buttons. + */ +public class ConfirmationContent { + + private final String title; + private final String message; + private final List actions; + + /** + * Creates a new confirmation content. + * + * @param title bold title text displayed at the top + * @param message description text (may be null) + * @param actions list of button actions for the confirmation UI + */ + public ConfirmationContent(String title, String message, + List actions) { + this.title = title; + this.message = message; + this.actions = actions; + } + + public String getTitle() { + return title; + } + + public String getMessage() { + return message; + } + + public List getActions() { + return actions; + } + + @Override + public int hashCode() { + return Objects.hash(actions, message, title); + } + + @Override + public boolean equals(Object obj) { + if (this == obj) { + return true; + } + if (obj == null) { + return false; + } + if (getClass() != obj.getClass()) { + return false; + } + ConfirmationContent other = (ConfirmationContent) obj; + return Objects.equals(actions, other.actions) + && Objects.equals(message, other.message) + && Objects.equals(title, other.title); + } + + @Override + public String toString() { + return new ToStringBuilder(this) + .append("title", title) + .append("message", message) + .append("actions", actions) + .toString(); + } +} diff --git a/com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/chat/ConfirmationResult.java b/com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/chat/ConfirmationResult.java index a702256a..89199cec 100644 --- a/com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/chat/ConfirmationResult.java +++ b/com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/chat/ConfirmationResult.java @@ -3,17 +3,68 @@ package com.microsoft.copilot.eclipse.core.chat; +import java.util.Objects; + +import org.apache.commons.lang3.builder.ToStringBuilder; + /** * Result of evaluating an auto-approve confirmation request. + * Either AUTO_APPROVED (no UI needed) or NEEDS_CONFIRMATION with content for the dialog. */ -public enum ConfirmationResult { - /** - * The tool invocation is automatically approved and no user confirmation is needed. - */ - AUTO_APPROVED, - - /** - * The tool invocation requires user confirmation before proceeding. - */ - NEEDS_CONFIRMATION +public class ConfirmationResult { + + /** Auto-approved, no user confirmation needed. */ + public static final ConfirmationResult AUTO_APPROVED = new ConfirmationResult(true, null); + + private final boolean autoApproved; + private final ConfirmationContent content; + + private ConfirmationResult(boolean autoApproved, ConfirmationContent content) { + this.autoApproved = autoApproved; + this.content = content; + } + + /** Creates a result that requires user confirmation with the given content. */ + public static ConfirmationResult needsConfirmation( + ConfirmationContent content) { + return new ConfirmationResult(false, content); + } + + public boolean isAutoApproved() { + return autoApproved; + } + + /** Returns the confirmation content, or null if auto-approved or using defaults. */ + public ConfirmationContent getContent() { + return content; + } + + @Override + public int hashCode() { + return Objects.hash(autoApproved, content); + } + + @Override + public boolean equals(Object obj) { + if (this == obj) { + return true; + } + if (obj == null) { + return false; + } + if (getClass() != obj.getClass()) { + return false; + } + ConfirmationResult other = (ConfirmationResult) obj; + return autoApproved == other.autoApproved + && Objects.equals(content, other.content); + } + + @Override + public String toString() { + return new ToStringBuilder(this) + .append("autoApproved", autoApproved) + .append("content", content) + .toString(); + } } diff --git a/com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/chat/TerminalAutoApproveRule.java b/com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/chat/TerminalAutoApproveRule.java new file mode 100644 index 00000000..b70a2797 --- /dev/null +++ b/com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/chat/TerminalAutoApproveRule.java @@ -0,0 +1,77 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + +package com.microsoft.copilot.eclipse.core.chat; + +import java.util.Objects; + +import org.apache.commons.lang3.builder.ToStringBuilder; + +/** + * A single terminal auto-approve rule mapping a command name or regex pattern to an + * allow/deny decision. + */ +public class TerminalAutoApproveRule { + private String command; + private boolean autoApprove; + + /** + * Creates a new rule. + * + * @param command the command name or regex pattern + * @param autoApprove true to auto-approve, false to always require confirmation + */ + public TerminalAutoApproveRule(String command, boolean autoApprove) { + this.command = command; + this.autoApprove = autoApprove; + } + + /** Default constructor for Gson deserialization. */ + public TerminalAutoApproveRule() { + } + + public String getCommand() { + return command; + } + + public void setCommand(String command) { + this.command = command; + } + + public boolean isAutoApprove() { + return autoApprove; + } + + public void setAutoApprove(boolean autoApprove) { + this.autoApprove = autoApprove; + } + + @Override + public int hashCode() { + return Objects.hash(autoApprove, command); + } + + @Override + public boolean equals(Object obj) { + if (this == obj) { + return true; + } + if (obj == null) { + return false; + } + if (getClass() != obj.getClass()) { + return false; + } + TerminalAutoApproveRule other = (TerminalAutoApproveRule) obj; + return autoApprove == other.autoApprove + && Objects.equals(command, other.command); + } + + @Override + public String toString() { + return new ToStringBuilder(this) + .append("command", command) + .append("autoApprove", autoApprove) + .toString(); + } +} diff --git a/com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/lsp/protocol/CopilotAgentSettings.java b/com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/lsp/protocol/CopilotAgentSettings.java index be30e612..5fb2e362 100644 --- a/com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/lsp/protocol/CopilotAgentSettings.java +++ b/com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/lsp/protocol/CopilotAgentSettings.java @@ -3,6 +3,7 @@ package com.microsoft.copilot.eclipse.core.lsp.protocol; +import java.util.Map; import java.util.Objects; import com.google.gson.annotations.SerializedName; @@ -16,9 +17,40 @@ public class CopilotAgentSettings { @SerializedName("maxToolCallingLoop") private int agentMaxRequests; - // Control if the editor handles all confirmation requests from CLS. + @SerializedName("autoApproveUnmatchedTerminal") + private boolean autoApproveUnmatchedTerminal; + + // Tells CLS to always send confirmation requests to the editor @SerializedName("editorHandlesAllConfirmation") - private boolean editorHandlesAllConfirmation; + private boolean editorHandlesAllConfirmation = true; + + private ToolsSettings tools; + + /** Nested tools settings matching CLS agent.tools structure. */ + public static class ToolsSettings { + private TerminalSettings terminal; + + /** Gets terminal settings, creating if needed. */ + public TerminalSettings getTerminal() { + if (terminal == null) { + terminal = new TerminalSettings(); + } + return terminal; + } + } + + /** Terminal auto-approve rules: command/pattern → allow(true)/deny(false). */ + public static class TerminalSettings { + private Map autoApprove; + + public Map getAutoApprove() { + return autoApprove; + } + + public void setAutoApprove(Map autoApprove) { + this.autoApprove = autoApprove; + } + } public int getAgentMaxRequests() { return agentMaxRequests; @@ -28,17 +60,26 @@ public void setAgentMaxRequests(int agentMaxRequests) { this.agentMaxRequests = agentMaxRequests; } - public boolean isEditorHandlesAllConfirmation() { - return editorHandlesAllConfirmation; + public boolean isAutoApproveUnmatchedTerminal() { + return autoApproveUnmatchedTerminal; } - public void setEditorHandlesAllConfirmation(boolean editorHandlesAllConfirmation) { - this.editorHandlesAllConfirmation = editorHandlesAllConfirmation; + public void setAutoApproveUnmatchedTerminal(boolean autoApproveUnmatchedTerminal) { + this.autoApproveUnmatchedTerminal = autoApproveUnmatchedTerminal; + } + + /** Gets tools settings, creating if needed. */ + public ToolsSettings getTools() { + if (tools == null) { + tools = new ToolsSettings(); + } + return tools; } @Override public int hashCode() { - return Objects.hash(agentMaxRequests, editorHandlesAllConfirmation); + return Objects.hash(agentMaxRequests, + autoApproveUnmatchedTerminal, tools); } @Override @@ -51,14 +92,16 @@ public boolean equals(Object obj) { } CopilotAgentSettings other = (CopilotAgentSettings) obj; return agentMaxRequests == other.agentMaxRequests - && editorHandlesAllConfirmation == other.editorHandlesAllConfirmation; + && autoApproveUnmatchedTerminal == other.autoApproveUnmatchedTerminal + && Objects.equals(tools, other.tools); } @Override public String toString() { ToStringBuilder builder = new ToStringBuilder(this); builder.append("agentMaxRequests", agentMaxRequests); - builder.append("editorHandlesAllConfirmation", editorHandlesAllConfirmation); + builder.append("autoApproveUnmatchedTerminal", autoApproveUnmatchedTerminal); + builder.append("tools", tools); return builder.toString(); } } diff --git a/com.microsoft.copilot.eclipse.swtbot.test/test-plans/terminal-auto-approve/terminal-auto-approve.md b/com.microsoft.copilot.eclipse.swtbot.test/test-plans/terminal-auto-approve/terminal-auto-approve.md new file mode 100644 index 00000000..f988ec22 --- /dev/null +++ b/com.microsoft.copilot.eclipse.swtbot.test/test-plans/terminal-auto-approve/terminal-auto-approve.md @@ -0,0 +1,365 @@ +# Terminal Auto Approve + +## Overview +Tests the terminal command auto-approve feature end-to-end: configuring rules +in the preference page, then triggering Agent Mode tool calls and observing +whether the confirmation dialog appears or the command runs automatically. + +Each test case exercises the full stack: preference store → CLS sync → +Agent Mode prompt → tool confirmation request → `ConfirmationService` → +`TerminalConfirmationHandler` → dialog (or auto-approve) → terminal +execution. This mirrors the real user workflow: tweak settings, chat with +Copilot, observe behavior. + +Entry points exercised: +- **Preferences → GitHub Copilot → Tool Auto Approve** — the terminal + rule table (add / remove / toggle / reset). +- **Agent Mode chat** — sending prompts that trigger `run_in_terminal` + tool calls. +- **Confirmation dialog** — the split-dropdown button with session/global + allow actions. + +--- + +## Prerequisites + +- Eclipse IDE with the GitHub Copilot for Eclipse plugin installed and + activated. +- A signed-in Copilot account on the host machine. +- Network access to `api.githubcopilot.com`. +- **Agent Mode** selected in the chat mode dropdown. +- No previous auto-approve rules beyond the defaults (reset via + "Reset to Defaults" before each scenario). + +--- + +## 1. Default deny rules block dangerous commands + +### TC-001: Default deny rules + Agent Mode terminal call + +**Type:** `Happy Path` +**Priority:** `P0` + +#### Preconditions +- Default rules are active (not modified). +- "Auto approve commands not covered by rules" is **unchecked**. + +#### Steps +1. Open **Preferences → GitHub Copilot → Tool Auto Approve**. +2. Verify the "Terminal Auto Approve" section is visible with a table + showing default deny rules (rm, rmdir, del, kill, curl, wget, eval, + chmod, chown, and the regex rules for subshells / backticks / braces). +3. Confirm "Auto approve commands not covered by rules" is unchecked. +4. Close preferences. +5. Open the **Copilot Chat** view and select **Agent** mode. +6. Wait for the model picker to resolve. +7. Type the prompt: `please run curl https://example.com`. +8. Wait for a Copilot turn to stream — the agent should invoke the + `run_in_terminal` tool with a `curl` command. +9. Observe the confirmation dialog that appears in the chat panel. +10. Verify the dialog shows: + - Bold title: **"Run command in terminal"** + - Message: *"The tool is about to run the following command in the + terminal."* + - The command text in a scrollable panel with `bg-command-panel` + background. + - A blue **"Allow Once ▾"** split-dropdown button and a **"Skip"** + button. +11. Click the dropdown arrow on "Allow Once ▾". +12. Verify the dropdown menu contains: + - "Allow 'curl' in this Session" + - "Always Allow 'curl'" + - "Allow this exact command in this Session" (if the full command + differs from "curl") + - "Always Allow this exact command" + - "Allow all commands in this Session" +13. Click **"Skip"**. +14. Verify the command was NOT executed — the agent receives a dismiss + result and continues without terminal output. + +#### Expected Result +- Default deny rule for `curl` causes the confirmation dialog to appear. +- The dialog renders correctly with split-dropdown actions. +- Skipping prevents execution. + +#### 📸 Key Screenshots +- [ ] Preference page with default rules visible. +- [ ] Confirmation dialog with dropdown expanded showing all actions. +- [ ] Agent turn after skip — no terminal output. + +--- + +## 2. Custom allow rule auto-approves matching commands + +### TC-002: Add allow rule → Agent auto-approves → verify execution + +**Type:** `Happy Path` +**Priority:** `P0` + +#### Steps +1. Open **Preferences → GitHub Copilot → Tool Auto Approve**. +2. Click **"Add..."** in the Terminal Auto Approve section. +3. Enter `systeminfo` as command, select **"Allow"**, click OK. +4. Verify `systeminfo` appears in the table with "Allow" status. +5. Click **"Apply and Close"**. +6. In Agent Mode chat, type: `run systeminfo to show my computer info`. +7. Wait for the agent to invoke `run_in_terminal`. +8. Observe that **no confirmation dialog appears** — the command runs + directly. +9. Wait for the tool call to complete — the agent should report terminal + output containing system information. +10. Open preferences again and verify the rule is still present. + +#### Expected Result +- The custom allow rule causes the `systeminfo` command to auto-approve. +- No confirmation dialog is shown. +- The terminal runs the command and the agent receives output. + +#### 📸 Key Screenshots +- [ ] Preference page with `systeminfo` Allow rule added. +- [ ] Agent turn showing "✔ Ran run_in_terminal tool" without a + confirmation dialog in between. +- [ ] Agent response containing system information output. + +--- + +## 3. Session "Allow command name" persists within conversation + +### TC-003: Allow name in Session → same command auto-approves → new +conversation resets + +**Type:** `Happy Path` +**Priority:** `P0` + +#### Steps +1. Ensure no custom rules for `echo` exist (only defaults). +2. In Agent Mode, type: `run echo hello world`. +3. Confirmation dialog appears (echo has no matching rule and unmatched + is disabled). +4. Click the dropdown arrow and select **"Allow 'echo' in this Session"**. +5. The command executes. +6. In the **same conversation**, type: `run echo second message`. +7. Observe that **no confirmation dialog appears** — the command + auto-approves because `echo` is session-approved. +8. Verify the agent shows terminal output for both commands. +9. Start a **new conversation** (click "New Chat" or equivalent). +10. Type: `run echo third message`. +11. Observe that the **confirmation dialog appears again** — session + approvals do not carry over to new conversations. + +#### Expected Result +- First invocation: dialog shown, user selects session allow. +- Second invocation (same conversation): auto-approved. +- Third invocation (new conversation): dialog shown again. + +#### 📸 Key Screenshots +- [ ] First dialog with dropdown showing "Allow 'echo' in this Session". +- [ ] Second invocation auto-approved — no dialog. +- [ ] New conversation — dialog reappears. + +--- + +## 4. "Always Allow" persists globally + +### TC-004: Always Allow command name → persists across conversations +and appears in preferences + +**Type:** `Happy Path` +**Priority:** `P0` + +#### Steps +1. Ensure no custom rules for `dir` exist. +2. In Agent Mode, type: `list files in current directory`. +3. Confirmation dialog appears for the `dir` (or `ls`) command. +4. Click the dropdown and select **"Always Allow 'dir'"**. +5. The command executes. +6. Open **Preferences → Tool Auto Approve**. +7. Verify `dir` appears in the rules table with **"Allow"** status. +8. Close preferences. +9. Start a **new conversation**. +10. Type: `show me what files are here` (triggers `dir` again). +11. Observe that **no confirmation dialog appears** — the global rule + persists. + +#### Expected Result +- The "Always Allow" action writes the command name to the preference + store. +- The rule appears in the preference page UI. +- The rule survives across conversations. + +#### 📸 Key Screenshots +- [ ] Dropdown selection: "Always Allow 'dir'". +- [ ] Preference page showing `dir` as Allow rule. +- [ ] New conversation: `dir` auto-approved. + +--- + +## 5. Exact command vs. command name distinction + +### TC-005: Exact command Session approval only matches the same +command line + +**Type:** `Edge Case` +**Priority:** `P1` + +#### Steps +1. In Agent Mode, type: `run echo hello world`. +2. Confirmation dialog appears. +3. Click dropdown and select **"Allow this exact command in this + Session"**. +4. The command executes. +5. In the same conversation, type: `run echo hello world` again. +6. Observe **auto-approved** — exact same command line. +7. In the same conversation, type: `run echo different text`. +8. Observe **confirmation dialog appears** — different command line, + even though command name `echo` is the same. + +#### Expected Result +- Exact command approval is strict: only the identical command line + is auto-approved. +- A different argument string requires separate confirmation. + +#### 📸 Key Screenshots +- [ ] First dialog: selecting "Allow this exact command in this Session". +- [ ] Same command: auto-approved. +- [ ] Different arguments: dialog appears again. + +--- + +## 6. Simple command hides redundant exact-command actions + +### TC-006: Single-word command shows minimal dropdown + +**Type:** `Edge Case` +**Priority:** `P1` + +#### Steps +1. In Agent Mode, trigger a single-word command like `ipconfig` + (or `hostname`). +2. Confirmation dialog appears. +3. Click the dropdown arrow. +4. Count the menu items. + +#### Expected Result +- "Allow 'ipconfig' in this Session" — present. +- "Always Allow 'ipconfig'" — present. +- "Allow this exact command in this Session" — **NOT present** + (redundant: exact command = command name for single-word commands). +- "Always Allow this exact command" — **NOT present**. +- "Allow all commands in this Session" — present. + +--- + +## 7. Unmatched auto-approve toggle + +### TC-007: Enable unmatched auto-approve → unknown commands pass +through → disable → they require confirmation again + +**Type:** `Happy Path` +**Priority:** `P1` + +#### Steps +1. Open **Preferences → Tool Auto Approve**. +2. Check **"Auto approve commands not covered by rules"**. +3. Click **"Apply and Close"**. +4. In Agent Mode, type: `run hostname`. +5. Observe **auto-approved** — `hostname` has no matching rule but + unmatched auto-approve is enabled. +6. Open preferences again. +7. **Uncheck** "Auto approve commands not covered by rules". +8. Click **"Apply and Close"**. +9. In the same or new conversation, type: `run hostname`. +10. Observe **confirmation dialog appears**. + +#### Expected Result +- With unmatched enabled: unknown commands auto-approve. +- With unmatched disabled: unknown commands require confirmation. + +#### 📸 Key Screenshots +- [ ] Preference: "Auto approve commands not covered by rules" checked. +- [ ] `hostname` auto-approved. +- [ ] Preference: unchecked. +- [ ] `hostname` shows confirmation dialog. + +--- + +## 8. Regex rule integration + +### TC-008: Add a case-insensitive regex allow rule → verify it +matches in Agent Mode + +**Type:** `Happy Path` +**Priority:** `P2` + +#### Steps +1. Open **Preferences → Tool Auto Approve**. +2. Click **"Add..."**, enter `/^npm\b/i`, select **"Allow"**, click OK. +3. Click **"Apply and Close"**. +4. In Agent Mode, type: `install lodash using npm`. +5. Agent invokes `npm install lodash`. +6. Observe **auto-approved** — regex `/^npm\b/i` matches. +7. Type: `run NPM run build` (uppercase). +8. Observe **auto-approved** — case-insensitive flag `/i` matches. + +#### Expected Result +- The regex allow rule auto-approves matching commands. +- Case-insensitive flag works correctly. + +--- + +## 9. "Allow all commands in this Session" — blanket approval + +### TC-009: Allow all → every subsequent terminal call auto-approves +in this conversation + +**Type:** `Happy Path` +**Priority:** `P1` + +#### Steps +1. In Agent Mode, trigger any terminal command. +2. Confirmation dialog appears. +3. Click dropdown and select **"Allow all commands in this Session"**. +4. The command executes. +5. In the same conversation, send multiple prompts that trigger + different terminal commands (e.g., "run dir", "run echo test", + "run ipconfig"). +6. Observe that **none of them** show a confirmation dialog. +7. Start a **new conversation**. +8. Trigger any terminal command. +9. Observe that the **confirmation dialog appears again**. + +#### Expected Result +- "Allow all" is a blanket session-scoped approval. +- Every terminal command in the same conversation auto-approves. +- New conversations start fresh. + +--- + +## 10. Reset to Defaults clears custom rules + +### TC-010: Add custom rules → Reset → verify Agent Mode behavior +reverts + +**Type:** `Happy Path` +**Priority:** `P2` + +#### Steps +1. Open preferences and add `echo` as Allow, `javac` as Allow. +2. Apply and close. +3. Verify in Agent Mode: `echo hello` auto-approves. +4. Open preferences again. +5. Click **"Reset to Defaults"** and confirm. +6. Verify only default deny rules remain — `echo` and `javac` are gone. +7. Apply and close. +8. In Agent Mode, trigger `echo hello`. +9. Observe **confirmation dialog appears** — the custom allow rule was + removed. + +#### Expected Result +- Reset removes all custom rules. +- Commands that were previously allowed now require confirmation. + +#### 📸 Key Screenshots +- [ ] Preference page after adding custom rules. +- [ ] Preference page after reset — only defaults. +- [ ] `echo hello` shows confirmation dialog post-reset. diff --git a/com.microsoft.copilot.eclipse.ui.test/src/com/microsoft/copilot/eclipse/ui/chat/confirmation/TerminalConfirmationHandlerTests.java b/com.microsoft.copilot.eclipse.ui.test/src/com/microsoft/copilot/eclipse/ui/chat/confirmation/TerminalConfirmationHandlerTests.java new file mode 100644 index 00000000..18ab98ae --- /dev/null +++ b/com.microsoft.copilot.eclipse.ui.test/src/com/microsoft/copilot/eclipse/ui/chat/confirmation/TerminalConfirmationHandlerTests.java @@ -0,0 +1,478 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + +package com.microsoft.copilot.eclipse.ui.chat.confirmation; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.Mockito.when; + +import java.util.List; +import java.util.Map; + +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.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; + +import com.microsoft.copilot.eclipse.core.Constants; +import com.microsoft.copilot.eclipse.core.chat.ConfirmationAction; +import com.microsoft.copilot.eclipse.core.chat.ConfirmationActionScope; +import com.microsoft.copilot.eclipse.core.chat.ConfirmationContent; +import com.microsoft.copilot.eclipse.core.chat.ConfirmationResult; +import com.microsoft.copilot.eclipse.core.chat.TerminalAutoApproveRule; +import com.microsoft.copilot.eclipse.core.lsp.protocol.InvokeClientToolConfirmationParams; +import com.microsoft.copilot.eclipse.core.lsp.protocol.ToolMetadata; +import com.microsoft.copilot.eclipse.core.lsp.protocol.ToolMetadata.TerminalCommandData; + +@ExtendWith(MockitoExtension.class) +class TerminalConfirmationHandlerTests { + + private static final String CONV_ID = "conv-1"; + private static final Gson GSON = new Gson(); + + @Mock + private IPreferenceStore preferenceStore; + + private TerminalConfirmationHandler handler; + + @BeforeEach + void setUp() { + handler = new TerminalConfirmationHandler(preferenceStore); + } + + // --- matchesRule --- + + @Test + void matchesRule_simpleRuleMatchesCommandAtStart() { + assertTrue(TerminalConfirmationHandler.matchesRule( + "rm -rf /tmp", "rm")); + } + + @Test + void matchesRule_simpleRuleDoesNotMatchMiddleOfWord() { + assertFalse(TerminalConfirmationHandler.matchesRule( + "remove something", "rm")); + } + + @Test + void matchesRule_regexCaseInsensitive() { + assertTrue(TerminalConfirmationHandler.matchesRule( + "Git status", "/^git\\b/i")); + } + + @Test + void matchesRule_regexDotallMatchesSubshell() { + assertTrue(TerminalConfirmationHandler.matchesRule( + "(echo hello)", "/(\\(.+\\))/s")); + } + + @Test + void matchesRule_nullSubCommandReturnsFalse() { + assertFalse(TerminalConfirmationHandler.matchesRule(null, "rm")); + } + + @Test + void matchesRule_emptySubCommandReturnsFalse() { + assertFalse(TerminalConfirmationHandler.matchesRule("", "rm")); + } + + @Test + void matchesRule_nullRuleReturnsFalse() { + assertFalse(TerminalConfirmationHandler.matchesRule("rm -rf", null)); + } + + @Test + void matchesRule_emptyRuleReturnsFalse() { + assertFalse(TerminalConfirmationHandler.matchesRule("rm -rf", "")); + } + + @Test + void matchesRule_blankInputsReturnFalse() { + assertFalse(TerminalConfirmationHandler.matchesRule(" ", "rm")); + assertFalse(TerminalConfirmationHandler.matchesRule("rm", " ")); + } + + // --- evaluate --- + + @Test + void evaluate_autoApprovedWhenAllSubCommandsMatchAllowRules() { + stubRules(List.of(new TerminalAutoApproveRule("echo", true))); + stubUnmatched(false); + + InvokeClientToolConfirmationParams params = + buildParams(new String[]{"echo hello"}, new String[]{"echo"}, + "echo hello"); + ConfirmationResult result = handler.evaluate(params); + + assertTrue(result.isAutoApproved()); + } + + @Test + void evaluate_needsConfirmationWhenDenyRuleMatches() { + stubRules(List.of(new TerminalAutoApproveRule("rm", false))); + stubUnmatched(false); + + InvokeClientToolConfirmationParams params = + buildParams(new String[]{"rm -rf /"}, new String[]{"rm"}, + "rm -rf /"); + ConfirmationResult result = handler.evaluate(params); + + assertFalse(result.isAutoApproved()); + assertNotNull(result.getContent()); + } + + @Test + void evaluate_needsConfirmationWhenNoRulesMatchAndUnmatchedFalse() { + stubRules(List.of(new TerminalAutoApproveRule("echo", true))); + stubUnmatched(false); + + InvokeClientToolConfirmationParams params = + buildParams(new String[]{"ls -la"}, new String[]{"ls"}, + "ls -la"); + ConfirmationResult result = handler.evaluate(params); + + assertFalse(result.isAutoApproved()); + } + + @Test + void evaluate_autoApprovedWhenNoRulesMatchAndUnmatchedTrue() { + stubRules(List.of(new TerminalAutoApproveRule("echo", true))); + stubUnmatched(true); + + InvokeClientToolConfirmationParams params = + buildParams(new String[]{"ls -la"}, new String[]{"ls"}, + "ls -la"); + ConfirmationResult result = handler.evaluate(params); + + assertTrue(result.isAutoApproved()); + } + + @Test + void evaluate_needsConfirmationWhenSubCommandsNull() { + stubRules(List.of()); + + InvokeClientToolConfirmationParams params = + buildParams(null, null, "echo hello"); + ConfirmationResult result = handler.evaluate(params); + + assertFalse(result.isAutoApproved()); + } + + @Test + void evaluate_needsConfirmationWhenSubCommandsEmpty() { + stubRules(List.of()); + + InvokeClientToolConfirmationParams params = + buildParams(new String[]{}, null, "echo hello"); + ConfirmationResult result = handler.evaluate(params); + + assertFalse(result.isAutoApproved()); + } + + @Test + void evaluate_emptyRulesUsesUnmatchedSetting() { + stubRules(List.of()); + stubUnmatched(true); + + InvokeClientToolConfirmationParams params = + buildParams(new String[]{"ls"}, new String[]{"ls"}, "ls"); + ConfirmationResult result = handler.evaluate(params); + + assertTrue(result.isAutoApproved()); + } + + // --- Session memory via persistDecision --- + + @Test + void persistDecision_acceptAllSession_autoApprovesSubsequent() { + stubRules(List.of()); + stubUnmatched(false); + + InvokeClientToolConfirmationParams params = + buildParams(new String[]{"echo"}, new String[]{"echo"}, + "echo hello"); + + ConfirmationAction allSession = buildSessionAction( + TerminalConfirmationHandler.Action.ACCEPT_ALL_SESSION); + handler.persistDecision(allSession, params); + + ConfirmationResult result = handler.evaluate(params); + assertTrue(result.isAutoApproved()); + } + + @Test + void persistDecision_acceptNamesSession_autoApprovesMatchingNames() { + stubRules(List.of()); + stubUnmatched(false); + + InvokeClientToolConfirmationParams params = + buildParams(new String[]{"echo"}, new String[]{"echo"}, + "echo hello"); + + ConfirmationAction namesSession = buildSessionAction( + TerminalConfirmationHandler.Action.ACCEPT_NAMES_SESSION); + handler.persistDecision(namesSession, params); + + ConfirmationResult result = handler.evaluate(params); + assertTrue(result.isAutoApproved()); + } + + @Test + void persistDecision_acceptExactSession_autoApprovesMatchingCommand() { + stubRules(List.of()); + stubUnmatched(false); + + InvokeClientToolConfirmationParams params = + buildParams(new String[]{"echo"}, new String[]{"echo"}, + "echo hello"); + + ConfirmationAction exactSession = buildSessionAction( + TerminalConfirmationHandler.Action.ACCEPT_EXACT_SESSION); + handler.persistDecision(exactSession, params); + + ConfirmationResult result = handler.evaluate(params); + assertTrue(result.isAutoApproved()); + } + + @Test + void clearSession_removesApprovalsForConversation() { + stubRules(List.of()); + stubUnmatched(false); + + InvokeClientToolConfirmationParams params = + buildParams(new String[]{"echo"}, new String[]{"echo"}, + "echo hello"); + + ConfirmationAction allSession = buildSessionAction( + TerminalConfirmationHandler.Action.ACCEPT_ALL_SESSION); + handler.persistDecision(allSession, params); + + handler.clearSession(CONV_ID); + + ConfirmationResult result = handler.evaluate(params); + assertFalse(result.isAutoApproved()); + } + + @Test + void clearSession_doesNotAffectOtherConversation() { + stubRules(List.of()); + stubUnmatched(false); + + InvokeClientToolConfirmationParams params = + buildParams(new String[]{"echo"}, new String[]{"echo"}, + "echo hello"); + + ConfirmationAction allSession = buildSessionAction( + TerminalConfirmationHandler.Action.ACCEPT_ALL_SESSION); + handler.persistDecision(allSession, params); + + handler.clearSession("other-conv"); + + ConfirmationResult result = handler.evaluate(params); + assertTrue(result.isAutoApproved()); + } + + // --- buildContent actions --- + + @Test + void buildContent_alwaysHasAllowOnceAsPrimary() { + stubRules(List.of()); + stubUnmatched(false); + + InvokeClientToolConfirmationParams params = + buildParams(new String[]{"echo"}, new String[]{"echo"}, + "echo hello"); + ConfirmationResult result = handler.evaluate(params); + + ConfirmationContent content = result.getContent(); + assertNotNull(content); + List actions = content.getActions(); + ConfirmationAction first = actions.get(0); + assertTrue(first.isPrimary()); + assertTrue(first.isAccept()); + } + + @Test + void buildContent_alwaysHasSkipAsDismiss() { + stubRules(List.of()); + stubUnmatched(false); + + InvokeClientToolConfirmationParams params = + buildParams(new String[]{"echo"}, new String[]{"echo"}, + "echo hello"); + ConfirmationResult result = handler.evaluate(params); + + List actions = result.getContent().getActions(); + ConfirmationAction last = actions.get(actions.size() - 1); + assertFalse(last.isAccept()); + } + + @Test + void buildContent_hasAllowAllSessionAction() { + stubRules(List.of()); + stubUnmatched(false); + + InvokeClientToolConfirmationParams params = + buildParams(new String[]{"echo"}, new String[]{"echo"}, + "echo hello"); + ConfirmationResult result = handler.evaluate(params); + + List actions = result.getContent().getActions(); + boolean hasAllSession = actions.stream().anyMatch(a -> + a.getMetadata().containsKey(ConfirmationAction.META_ACTION) + && a.getMetadata().get(ConfirmationAction.META_ACTION) + .equals( + TerminalConfirmationHandler.Action.ACCEPT_ALL_SESSION + .name())); + assertTrue(hasAllSession); + } + + @Test + void buildContent_hasCommandNameActionsWhenNamesPresent() { + stubRules(List.of()); + stubUnmatched(false); + + InvokeClientToolConfirmationParams params = + buildParams(new String[]{"echo"}, new String[]{"echo"}, + "echo hello"); + ConfirmationResult result = handler.evaluate(params); + + List actions = result.getContent().getActions(); + boolean hasNamesSession = actions.stream().anyMatch(a -> + hasActionType(a, + TerminalConfirmationHandler.Action.ACCEPT_NAMES_SESSION)); + boolean hasNamesGlobal = actions.stream().anyMatch(a -> + hasActionType(a, + TerminalConfirmationHandler.Action.ACCEPT_NAMES_GLOBAL)); + assertTrue(hasNamesSession); + assertTrue(hasNamesGlobal); + } + + @Test + void buildContent_hasExactCommandActionsWhenDifferentFromName() { + stubRules(List.of()); + stubUnmatched(false); + + // commandLine "echo hello" differs from single commandName "echo" + InvokeClientToolConfirmationParams params = + buildParams(new String[]{"echo"}, new String[]{"echo"}, + "echo hello"); + ConfirmationResult result = handler.evaluate(params); + + List actions = result.getContent().getActions(); + boolean hasExactSession = actions.stream().anyMatch(a -> + hasActionType(a, + TerminalConfirmationHandler.Action.ACCEPT_EXACT_SESSION)); + boolean hasExactGlobal = actions.stream().anyMatch(a -> + hasActionType(a, + TerminalConfirmationHandler.Action.ACCEPT_EXACT_GLOBAL)); + assertTrue(hasExactSession); + assertTrue(hasExactGlobal); + } + + @Test + void buildContent_noExactActionsWhenSingleSubCommandEqualsName() { + stubRules(List.of()); + stubUnmatched(false); + + // commandLine equals the single commandName + InvokeClientToolConfirmationParams params = + buildParams(new String[]{"echo"}, new String[]{"echo"}, "echo"); + ConfirmationResult result = handler.evaluate(params); + + List actions = result.getContent().getActions(); + boolean hasExact = actions.stream().anyMatch(a -> + hasActionType(a, + TerminalConfirmationHandler.Action.ACCEPT_EXACT_SESSION) + || hasActionType(a, + TerminalConfirmationHandler.Action.ACCEPT_EXACT_GLOBAL)); + assertFalse(hasExact); + } + + @Test + void buildContent_actionScopesAreCorrect() { + stubRules(List.of()); + stubUnmatched(false); + + InvokeClientToolConfirmationParams params = + buildParams(new String[]{"echo"}, new String[]{"echo"}, + "echo hello"); + ConfirmationResult result = handler.evaluate(params); + + List actions = result.getContent().getActions(); + + // Allow Once → ONCE scope + assertEquals(ConfirmationActionScope.ONCE, actions.get(0).getScope()); + + // Session actions have SESSION scope + actions.stream() + .filter(a -> hasActionType(a, + TerminalConfirmationHandler.Action.ACCEPT_NAMES_SESSION) + || hasActionType(a, + TerminalConfirmationHandler.Action.ACCEPT_EXACT_SESSION) + || hasActionType(a, + TerminalConfirmationHandler.Action.ACCEPT_ALL_SESSION)) + .forEach(a -> assertEquals(ConfirmationActionScope.SESSION, + a.getScope())); + + // Global actions have GLOBAL scope + actions.stream() + .filter(a -> hasActionType(a, + TerminalConfirmationHandler.Action.ACCEPT_NAMES_GLOBAL) + || hasActionType(a, + TerminalConfirmationHandler.Action.ACCEPT_EXACT_GLOBAL)) + .forEach(a -> assertEquals(ConfirmationActionScope.GLOBAL, + a.getScope())); + } + + // --- Helpers --- + + private void stubRules(List rules) { + when(preferenceStore.getString(Constants.AUTO_APPROVE_TERMINAL_RULES)) + .thenReturn(GSON.toJson(rules)); + } + + private void stubUnmatched(boolean value) { + when(preferenceStore.getBoolean( + Constants.AUTO_APPROVE_UNMATCHED_TERMINAL)).thenReturn(value); + } + + private static InvokeClientToolConfirmationParams buildParams( + String[] subCommands, String[] commandNames, String commandLine) { + TerminalCommandData tcd = new TerminalCommandData(); + tcd.setSubCommands(subCommands); + tcd.setCommandNames(commandNames); + + ToolMetadata meta = new ToolMetadata(); + meta.setTerminalCommandData(tcd); + + InvokeClientToolConfirmationParams params = + new InvokeClientToolConfirmationParams(); + params.setConversationId(CONV_ID); + params.setToolMetadata(meta); + params.setInput(Map.of("toolType", "terminal", "command", + commandLine != null ? commandLine : "")); + return params; + } + + private static ConfirmationAction buildSessionAction( + TerminalConfirmationHandler.Action actionType) { + Map meta = Map.of( + ConfirmationAction.META_ACTION, actionType.name()); + return new ConfirmationAction( + "test", true, ConfirmationActionScope.SESSION, meta, false); + } + + private static boolean hasActionType(ConfirmationAction action, + TerminalConfirmationHandler.Action type) { + return action.getMetadata().containsKey(ConfirmationAction.META_ACTION) + && action.getMetadata().get(ConfirmationAction.META_ACTION) + .equals(type.name()); + } +} diff --git a/com.microsoft.copilot.eclipse.ui/META-INF/MANIFEST.MF b/com.microsoft.copilot.eclipse.ui/META-INF/MANIFEST.MF index 39b10de8..00bd6634 100644 --- a/com.microsoft.copilot.eclipse.ui/META-INF/MANIFEST.MF +++ b/com.microsoft.copilot.eclipse.ui/META-INF/MANIFEST.MF @@ -7,6 +7,7 @@ Bundle-Vendor: GitHub Copilot Bundle-Localization: plugin Export-Package: com.microsoft.copilot.eclipse.ui, com.microsoft.copilot.eclipse.ui.chat, + com.microsoft.copilot.eclipse.ui.chat.confirmation;x-friends:="com.microsoft.copilot.eclipse.ui.test", com.microsoft.copilot.eclipse.ui.chat.services, com.microsoft.copilot.eclipse.ui.chat.tools, com.microsoft.copilot.eclipse.ui.completion, diff --git a/com.microsoft.copilot.eclipse.ui/css/dark.css b/com.microsoft.copilot.eclipse.ui/css/dark.css index f3712622..528ede1e 100644 --- a/com.microsoft.copilot.eclipse.ui/css/dark.css +++ b/com.microsoft.copilot.eclipse.ui/css/dark.css @@ -91,7 +91,7 @@ background-color: #3584F1; } -#chat-content-viewer > Composite > CopilotTurnWidget > InvokeToolConfirmationDialog > .bg-command-panel { +#chat-content-viewer > Composite > CopilotTurnWidget > InvokeToolConfirmationDialog > ScrolledComposite > .bg-command-panel { background-color: #48484C; } diff --git a/com.microsoft.copilot.eclipse.ui/css/light.css b/com.microsoft.copilot.eclipse.ui/css/light.css index f9fb3143..91be4717 100644 --- a/com.microsoft.copilot.eclipse.ui/css/light.css +++ b/com.microsoft.copilot.eclipse.ui/css/light.css @@ -91,7 +91,7 @@ background-color: #3584F1; } -#chat-content-viewer > Composite > CopilotTurnWidget > InvokeToolConfirmationDialog > .bg-command-panel { +#chat-content-viewer > Composite > CopilotTurnWidget > InvokeToolConfirmationDialog > ScrolledComposite > .bg-command-panel { background-color: #FFFFFF; } diff --git a/com.microsoft.copilot.eclipse.ui/plugin.properties b/com.microsoft.copilot.eclipse.ui/plugin.properties index 38fd45a7..636fc1fb 100644 --- a/com.microsoft.copilot.eclipse.ui/plugin.properties +++ b/com.microsoft.copilot.eclipse.ui/plugin.properties @@ -47,4 +47,5 @@ intro.quickStart.description=Boost your coding efficiency with built-in Copilot theme.category.label=GitHub Copilot theme.category.description=Font and color settings for GitHub Copilot theme.chatFont.label=Chat Font -theme.chatFont.description=The font used for text in the Copilot Chat view \ No newline at end of file +theme.chatFont.description=The font used for text in the Copilot Chat view +page.preferencesPage.autoApprove.name=Tool Auto Approve \ No newline at end of file diff --git a/com.microsoft.copilot.eclipse.ui/plugin.xml b/com.microsoft.copilot.eclipse.ui/plugin.xml index be871853..3d75c0b6 100644 --- a/com.microsoft.copilot.eclipse.ui/plugin.xml +++ b/com.microsoft.copilot.eclipse.ui/plugin.xml @@ -175,6 +175,12 @@ category="com.microsoft.copilot.eclipse.ui.preferences.CopilotPreferencesPage" class="com.microsoft.copilot.eclipse.ui.preferences.CustomModesPreferencePage"> + + @@ -1048,10 +1054,12 @@ + name="com.microsoft.copilot.eclipse.ui.nes.change" + markerType="org.eclipse.core.resources.textmarker"> + name="com.microsoft.copilot.eclipse.ui.nes.delete" + markerType="org.eclipse.core.resources.textmarker"> @@ -1111,4 +1119,4 @@ %theme.chatFont.description - + \ No newline at end of file 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 66e64742..a9031628 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.osgi.service.event.EventHandler; import com.microsoft.copilot.eclipse.core.CopilotCore; +import com.microsoft.copilot.eclipse.core.chat.ConfirmationContent; import com.microsoft.copilot.eclipse.core.events.CopilotEventConstants; import com.microsoft.copilot.eclipse.core.lsp.protocol.AgentToolCall; import com.microsoft.copilot.eclipse.core.lsp.protocol.LanguageModelToolConfirmationResult; @@ -61,6 +62,11 @@ public abstract class BaseTurnWidget extends Composite { protected Font boldFont = null; protected InvokeToolConfirmationDialog confirmDialog; + /** Returns the current confirmation dialog, or {@code null} if none active. */ + public InvokeToolConfirmationDialog getConfirmDialog() { + return confirmDialog; + } + // Footer protected Composite footer; @@ -463,20 +469,18 @@ protected void createAgentMessageWidget(CodingAgentMessageRequestParams params) /** * Prompts the user to confirm or deny a tool execution. * - * @param title The title of the confirmation dialog. - * @param message The message to display in the confirmation dialog. + * @param content The confirmation content with title, message, and action buttons. * @param input The input object to be passed to the tool. */ - public CompletableFuture requestToolExecutionConfirmation(String title, - String message, Object input) { - // process all the messages before showing the confirmation dialog + public CompletableFuture requestToolExecutionConfirmation( + ConfirmationContent content, Object input) { reset(); - this.confirmDialog = new InvokeToolConfirmationDialog(this, title, message, input); + this.confirmDialog = new InvokeToolConfirmationDialog(this, content, input); CompletableFuture toolConfirmationFuture = this.confirmDialog .getConfirmationFuture(); - this.getParent().layout(); + this.getParent().requestLayout(); return toolConfirmationFuture; } diff --git a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/InvokeToolConfirmationDialog.java b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/InvokeToolConfirmationDialog.java index 745f62a5..fc4634ea 100644 --- a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/InvokeToolConfirmationDialog.java +++ b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/InvokeToolConfirmationDialog.java @@ -3,6 +3,8 @@ package com.microsoft.copilot.eclipse.ui.chat; +import java.util.ArrayList; +import java.util.List; import java.util.Map; import java.util.concurrent.CompletableFuture; @@ -13,23 +15,34 @@ import org.eclipse.swt.custom.ScrolledComposite; import org.eclipse.swt.events.ControlAdapter; import org.eclipse.swt.events.ControlEvent; +import org.eclipse.swt.events.SelectionAdapter; +import org.eclipse.swt.events.SelectionEvent; import org.eclipse.swt.graphics.Font; import org.eclipse.swt.graphics.Point; +import org.eclipse.swt.graphics.Rectangle; import org.eclipse.swt.layout.GridData; import org.eclipse.swt.layout.GridLayout; import org.eclipse.swt.widgets.Button; import org.eclipse.swt.widgets.Composite; import org.eclipse.swt.widgets.Label; +import org.eclipse.swt.widgets.Menu; +import org.eclipse.swt.widgets.MenuItem; +import com.microsoft.copilot.eclipse.core.chat.ConfirmationAction; +import com.microsoft.copilot.eclipse.core.chat.ConfirmationContent; import com.microsoft.copilot.eclipse.core.lsp.protocol.LanguageModelToolConfirmationResult; import com.microsoft.copilot.eclipse.core.lsp.protocol.LanguageModelToolConfirmationResult.ToolConfirmationResult; import com.microsoft.copilot.eclipse.ui.CopilotUi; import com.microsoft.copilot.eclipse.ui.swt.CssConstants; +import com.microsoft.copilot.eclipse.ui.swt.SplitDropdownButton; import com.microsoft.copilot.eclipse.ui.utils.SwtUtils; import com.microsoft.copilot.eclipse.ui.utils.UiUtils; /** - * Dialog to confirm tool execution. + * Dialog to confirm tool execution. Renders a title, message, optional command + * block, and action buttons driven by {@link ConfirmationContent}. The primary + * action is shown as a {@link SplitDropdownButton} with secondary actions in + * the dropdown menu. */ public class InvokeToolConfirmationDialog extends Composite { @@ -47,32 +60,76 @@ public class InvokeToolConfirmationDialog extends Composite { * The key for the action in the input map (used by debugger tool). */ private static final String ACTION_KEY = "action"; + private CompletableFuture toolConfirmationFuture; private String cancelMessage; private Label titleLbl; private Font boldFont; private Runnable titleFontChangeCallback; + private ConfirmationContent confirmationContent; + private ConfirmationAction selectedAction; /** - * Create a new confirmation dialog for tool execution. + * Create a new confirmation dialog driven by {@link ConfirmationContent}. * - * @param parent The parent composite - * @param title The title of the confirmation dialog - * @param message The message to display - * @param input The input object to pass to the tool + * @param parent the parent composite + * @param content confirmation content with title, message, and action buttons + * @param input the input object to pass to the tool */ - public InvokeToolConfirmationDialog(Composite parent, String title, String message, Object input) { + public InvokeToolConfirmationDialog(Composite parent, + ConfirmationContent content, Object input) { super(parent, SWT.BORDER | SWT.WRAP); this.setLayout(new GridLayout(1, false)); this.setLayoutData(new GridData(SWT.FILL, SWT.FILL, true, false)); - createDialogContent(title, message, input); + this.confirmationContent = content; + createDialogContent(content.getTitle(), content.getMessage(), input); this.toolConfirmationFuture = new CompletableFuture<>(); } - private void createDialogContent(String title, String message, Object input) { - // Title of the confirmation dialog + /** + * Returns the action the user selected, or {@code null} if dismissed. + */ + public ConfirmationAction getSelectedAction() { + return selectedAction; + } + + /** + * Get the future that will be completed when the user makes a choice. + * + * @return CompletableFuture containing the result of user's choice + */ + public CompletableFuture getConfirmationFuture() { + return toolConfirmationFuture; + } + + /** + * Cancels the current tool confirmation dialog programmatically. This has + * the same effect as clicking the Cancel / Skip button. + */ + public void cancelConfirmation() { + if (toolConfirmationFuture != null && !toolConfirmationFuture.isDone()) { + toolConfirmationFuture.complete( + new LanguageModelToolConfirmationResult(ToolConfirmationResult.DISMISS)); + + Composite parent = this.getParent(); + SwtUtils.invokeOnDisplayThread(() -> { + if (StringUtils.isNotEmpty(this.cancelMessage)) { + new AgentToolCancelLabel(this.getParent(), SWT.NONE, this.cancelMessage); + } + this.dispose(); + if (parent != null && !parent.isDisposed()) { + parent.requestLayout(); + } + }, this); + } + } + + // --------------- content creation --------------- + + private void createDialogContent(String title, String message, + Object input) { titleLbl = new Label(this, SWT.LEFT | SWT.WRAP); titleLbl.setLayoutData(new GridData(SWT.FILL, SWT.FILL, true, false)); titleLbl.setText(title); @@ -93,160 +150,173 @@ private void createDialogContent(String title, String message, Object input) { } }); - // Confirmation message of the confirmation dialog Label messageLbl = new Label(this, SWT.LEFT | SWT.WRAP); - GridData messageGridData = new GridData(SWT.FILL, SWT.FILL, true, false); - messageLbl.setLayoutData(messageGridData); + messageLbl.setLayoutData( + new GridData(SWT.FILL, SWT.FILL, true, false)); messageLbl.setText(message); registerControlForFontUpdates(messageLbl); - // More information about the tool invocation - if (input != null) { - Map inputMap = (Map) input; - - // For debugger tool, show all input parameters - if (inputMap.containsKey(ACTION_KEY)) { - String displayText = formatDebuggerInput(inputMap); - - // Create a scrollable container for the input text - ScrolledComposite commandScroll = new ScrolledComposite(this, SWT.H_SCROLL | SWT.V_SCROLL); - commandScroll.setLayoutData(new GridData(SWT.FILL, SWT.FILL, true, false)); - commandScroll.setExpandHorizontal(true); - commandScroll.setExpandVertical(true); - - Label commandLbl = new Label(commandScroll, SWT.LEFT); - // Escape & characters that are followed by non-space characters, needed for SWT labels where & is used as a - // mnemonic character - String escapedCommand = displayText.replace("&", "&&"); - commandLbl.setText(escapedCommand); - commandLbl.setData(CssConstants.CSS_CLASS_NAME_KEY, "bg-command-panel"); - this.cancelMessage = escapedCommand; - registerControlForFontUpdates(commandLbl); - - commandScroll.setContent(commandLbl); - commandScroll.addControlListener(new ControlAdapter() { - @Override - public void controlResized(ControlEvent e) { - Point size = commandLbl.computeSize(SWT.DEFAULT, SWT.DEFAULT); - commandLbl.setSize(size); - commandScroll.setMinSize(size); - } - }); - // Initial size computation - Point size = commandLbl.computeSize(SWT.DEFAULT, SWT.DEFAULT); - commandLbl.setSize(size); - commandScroll.setMinSize(size); - } else if (inputMap.containsKey(COMMAND_KEY)) { - // For terminal tool, show command - // Create a scrollable container for the command text - ScrolledComposite commandScroll = new ScrolledComposite(this, SWT.H_SCROLL); - commandScroll.setLayoutData(new GridData(SWT.FILL, SWT.FILL, true, false)); - commandScroll.setExpandHorizontal(true); - commandScroll.setExpandVertical(true); - - Label commandLbl = new Label(commandScroll, SWT.LEFT); - String command = (String) inputMap.get(COMMAND_KEY); - // Escape & characters that are followed by non-space characters, needed for SWT labels where & is used as a - // mnemonic character - String escapedCommand = command.replace("&", "&&"); - commandLbl.setText(escapedCommand); - commandLbl.setData(CssConstants.CSS_CLASS_NAME_KEY, "bg-command-panel"); - this.cancelMessage = escapedCommand; - registerControlForFontUpdates(commandLbl); - - commandScroll.setContent(commandLbl); - commandScroll.addControlListener(new ControlAdapter() { - @Override - public void controlResized(ControlEvent e) { - Point size = commandLbl.computeSize(SWT.DEFAULT, SWT.DEFAULT); - commandLbl.setSize(size); - commandScroll.setMinSize(size); - } - }); - // Initial size computation + createInputContent(input); + createActionButtons(); + } + + @SuppressWarnings("unchecked") + private void createInputContent(Object input) { + if (input == null) { + return; + } + Map inputMap = (Map) input; + + if (inputMap.containsKey(ACTION_KEY)) { + createScrollableCommand(formatDebuggerInput(inputMap), + SWT.H_SCROLL | SWT.V_SCROLL); + } else if (inputMap.containsKey(COMMAND_KEY)) { + createScrollableCommand((String) inputMap.get(COMMAND_KEY), + SWT.H_SCROLL); + } + + if (inputMap.containsKey(EXPLANATION_KEY)) { + Label explanationLbl = new Label(this, SWT.LEFT | SWT.WRAP); + explanationLbl.setLayoutData( + new GridData(SWT.FILL, SWT.FILL, true, false)); + explanationLbl.setText((String) inputMap.get(EXPLANATION_KEY)); + registerControlForFontUpdates(explanationLbl); + } + } + + private void createScrollableCommand(String text, int scrollStyle) { + ScrolledComposite commandScroll = + new ScrolledComposite(this, scrollStyle); + commandScroll.setLayoutData( + new GridData(SWT.FILL, SWT.FILL, true, false)); + commandScroll.setExpandHorizontal(true); + commandScroll.setExpandVertical(true); + + Label commandLbl = new Label(commandScroll, SWT.LEFT); + String escapedCommand = text.replace("&", "&&"); + commandLbl.setText(escapedCommand); + commandLbl.setData(CssConstants.CSS_CLASS_NAME_KEY, "bg-command-panel"); + this.cancelMessage = escapedCommand; + registerControlForFontUpdates(commandLbl); + + commandScroll.setContent(commandLbl); + commandScroll.addControlListener(new ControlAdapter() { + @Override + public void controlResized(ControlEvent e) { Point size = commandLbl.computeSize(SWT.DEFAULT, SWT.DEFAULT); commandLbl.setSize(size); commandScroll.setMinSize(size); } + }); + Point size = commandLbl.computeSize(SWT.DEFAULT, SWT.DEFAULT); + commandLbl.setSize(size); + commandScroll.setMinSize(size); + } + + // --------------- action buttons with dropdown --------------- + + private void createActionButtons() { + List actions = confirmationContent.getActions(); - if (inputMap.containsKey(EXPLANATION_KEY)) { - Label explanationLbl = new Label(this, SWT.LEFT | SWT.WRAP); - explanationLbl.setLayoutData(new GridData(SWT.FILL, SWT.FILL, true, false)); - explanationLbl.setText((String) inputMap.get(EXPLANATION_KEY)); - registerControlForFontUpdates(explanationLbl); + ConfirmationAction primaryAction = null; + ConfirmationAction dismissAction = null; + List dropdownActions = new ArrayList<>(); + + for (ConfirmationAction action : actions) { + if (!action.isAccept()) { + dismissAction = action; + } else if (action.isPrimary()) { + primaryAction = action; + } else { + dropdownActions.add(action); } } - createButtons(); - } + if (primaryAction == null) { + return; + } - private void createButtons() { - GridLayout actionLayout = new GridLayout(2, false); - actionLayout.marginLeft = 0; - actionLayout.marginRight = 0; - actionLayout.marginWidth = 0; - actionLayout.horizontalSpacing = 0; - actionLayout.marginHeight = 0; - Composite actionArea = new Composite(this, SWT.NONE); - actionArea.setLayout(actionLayout); - actionArea.setLayoutData(new GridData(SWT.FILL, SWT.CENTER, true, false)); - - Button continueButton = new Button(actionArea, SWT.PUSH); - continueButton.setLayoutData(new GridData(SWT.BEGINNING, SWT.CENTER, false, false)); - continueButton.setText("Continue"); - continueButton.addListener(SWT.Selection, e -> { - this.toolConfirmationFuture.complete(new LanguageModelToolConfirmationResult(ToolConfirmationResult.ACCEPT)); - - // Store parent reference before disposal - Composite parent = this.getParent(); - this.dispose(); - // Check if parent is still valid before using it - if (parent != null && !parent.isDisposed()) { - parent.layout(); + // Column count: primary dropdown button + dismiss + Composite actionArea = newButtonArea(2); + + // --- primary dropdown button --- + SplitDropdownButton primaryDropdown = + new SplitDropdownButton(actionArea, SWT.PUSH); + primaryDropdown.setText(primaryAction.getLabel()); + primaryDropdown.setShowArrow(!dropdownActions.isEmpty()); + primaryDropdown.setSeparatorColor( + getDisplay().getSystemColor(SWT.COLOR_WHITE)); + + Button primaryBtn = primaryDropdown.getButton(); + primaryBtn.setData(CssConstants.CSS_CLASS_NAME_KEY, "btn-primary"); + primaryBtn.setLayoutData( + new GridData(SWT.BEGINNING, SWT.CENTER, false, false)); + registerControlForFontUpdates(primaryBtn); + + final ConfirmationAction primaryRef = primaryAction; + primaryDropdown.addSelectionListener(new SelectionAdapter() { + @Override + public void widgetSelected(SelectionEvent e) { + if (e.detail == SWT.ARROW && !dropdownActions.isEmpty()) { + Menu menu = new Menu(primaryBtn.getShell(), SWT.POP_UP); + for (ConfirmationAction action : dropdownActions) { + MenuItem item = new MenuItem(menu, SWT.PUSH); + item.setText(action.getLabel()); + item.addListener(SWT.Selection, + ev -> acceptAndDispose(action)); + } + Rectangle bounds = primaryBtn.getBounds(); + Point loc = primaryBtn.getParent() + .toDisplay(bounds.x, bounds.y + bounds.height); + menu.setLocation(loc); + menu.setVisible(true); + } else { + acceptAndDispose(primaryRef); + } } }); - continueButton.setData(CssConstants.CSS_CLASS_NAME_KEY, "btn-primary"); - registerControlForFontUpdates(continueButton); - Button cancelButton = new Button(actionArea, SWT.PUSH); - cancelButton.setLayoutData(new GridData(SWT.BEGINNING, SWT.CENTER, false, false)); - cancelButton.setText("Cancel"); - cancelButton.addListener(SWT.Selection, e -> { + // --- dismiss (skip) button --- + Button dismissBtn = new Button(actionArea, SWT.PUSH); + dismissBtn.setLayoutData( + new GridData(SWT.BEGINNING, SWT.CENTER, false, false)); + dismissBtn.setText( + dismissAction != null ? dismissAction.getLabel() : "Cancel"); + registerControlForFontUpdates(dismissBtn); + + final ConfirmationAction dismissRef = dismissAction; + dismissBtn.addListener(SWT.Selection, e -> { + this.selectedAction = dismissRef; cancelConfirmation(); }); - registerControlForFontUpdates(cancelButton); } - /** - * Get the future that will be completed when the user makes a choice. - * - * @return CompletableFuture containing the result of user's choice - */ - public CompletableFuture getConfirmationFuture() { - return toolConfirmationFuture; + // --------------- helpers --------------- + + private Composite newButtonArea(int columns) { + GridLayout layout = new GridLayout(columns, false); + layout.marginLeft = 0; + layout.marginRight = 0; + layout.marginWidth = 0; + layout.horizontalSpacing = 0; + layout.marginHeight = 0; + + Composite area = new Composite(this, SWT.NONE); + area.setLayout(layout); + area.setLayoutData(new GridData(SWT.FILL, SWT.CENTER, true, false)); + return area; } - /** - * Cancels the current tool confirmation dialog programmatically. This has the same effect as clicking the Cancel - * button in the confirmation dialog. - */ - public void cancelConfirmation() { - if (toolConfirmationFuture != null && !toolConfirmationFuture.isDone()) { - toolConfirmationFuture.complete(new LanguageModelToolConfirmationResult(ToolConfirmationResult.DISMISS)); + private void acceptAndDispose(ConfirmationAction action) { + this.selectedAction = action; + this.toolConfirmationFuture.complete( + new LanguageModelToolConfirmationResult( + ToolConfirmationResult.ACCEPT)); - // Store parent reference before disposal - Composite parent = this.getParent(); - SwtUtils.invokeOnDisplayThread(() -> { - // Only show the cancel widget for special cases when the tool has a parameter "command" in the input map - if (StringUtils.isNotEmpty(this.cancelMessage)) { - new AgentToolCancelLabel(this.getParent(), SWT.NONE, this.cancelMessage); - } - this.dispose(); - // Check if parent is still valid before using it - if (parent != null && !parent.isDisposed()) { - parent.layout(); - } - }, this); + Composite parent = this.getParent(); + this.dispose(); + if (parent != null && !parent.isDisposed()) { + parent.requestLayout(); } } diff --git a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/Messages.java b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/Messages.java index 0f17edd2..97ee8c5e 100644 --- a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/Messages.java +++ b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/Messages.java @@ -38,6 +38,22 @@ public final class Messages extends NLS { public static String todoList_expandTooltip; public static String todoList_collapseTooltip; + // Confirmation dialog action labels + public static String confirmation_action_allowOnce; + public static String confirmation_action_skip; + public static String confirmation_action_allowAllCommands; + public static String confirmation_action_allowNamesSession; + public static String confirmation_action_alwaysAllowNames; + public static String confirmation_action_allowExactSession; + public static String confirmation_action_alwaysAllowExact; + public static String confirmation_action_alwaysAllow; + + // Confirmation dialog titles + public static String confirmation_title_terminal; + + // Misc + public static String confirmation_autoApprovedDescription; + static { // initialize resource bundle NLS.initializeMessages(BUNDLE_NAME, Messages.class); diff --git a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/confirmation/ConfirmationHandler.java b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/confirmation/ConfirmationHandler.java new file mode 100644 index 00000000..a389e184 --- /dev/null +++ b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/confirmation/ConfirmationHandler.java @@ -0,0 +1,42 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + +package com.microsoft.copilot.eclipse.ui.chat.confirmation; + +import com.microsoft.copilot.eclipse.core.chat.ConfirmationAction; +import com.microsoft.copilot.eclipse.core.chat.ConfirmationResult; +import com.microsoft.copilot.eclipse.core.lsp.protocol.InvokeClientToolConfirmationParams; + +/** + * Evaluates whether a tool confirmation request can be auto-approved. + * Each implementation handles a specific category of tool (terminal, file operations, MCP, etc.). + */ +public interface ConfirmationHandler { + + /** + * Evaluates whether the given confirmation request should be auto-approved. + * Implementations should check both global rules and session memory. + * + * @param params the confirmation request parameters from CLS + * @return the confirmation result + */ + ConfirmationResult evaluate(InvokeClientToolConfirmationParams params); + + /** + * Persists a user's decision based on the action scope. + * SESSION actions are stored in-memory per conversation; + * GLOBAL actions are written to preferences. + * + * @param action the user's selected action with metadata + * @param params the original confirmation params (for conversationId etc.) + */ + default void persistDecision(ConfirmationAction action, + InvokeClientToolConfirmationParams params) { + // no-op by default + } + + /** Clears session-scoped approvals for the given conversation. */ + default void clearSession(String conversationId) { + // no-op by default + } +} diff --git a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/confirmation/ConfirmationService.java b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/confirmation/ConfirmationService.java index 1e617bd5..66a725b6 100644 --- a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/confirmation/ConfirmationService.java +++ b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/confirmation/ConfirmationService.java @@ -3,23 +3,129 @@ package com.microsoft.copilot.eclipse.ui.chat.confirmation; +import java.util.EnumMap; +import java.util.Map; + +import org.eclipse.jface.preference.IPreferenceStore; + +import com.microsoft.copilot.eclipse.core.Constants; +import com.microsoft.copilot.eclipse.core.chat.ConfirmationAction; +import com.microsoft.copilot.eclipse.core.chat.ConfirmationActionScope; import com.microsoft.copilot.eclipse.core.chat.ConfirmationResult; import com.microsoft.copilot.eclipse.core.lsp.protocol.InvokeClientToolConfirmationParams; /** - * Central entry point for auto-approve evaluation. Decides whether a tool confirmation - * request should be auto-approved or shown to the user. Currently a no-op skeleton that - * always requires confirmation; concrete logic will be added in subsequent sub-tasks. + * Central entry point for auto-approve evaluation. Classifies each tool confirmation request + * into a category using the toolType field provided by CLS, then dispatches to the registered + * handler for that category. All session/global persist logic lives in each handler. */ public class ConfirmationService { + /** Tool functional types matching CLS ToolFunctionalType enum values. */ + public enum ToolCategory { + TERMINAL("terminal"), + FILE_READ("file_read"), + FILE_WRITE("file_write"), + FILE_OPERATION("file_operation"), + MCP_TOOL("mcp_tool"), + SAFE_TOOL("safe_tool"), + WEB("web"), + UNKNOWN("unknown"); + + private final String value; + + ToolCategory(String value) { + this.value = value; + } + + public String getValue() { + return value; + } + + /** + * Resolves a CLS toolType string to a ToolCategory. + */ + public static ToolCategory fromValue(String value) { + if (value != null) { + for (ToolCategory category : values()) { + if (category.value.equals(value)) { + return category; + } + } + } + return UNKNOWN; + } + } + + private final Map handlers = + new EnumMap<>(ToolCategory.class); + private final IPreferenceStore preferenceStore; + /** - * Evaluates whether a tool confirmation request should be auto-approved. + * Creates a new ConfirmationService. * - * @param params the confirmation request parameters from CLS - * @return the confirmation result (currently always {@link ConfirmationResult#NEEDS_CONFIRMATION}) + * @param preferenceStore the preference store for reading auto-approve settings + */ + public ConfirmationService(IPreferenceStore preferenceStore) { + this.preferenceStore = preferenceStore; + handlers.put(ToolCategory.TERMINAL, + new TerminalConfirmationHandler(preferenceStore)); + } + + /** + * Evaluates whether a tool confirmation request should be auto-approved. + */ + public ConfirmationResult evaluate( + InvokeClientToolConfirmationParams params) { + ToolCategory category = classify(params); + if (category == ToolCategory.SAFE_TOOL + || category == ToolCategory.UNKNOWN) { + return ConfirmationResult.AUTO_APPROVED; + } + + ConfirmationHandler handler = handlers.get(category); + if (handler != null) { + return handler.evaluate(params); + } + return ConfirmationResult.AUTO_APPROVED; + } + + /** + * Dispatches a persist call to the handler that owns this tool category. */ - public ConfirmationResult evaluate(InvokeClientToolConfirmationParams params) { - return ConfirmationResult.NEEDS_CONFIRMATION; + public void persistDecision(ConfirmationAction action, + InvokeClientToolConfirmationParams params) { + if (action == null || action.getScope() == null + || action.getScope() == ConfirmationActionScope.ONCE) { + return; + } + ToolCategory category = classify(params); + ConfirmationHandler handler = handlers.get(category); + if (handler != null) { + handler.persistDecision(action, params); + } + } + + /** Clears session-scoped approvals for a conversation across all handlers. */ + public void clearSession(String conversationId) { + for (ConfirmationHandler handler : handlers.values()) { + handler.clearSession(conversationId); + } + } + + ToolCategory classify(InvokeClientToolConfirmationParams params) { + return ToolCategory.fromValue(extractToolType(params)); + } + + private String extractToolType( + InvokeClientToolConfirmationParams params) { + Object input = params.getInput(); + if (input instanceof Map inputMap) { + Object toolType = inputMap.get("toolType"); + if (toolType instanceof String) { + return (String) toolType; + } + } + return null; } } diff --git a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/confirmation/TerminalConfirmationHandler.java b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/confirmation/TerminalConfirmationHandler.java new file mode 100644 index 00000000..7b96a127 --- /dev/null +++ b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/confirmation/TerminalConfirmationHandler.java @@ -0,0 +1,418 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + +package com.microsoft.copilot.eclipse.ui.chat.confirmation; + +import java.lang.reflect.Type; +import java.util.ArrayList; +import java.util.Collections; +import java.util.LinkedHashSet; +import java.util.List; +import java.util.Map; +import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; +import java.util.regex.Pattern; +import java.util.regex.PatternSyntaxException; + +import com.google.gson.Gson; +import com.google.gson.reflect.TypeToken; +import org.apache.commons.lang3.StringUtils; +import org.eclipse.jface.preference.IPreferenceStore; +import org.eclipse.osgi.util.NLS; + +import com.microsoft.copilot.eclipse.core.Constants; +import com.microsoft.copilot.eclipse.core.CopilotCore; +import com.microsoft.copilot.eclipse.core.chat.ConfirmationAction; +import com.microsoft.copilot.eclipse.core.chat.ConfirmationActionScope; +import com.microsoft.copilot.eclipse.core.chat.ConfirmationContent; +import com.microsoft.copilot.eclipse.core.chat.ConfirmationResult; +import com.microsoft.copilot.eclipse.core.chat.TerminalAutoApproveRule; +import com.microsoft.copilot.eclipse.core.lsp.protocol.InvokeClientToolConfirmationParams; +import com.microsoft.copilot.eclipse.core.lsp.protocol.ToolMetadata; +import com.microsoft.copilot.eclipse.ui.chat.Messages; + +/** + * Evaluates terminal command confirmation requests against user-configured allow/deny rules. + * Rules are matched against command names provided by CLS in toolMetadata.terminalCommandData. + */ +public class TerminalConfirmationHandler implements ConfirmationHandler { + + /** Action types matching IntelliJ's TerminalAutoApproveAction enum. */ + public enum Action { + ACCEPT_NAMES_SESSION, + ACCEPT_NAMES_GLOBAL, + ACCEPT_EXACT_SESSION, + ACCEPT_EXACT_GLOBAL, + ACCEPT_ALL_SESSION + } + + static final String META_COMMAND_NAMES = "commandNames"; + static final String META_COMMAND_LINE = "commandLine"; + + /** Default deny rules for dangerous terminal commands. */ + public static final List DEFAULT_RULES = List.of( + new TerminalAutoApproveRule("rm", false), + new TerminalAutoApproveRule("rmdir", false), + new TerminalAutoApproveRule("del", false), + new TerminalAutoApproveRule("kill", false), + new TerminalAutoApproveRule("curl", false), + new TerminalAutoApproveRule("wget", false), + new TerminalAutoApproveRule("eval", false), + new TerminalAutoApproveRule("chmod", false), + new TerminalAutoApproveRule("chown", false), + new TerminalAutoApproveRule("/^Remove-Item\\b/i", false), + new TerminalAutoApproveRule("/(\\(.+\\))/s", false), + new TerminalAutoApproveRule("/`.+`/s", false), + new TerminalAutoApproveRule("/\\{.+\\}/s", false)); + + private static final Type RULES_TYPE = new TypeToken>() { + }.getType(); + + private final IPreferenceStore preferenceStore; + + // Session-scoped in-memory storage keyed by conversationId + private final ConcurrentHashMap> allowedCommandNames = + new ConcurrentHashMap<>(); + private final ConcurrentHashMap> allowedExactCommands = + new ConcurrentHashMap<>(); + private final Set allowAllConversations = ConcurrentHashMap.newKeySet(); + + /** + * Creates a new TerminalConfirmationHandler. + * + * @param preferenceStore the preference store for reading terminal auto-approve rules + */ + public TerminalConfirmationHandler(IPreferenceStore preferenceStore) { + this.preferenceStore = preferenceStore; + } + + /** + * Evaluates a terminal confirmation request. Check order follows IntelliJ: + * 1. Session "allow all" flag + * 2. Session exact commandLine match + * 3. Session command name match (all names must be approved) + * 4. Global exact commandLine match against rules + * 5. Global per-subCommand regex/prefix match against rules + * 6. Unmatched fallback (auto-approve if preference enabled) + */ + @Override + public ConfirmationResult evaluate( + InvokeClientToolConfirmationParams params) { + String convId = params.getConversationId(); + String commandLine = extractCommandLine(params); + + // 1. Session: all commands allowed for this conversation + if (allowAllConversations.contains(convId)) { + return ConfirmationResult.AUTO_APPROVED; + } + + // 2. Session: exact commandLine previously approved + Set exactSet = allowedExactCommands.get(convId); + if (commandLine != null && exactSet != null + && exactSet.contains(commandLine.trim())) { + return ConfirmationResult.AUTO_APPROVED; + } + + // 3. Session: all command names (e.g. "tree", "echo") approved + String[] cmdNames = getCommandNames(params); + Set namesSet = allowedCommandNames.get(convId); + if (cmdNames != null && namesSet != null && cmdNames.length > 0) { + boolean allApproved = true; + for (String name : cmdNames) { + if (!namesSet.contains(name)) { + allApproved = false; + break; + } + } + if (allApproved) { + return ConfirmationResult.AUTO_APPROVED; + } + } + + // 4-6. Global rules from preference store + String[] subCommands = getSubCommands(params); + if (subCommands == null || subCommands.length == 0) { + return ConfirmationResult.needsConfirmation(buildContent(params)); + } + + List rules = loadRules(); + if (rules.isEmpty()) { + return evaluateUnmatched(params); + } + + // 4. Global: exact commandLine match (for "Always Allow this + // exact command" — the full commandLine is stored as a rule) + if (commandLine != null) { + for (TerminalAutoApproveRule rule : rules) { + if (commandLine.trim().equals(rule.getCommand().trim()) + && rule.isAutoApprove()) { + return ConfirmationResult.AUTO_APPROVED; + } + } + } + + // 5. Global: per-subCommand match against regex/prefix rules. + // Any deny match → needs confirmation immediately. + // All subCommands must match an allow rule for auto-approve. + boolean allMatched = true; + for (String subCommand : subCommands) { + boolean matched = false; + for (TerminalAutoApproveRule rule : rules) { + if (matchesRule(subCommand, rule.getCommand())) { + if (!rule.isAutoApprove()) { + return ConfirmationResult.needsConfirmation(buildContent(params)); + } + matched = true; + break; + } + } + if (!matched) { + allMatched = false; + } + } + + if (allMatched) { + return ConfirmationResult.AUTO_APPROVED; + } + // 6. No rule matched — defer to unmatched preference + return evaluateUnmatched(params); + } + + private ConfirmationResult evaluateUnmatched( + InvokeClientToolConfirmationParams params) { + if (preferenceStore.getBoolean(Constants.AUTO_APPROVE_UNMATCHED_TERMINAL)) { + return ConfirmationResult.AUTO_APPROVED; + } + return ConfirmationResult.needsConfirmation(buildContent(params)); + } + + private ConfirmationContent buildContent( + InvokeClientToolConfirmationParams params) { + final String[] commandNames = getCommandNames(params); + String commandLine = extractCommandLine(params); + + List uniqueNames = dedup(commandNames); + String label = !uniqueNames.isEmpty() + ? "'" + String.join(", ", uniqueNames) + "'" : "'command'"; + String namesValue = String.join(",", uniqueNames); + + // Show exact command actions when commandLine differs from + // a single command name (otherwise redundant). + boolean showExact = commandLine != null + && !(uniqueNames.size() == 1 + && commandLine.trim().equals(uniqueNames.get(0))); + + List actions = new ArrayList<>(); + actions.add(ConfirmationAction.allowOnce(Messages.confirmation_action_allowOnce)); + if (!uniqueNames.isEmpty()) { + actions.add(action(Action.ACCEPT_NAMES_SESSION, + NLS.bind(Messages.confirmation_action_allowNamesSession, label), + ConfirmationActionScope.SESSION, + Map.of(META_COMMAND_NAMES, namesValue))); + actions.add(action(Action.ACCEPT_NAMES_GLOBAL, + NLS.bind(Messages.confirmation_action_alwaysAllowNames, label), + ConfirmationActionScope.GLOBAL, + Map.of(META_COMMAND_NAMES, namesValue))); + } + if (showExact) { + actions.add(action(Action.ACCEPT_EXACT_SESSION, + Messages.confirmation_action_allowExactSession, + ConfirmationActionScope.SESSION, + Map.of(META_COMMAND_LINE, commandLine))); + actions.add(action(Action.ACCEPT_EXACT_GLOBAL, + Messages.confirmation_action_alwaysAllowExact, + ConfirmationActionScope.GLOBAL, + Map.of(META_COMMAND_LINE, commandLine))); + } + actions.add(action(Action.ACCEPT_ALL_SESSION, + Messages.confirmation_action_allowAllCommands, + ConfirmationActionScope.SESSION, Map.of())); + actions.add(ConfirmationAction.skip(Messages.confirmation_action_skip)); + + String title = params.getTitle() != null + ? params.getTitle() : Messages.confirmation_title_terminal; + return new ConfirmationContent(title, params.getMessage(), actions); + } + + private static ConfirmationAction action(Action type, String label, + ConfirmationActionScope scope, Map extra) { + Map meta = new java.util.HashMap<>(extra); + meta.put(ConfirmationAction.META_ACTION, type.name()); + return new ConfirmationAction(label, true, scope, meta, false); + } + + private String extractCommandLine( + InvokeClientToolConfirmationParams params) { + Object input = params.getInput(); + if (input instanceof Map inputMap) { + Object cmd = inputMap.get("command"); + if (cmd instanceof String) { + return (String) cmd; + } + } + return null; + } + + private static List dedup(String[] items) { + if (items == null || items.length == 0) { + return Collections.emptyList(); + } + LinkedHashSet set = new LinkedHashSet<>(); + for (String item : items) { + if (item != null && !item.isBlank()) { + set.add(item); + } + } + return new ArrayList<>(set); + } + + private String[] getSubCommands(InvokeClientToolConfirmationParams params) { + ToolMetadata metadata = params.getToolMetadata(); + if (metadata != null && metadata.getTerminalCommandData() != null) { + return metadata.getTerminalCommandData().getSubCommands(); + } + return null; + } + + private String[] getCommandNames(InvokeClientToolConfirmationParams params) { + ToolMetadata metadata = params.getToolMetadata(); + if (metadata != null && metadata.getTerminalCommandData() != null) { + return metadata.getTerminalCommandData().getCommandNames(); + } + return null; + } + + /** + * Matches a sub-command against a rule. Exact string match is checked + * first (for exact-command rules). Then regex rules (/pattern/flags) are + * used directly, and simple rules (e.g., "rm") are converted to ^rm\b. + */ + static boolean matchesRule(String subCommand, String rulePattern) { + if (StringUtils.isBlank(subCommand) + || StringUtils.isBlank(rulePattern)) { + return false; + } + + // Exact match first + if (subCommand.trim().equals(rulePattern.trim())) { + return true; + } + + String regex; + int regexFlags = 0; + + if (rulePattern.startsWith("/") + && rulePattern.lastIndexOf('/') > 0) { + // Explicit regex: "/^git\b/i" + int lastSlash = rulePattern.lastIndexOf('/'); + regex = rulePattern.substring(1, lastSlash); + String flags = rulePattern.substring(lastSlash + 1); + if (flags.contains("i")) { + regexFlags |= Pattern.CASE_INSENSITIVE; + } + if (flags.contains("s")) { + regexFlags |= Pattern.DOTALL; + } + } else { + // Simple rule: "rm" → "^rm\b" + regex = "^" + Pattern.quote(rulePattern) + "\\b"; + } + + try { + return Pattern.compile(regex, regexFlags) + .matcher(subCommand).find(); + } catch (PatternSyntaxException e) { + CopilotCore.LOGGER.error( + "Invalid terminal auto-approve regex: " + rulePattern, e); + return false; + } + } + + List loadRules() { + String json = preferenceStore.getString(Constants.AUTO_APPROVE_TERMINAL_RULES); + if (StringUtils.isBlank(json) || "[]".equals(json.trim())) { + return Collections.emptyList(); + } + try { + List rules = new Gson().fromJson(json, RULES_TYPE); + return rules != null ? rules : Collections.emptyList(); + } catch (Exception e) { + CopilotCore.LOGGER.error("Failed to parse terminal auto-approve rules", e); + return Collections.emptyList(); + } + } + + @Override + public void persistDecision(ConfirmationAction confirmAction, + InvokeClientToolConfirmationParams params) { + String actionName = confirmAction.getMetadata() + .get(ConfirmationAction.META_ACTION); + if (actionName == null) { + return; + } + Action type; + try { + type = Action.valueOf(actionName); + } catch (IllegalArgumentException e) { + return; + } + + String convId = params.getConversationId(); + String[] cmdNames = getCommandNames(params); + String commandLine = extractCommandLine(params); + + switch (type) { + case ACCEPT_NAMES_SESSION: + if (cmdNames != null) { + Set nameSet = allowedCommandNames.computeIfAbsent( + convId, k -> ConcurrentHashMap.newKeySet()); + Collections.addAll(nameSet, cmdNames); + } + break; + case ACCEPT_EXACT_SESSION: + if (commandLine != null && !commandLine.isBlank()) { + allowedExactCommands.computeIfAbsent( + convId, k -> ConcurrentHashMap.newKeySet()) + .add(commandLine.trim()); + } + break; + case ACCEPT_ALL_SESSION: + allowAllConversations.add(convId); + break; + case ACCEPT_NAMES_GLOBAL: + if (cmdNames != null) { + addGlobalRules(List.of(cmdNames)); + } + break; + case ACCEPT_EXACT_GLOBAL: + if (commandLine != null && !commandLine.isBlank()) { + addGlobalRules(List.of(commandLine.trim())); + } + break; + default: + break; + } + } + + @Override + public void clearSession(String conversationId) { + allowedCommandNames.remove(conversationId); + allowedExactCommands.remove(conversationId); + allowAllConversations.remove(conversationId); + } + + private void addGlobalRules(List commands) { + List rules = new ArrayList<>(loadRules()); + boolean changed = false; + for (String cmd : commands) { + if (rules.stream().noneMatch(r -> r.getCommand().equals(cmd))) { + rules.add(new TerminalAutoApproveRule(cmd, true)); + changed = true; + } + } + if (changed) { + preferenceStore.setValue(Constants.AUTO_APPROVE_TERMINAL_RULES, + new Gson().toJson(rules)); + } + } +} diff --git a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/messages.properties b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/messages.properties index 30864676..9fcf2413 100644 --- a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/messages.properties +++ b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/messages.properties @@ -31,3 +31,19 @@ todoList_clearButton=Clear all todos todoList_clearButtonDisabled=Cannot clear todos while a task is in progress todoList_expandTooltip=Click to expand the todo list todoList_collapseTooltip=Click to collapse the todo list + +# Confirmation dialog action labels +confirmation_action_allowOnce=Allow Once +confirmation_action_skip=Skip +confirmation_action_allowAllCommands=Allow all commands in this Session +confirmation_action_allowNamesSession=Allow {0} in this Session +confirmation_action_alwaysAllowNames=Always Allow {0} +confirmation_action_allowExactSession=Allow this exact command in this Session +confirmation_action_alwaysAllowExact=Always Allow this exact command +confirmation_action_alwaysAllow=Always Allow + +# Confirmation dialog titles +confirmation_title_terminal=Run command in terminal + +# Misc +confirmation_autoApprovedDescription=Auto-approved from dialog 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 cbe496cc..a4be4a68 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 @@ -18,6 +18,8 @@ import com.microsoft.copilot.eclipse.core.CopilotCore; import com.microsoft.copilot.eclipse.core.chat.ChatEventsManager; +import com.microsoft.copilot.eclipse.core.chat.ConfirmationAction; +import com.microsoft.copilot.eclipse.core.chat.ConfirmationContent; import com.microsoft.copilot.eclipse.core.chat.ConfirmationResult; import com.microsoft.copilot.eclipse.core.chat.ToolInvocationListener; import com.microsoft.copilot.eclipse.core.lsp.CopilotLanguageServerConnection; @@ -33,9 +35,11 @@ import com.microsoft.copilot.eclipse.core.utils.PlatformUtils; import com.microsoft.copilot.eclipse.terminal.api.IRunInTerminalTool; import com.microsoft.copilot.eclipse.terminal.api.TerminalServiceManager; +import com.microsoft.copilot.eclipse.ui.CopilotUi; import com.microsoft.copilot.eclipse.ui.chat.BaseTurnWidget; import com.microsoft.copilot.eclipse.ui.chat.ChatContentViewer; import com.microsoft.copilot.eclipse.ui.chat.ChatView; +import com.microsoft.copilot.eclipse.ui.chat.InvokeToolConfirmationDialog; import com.microsoft.copilot.eclipse.ui.chat.confirmation.ConfirmationService; import com.microsoft.copilot.eclipse.ui.chat.tools.BaseTool; import com.microsoft.copilot.eclipse.ui.chat.tools.CreateFileTool; @@ -65,7 +69,8 @@ public class AgentToolService implements ToolInvocationListener, TerminalService public AgentToolService(CopilotLanguageServerConnection lsConnection) { this.tools = new ConcurrentHashMap<>(); this.lsConnection = lsConnection; - this.confirmationService = new ConfirmationService(); + this.confirmationService = new ConfirmationService( + CopilotUi.getPlugin().getPreferenceStore()); TerminalServiceManager terminalManager = TerminalServiceManager.getInstance(); if (terminalManager != null) { terminalManager.addListener(this); @@ -248,7 +253,8 @@ public CompletableFuture onToolConfirmation } // Auto-approve evaluation - if (confirmationService.evaluate(params) == ConfirmationResult.AUTO_APPROVED) { + ConfirmationResult autoApproveResult = confirmationService.evaluate(params); + if (autoApproveResult.isAutoApproved()) { return CompletableFuture.completedFuture( new LanguageModelToolConfirmationResult(ToolConfirmationResult.ACCEPT)); } @@ -264,13 +270,28 @@ public CompletableFuture onToolConfirmation BaseTurnWidget activeTurnWidget = turnWidget.getActiveTurnWidget(); AtomicReference> ref = new AtomicReference<>(); + ConfirmationContent content = autoApproveResult.getContent(); SwtUtils.invokeOnDisplayThread(() -> { - ref.set( - activeTurnWidget.requestToolExecutionConfirmation(params.getTitle(), params.getMessage(), params.getInput())); + ref.set(activeTurnWidget.requestToolExecutionConfirmation( + content, params.getInput())); boundChatView.getChatContentViewer().refreshScrollerLayout(); }); - return ref.get(); + CompletableFuture future = ref.get(); + if (future != null && content != null) { + // Capture dialog reference before it can be reset by a new request + final InvokeToolConfirmationDialog dialog = + activeTurnWidget.getConfirmDialog(); + future = future.thenApply(result -> { + ConfirmationAction selected = dialog != null + ? dialog.getSelectedAction() : null; + if (selected != null && selected.isAccept()) { + confirmationService.persistDecision(selected, params); + } + return result; + }); + } + return future; } private boolean validToolConfirmInvokeParams(String conversationId, String turnId) { diff --git a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/dialogs/mcp/McpServerItem.java b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/dialogs/mcp/McpServerItem.java index f59f74ec..2c15179e 100644 --- a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/dialogs/mcp/McpServerItem.java +++ b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/dialogs/mcp/McpServerItem.java @@ -45,6 +45,7 @@ import com.microsoft.copilot.eclipse.ui.dialogs.mcp.McpServerInstallManager.ActionType; import com.microsoft.copilot.eclipse.ui.dialogs.mcp.McpServerInstallManager.ButtonState; import com.microsoft.copilot.eclipse.ui.swt.CssConstants; +import com.microsoft.copilot.eclipse.ui.swt.SplitDropdownButton; import com.microsoft.copilot.eclipse.ui.utils.McpUtils; import com.microsoft.copilot.eclipse.ui.utils.UiUtils; @@ -301,15 +302,17 @@ public void widgetSelected(SelectionEvent e) { disabledInstallButton.setToolTipText(Messages.mcpServerItem_noInstallOptions); actionButton = disabledInstallButton; } else { - DropDownButton dropDownInstallButton = createDropDownInstallButton(actionComposite, initialState, + SplitDropdownButton dropDownInstallButton = createDropDownInstallButton(actionComposite, initialState, installOptions); actionButton = dropDownInstallButton.getButton(); } } } - private DropDownButton createDropDownInstallButton(Composite parent, ButtonState state, List options) { - DropDownButton dropDownInstallButton = new DropDownButton(parent, SWT.NONE); + private SplitDropdownButton createDropDownInstallButton( + Composite parent, ButtonState state, + List options) { + SplitDropdownButton dropDownInstallButton = new SplitDropdownButton(parent, SWT.NONE); dropDownInstallButton.setShowArrow(options.size() > 1); dropDownInstallButton.setText(state.getText()); diff --git a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/preferences/AddTerminalRuleDialog.java b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/preferences/AddTerminalRuleDialog.java new file mode 100644 index 00000000..1f64fbc5 --- /dev/null +++ b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/preferences/AddTerminalRuleDialog.java @@ -0,0 +1,103 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + +package com.microsoft.copilot.eclipse.ui.preferences; + +import org.eclipse.jface.dialogs.Dialog; +import org.eclipse.swt.SWT; +import org.eclipse.swt.layout.GridData; +import org.eclipse.swt.layout.GridLayout; +import org.eclipse.swt.widgets.Button; +import org.eclipse.swt.widgets.Composite; +import org.eclipse.swt.widgets.Control; +import org.eclipse.swt.widgets.Label; +import org.eclipse.swt.widgets.Shell; +import org.eclipse.swt.widgets.Text; + +/** + * Dialog for adding a terminal auto-approve rule with command and allow/deny. + */ +public class AddTerminalRuleDialog extends Dialog { + + private Text commandText; + private Button allowRadio; + + private String command; + private boolean autoApprove; + + /** + * Creates the dialog. + * + * @param parent the parent shell + */ + public AddTerminalRuleDialog(Shell parent) { + super(parent); + } + + @Override + protected void configureShell(Shell shell) { + super.configureShell(shell); + shell.setText(Messages.preferences_page_terminal_auto_approve_add_dialog_title); + } + + @Override + protected Control createDialogArea(Composite parent) { + Composite area = (Composite) super.createDialogArea(parent); + Composite container = new Composite(area, SWT.NONE); + container.setLayout(new GridLayout(2, false)); + container.setLayoutData(new GridData(SWT.FILL, SWT.FILL, true, true)); + + // Command * + Label commandLabel = new Label(container, SWT.NONE); + commandLabel.setText( + Messages.preferences_page_terminal_auto_approve_add_dialog_command + " *"); + commandText = new Text(container, SWT.BORDER); + GridData commandData = new GridData(SWT.FILL, SWT.CENTER, true, false); + commandData.widthHint = 300; + commandText.setLayoutData(commandData); + commandText.setMessage("e.g., npm, git, /^apt-get\\b/"); + commandText.addModifyListener(e -> updateOkButton()); + + // Allow / Deny + Label approveLabel = new Label(container, SWT.NONE); + approveLabel.setText( + Messages.preferences_page_terminal_auto_approve_add_dialog_approve); + Composite radioGroup = new Composite(container, SWT.NONE); + radioGroup.setLayout(new GridLayout(2, false)); + allowRadio = new Button(radioGroup, SWT.RADIO); + allowRadio.setText(Messages.preferences_page_terminal_auto_approve_allow); + allowRadio.setSelection(true); + Button denyRadio = new Button(radioGroup, SWT.RADIO); + denyRadio.setText(Messages.preferences_page_terminal_auto_approve_deny); + + return area; + } + + @Override + protected void createButtonsForButtonBar(Composite parent) { + super.createButtonsForButtonBar(parent); + updateOkButton(); + } + + private void updateOkButton() { + Button ok = getButton(OK); + if (ok != null) { + ok.setEnabled(commandText != null && !commandText.getText().trim().isEmpty()); + } + } + + @Override + protected void okPressed() { + command = commandText.getText().trim(); + autoApprove = allowRadio.getSelection(); + super.okPressed(); + } + + public String getCommand() { + return command; + } + + public boolean isAutoApprove() { + return autoApprove; + } +} 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 new file mode 100644 index 00000000..1aa5c4d5 --- /dev/null +++ b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/preferences/AutoApprovePreferencePage.java @@ -0,0 +1,55 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + +package com.microsoft.copilot.eclipse.ui.preferences; + +import org.eclipse.jface.preference.IPreferenceStore; +import org.eclipse.jface.preference.PreferencePage; +import org.eclipse.swt.SWT; +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.ui.IWorkbench; +import org.eclipse.ui.IWorkbenchPreferencePage; + +import com.microsoft.copilot.eclipse.ui.CopilotUi; + +/** + * Auto-Approve preference page for terminal command auto-approval rules. + */ +public class AutoApprovePreferencePage extends PreferencePage + implements IWorkbenchPreferencePage { + + public static final String ID = + "com.microsoft.copilot.eclipse.ui.preferences.AutoApprovePreferencePage"; + + private TerminalAutoApproveSection terminalSection; + + @Override + public void init(IWorkbench workbench) { + setPreferenceStore(CopilotUi.getPlugin().getPreferenceStore()); + noDefaultAndApplyButton(); + } + + @Override + protected Control createContents(Composite parent) { + Composite root = new Composite(parent, SWT.NONE); + root.setLayout(new GridLayout(1, false)); + root.setLayoutData(new GridData(SWT.FILL, SWT.FILL, true, true)); + + terminalSection = new TerminalAutoApproveSection(root, SWT.NONE); + + IPreferenceStore store = getPreferenceStore(); + terminalSection.loadFromPreferences(store); + + return root; + } + + @Override + public boolean performOk() { + IPreferenceStore store = getPreferenceStore(); + terminalSection.saveToPreferences(store); + return true; + } +} \ No newline at end of file diff --git a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/preferences/CopilotPreferenceInitializer.java b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/preferences/CopilotPreferenceInitializer.java index 0fe32ff0..c95f88af 100644 --- a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/preferences/CopilotPreferenceInitializer.java +++ b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/preferences/CopilotPreferenceInitializer.java @@ -3,6 +3,7 @@ package com.microsoft.copilot.eclipse.ui.preferences; +import com.google.gson.Gson; import org.eclipse.core.runtime.preferences.AbstractPreferenceInitializer; import org.eclipse.core.runtime.preferences.ConfigurationScope; import org.eclipse.core.runtime.preferences.IEclipsePreferences; @@ -10,6 +11,7 @@ import com.microsoft.copilot.eclipse.core.Constants; import com.microsoft.copilot.eclipse.ui.CopilotUi; +import com.microsoft.copilot.eclipse.ui.chat.confirmation.TerminalConfirmationHandler; /** * A class to initialize the default preferences for the plugin. @@ -56,6 +58,11 @@ public void initializeDefaultPreferences() { """); pref.setDefault(Constants.MCP_TOOLS_STATUS, "{}"); + // Auto-approve defaults + pref.setDefault(Constants.AUTO_APPROVE_TERMINAL_RULES, + new Gson().toJson(TerminalConfirmationHandler.DEFAULT_RULES)); + pref.setDefault(Constants.AUTO_APPROVE_UNMATCHED_TERMINAL, false); + IEclipsePreferences configPrefs = ConfigurationScope.INSTANCE .getNode(CopilotUi.getPlugin().getBundle().getSymbolicName()); boolean autoShowWhatsNew = configPrefs.getBoolean(Constants.AUTO_SHOW_WHAT_IS_NEW, true); diff --git a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/preferences/CopilotPreferencesPage.java b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/preferences/CopilotPreferencesPage.java index 473c9313..a6e6da99 100644 --- a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/preferences/CopilotPreferencesPage.java +++ b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/preferences/CopilotPreferencesPage.java @@ -38,6 +38,8 @@ protected Control createContents(Composite parent) { McpPreferencePage.ID); PreferencePageUtils.createPreferenceLink(getShell(), container, "Model Management", null, ByokPreferencePage.ID); + PreferencePageUtils.createPreferenceLink(getShell(), container, "Tool Auto Approve", null, + AutoApprovePreferencePage.ID); return container; } diff --git a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/preferences/LanguageServerSettingManager.java b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/preferences/LanguageServerSettingManager.java index 02f83e53..6a8e1230 100644 --- a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/preferences/LanguageServerSettingManager.java +++ b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/preferences/LanguageServerSettingManager.java @@ -5,10 +5,12 @@ import java.net.URI; import java.util.ArrayList; +import java.util.LinkedHashMap; import java.util.List; import java.util.Map; import java.util.concurrent.CompletableFuture; +import com.google.gson.Gson; import com.google.gson.JsonSyntaxException; import com.google.gson.reflect.TypeToken; import org.apache.commons.lang3.StringUtils; @@ -26,6 +28,7 @@ import com.microsoft.copilot.eclipse.core.Constants; import com.microsoft.copilot.eclipse.core.CopilotCore; import com.microsoft.copilot.eclipse.core.chat.CustomChatModeManager; +import com.microsoft.copilot.eclipse.core.chat.TerminalAutoApproveRule; import com.microsoft.copilot.eclipse.core.events.CopilotEventConstants; import com.microsoft.copilot.eclipse.core.lsp.CopilotLanguageServerConnection; import com.microsoft.copilot.eclipse.core.lsp.mcp.McpServerToolsStatusCollection; @@ -82,6 +85,10 @@ public LanguageServerSettingManager(CopilotLanguageServerConnection conn, IProxy // agent related settings getSettings().getGithubSettings().getCopilotSettings().getAgent() .setAgentMaxRequests(preferenceStore.getInt(Constants.AGENT_MAX_REQUESTS)); + getSettings().getGithubSettings().getCopilotSettings().getAgent() + .setAutoApproveUnmatchedTerminal( + preferenceStore.getBoolean(Constants.AUTO_APPROVE_UNMATCHED_TERMINAL)); + syncTerminalRulesToCls(); // Set workspace context instructions when it is enabled if (preferenceStore.getBoolean(Constants.CUSTOM_INSTRUCTIONS_WORKSPACE_ENABLED)) { @@ -168,6 +175,16 @@ public void propertyChange(PropertyChangeEvent event) { .setAgentMaxRequests(preferenceStore.getInt(Constants.AGENT_MAX_REQUESTS)); singleSetting = new CopilotLanguageServerSettings(null, null, null, settings.getGithubSettings()); break; + case Constants.AUTO_APPROVE_UNMATCHED_TERMINAL: + settings.getGithubSettings().getCopilotSettings().getAgent() + .setAutoApproveUnmatchedTerminal( + preferenceStore.getBoolean(Constants.AUTO_APPROVE_UNMATCHED_TERMINAL)); + singleSetting = new CopilotLanguageServerSettings(null, null, null, settings.getGithubSettings()); + break; + case Constants.AUTO_APPROVE_TERMINAL_RULES: + syncTerminalRulesToCls(); + singleSetting = new CopilotLanguageServerSettings(null, null, null, settings.getGithubSettings()); + break; default: return; } @@ -218,6 +235,30 @@ public void syncMcpRegistrationConfiguration() { syncSingleConfiguration(new CopilotLanguageServerSettings(null, null, null, settings.getGithubSettings())); } + /** + * Converts terminal auto-approve rules from preference store JSON to the Map format + * expected by CLS and syncs them. + */ + private void syncTerminalRulesToCls() { + String json = preferenceStore.getString(Constants.AUTO_APPROVE_TERMINAL_RULES); + Map rulesMap = new LinkedHashMap<>(); + if (StringUtils.isNotBlank(json)) { + try { + List rules = + new Gson().fromJson(json, + new TypeToken>() { + }.getType()); + if (rules != null) { + rules.forEach(r -> rulesMap.put(r.getCommand(), r.isAutoApprove())); + } + } catch (Exception e) { + CopilotCore.LOGGER.error("Failed to parse terminal rules for CLS sync", e); + } + } + settings.getGithubSettings().getCopilotSettings().getAgent() + .getTools().getTerminal().setAutoApprove(rulesMap); + } + /** * Initializes the MCP tools status from the preference store for built-in agent mode only. * Custom agent modes get their tool configuration from the LSP/file, not from preferences. diff --git a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/preferences/Messages.java b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/preferences/Messages.java index bce65aed..acea2e53 100644 --- a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/preferences/Messages.java +++ b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/preferences/Messages.java @@ -154,6 +154,25 @@ public class Messages extends NLS { public static String setting_managed_by_organization; public static String setting_disabled_by_organization; + // Terminal Auto-Approve + public static String preferences_page_terminal_auto_approve_title; + public static String preferences_page_terminal_auto_approve_description; + public static String preferences_page_terminal_auto_approve_column_command; + public static String preferences_page_terminal_auto_approve_column_status; + public static String preferences_page_terminal_auto_approve_add; + public static String preferences_page_terminal_auto_approve_remove; + public static String preferences_page_terminal_auto_approve_reset; + public static String preferences_page_terminal_auto_approve_reset_title; + public static String preferences_page_terminal_auto_approve_reset_message; + public static String preferences_page_terminal_auto_approve_allow; + public static String preferences_page_terminal_auto_approve_deny; + public static String preferences_page_terminal_auto_approve_unmatched; + public static String preferences_page_terminal_auto_approve_unmatched_note; + public static String preferences_page_terminal_auto_approve_add_dialog_title; + public static String preferences_page_terminal_auto_approve_add_dialog_message; + public static String preferences_page_terminal_auto_approve_add_dialog_command; + public static String preferences_page_terminal_auto_approve_add_dialog_approve; + static { // initialize resource bundle NLS.initializeMessages(BUNDLE_NAME, Messages.class); diff --git a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/preferences/TerminalAutoApproveSection.java b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/preferences/TerminalAutoApproveSection.java new file mode 100644 index 00000000..50265936 --- /dev/null +++ b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/preferences/TerminalAutoApproveSection.java @@ -0,0 +1,273 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + +package com.microsoft.copilot.eclipse.ui.preferences; + +import java.lang.reflect.Type; +import java.util.ArrayList; +import java.util.List; + +import com.google.gson.Gson; +import com.google.gson.reflect.TypeToken; +import org.apache.commons.lang3.StringUtils; +import org.eclipse.jface.dialogs.MessageDialog; +import org.eclipse.jface.preference.IPreferenceStore; +import org.eclipse.jface.viewers.ArrayContentProvider; +import org.eclipse.jface.viewers.ColumnLabelProvider; +import org.eclipse.jface.viewers.IStructuredSelection; +import org.eclipse.jface.viewers.TableViewer; +import org.eclipse.jface.viewers.TableViewerColumn; +import org.eclipse.swt.SWT; +import org.eclipse.swt.layout.GridData; +import org.eclipse.swt.layout.GridLayout; +import org.eclipse.swt.widgets.Button; +import org.eclipse.swt.widgets.Composite; +import org.eclipse.swt.widgets.Group; +import org.eclipse.swt.widgets.Label; +import org.eclipse.swt.widgets.Table; + +import com.microsoft.copilot.eclipse.core.Constants; +import com.microsoft.copilot.eclipse.core.chat.TerminalAutoApproveRule; +import com.microsoft.copilot.eclipse.ui.chat.confirmation.TerminalConfirmationHandler; + +/** + * Terminal auto-approve section with a rule table, action buttons, and + * unmatched-command checkbox. + */ +public class TerminalAutoApproveSection extends Composite { + + private static final int TABLE_HEIGHT_HINT = 200; + private static final Type TERMINAL_RULES_TYPE = + new TypeToken>() {}.getType(); + + private TableViewer tableViewer; + private List rules = new ArrayList<>(); + private Button removeButton; + private Button toggleButton; + private Button resetButton; + private Button unmatchedCheckbox; + + /** Creates the terminal auto-approve section inside the given parent. */ + public TerminalAutoApproveSection(Composite parent, int style) { + super(parent, style); + setLayout(new GridLayout(1, false)); + setLayoutData(new GridData(SWT.FILL, SWT.FILL, true, false)); + createContents(); + } + + private void createContents() { + Group group = new Group(this, SWT.NONE); + group.setText(Messages.preferences_page_terminal_auto_approve_title); + group.setLayout(new GridLayout(1, false)); + group.setLayoutData(new GridData(SWT.FILL, SWT.FILL, true, false)); + group.setBackgroundMode(SWT.INHERIT_FORCE); + + Label description = new Label(group, SWT.WRAP); + description.setText( + Messages.preferences_page_terminal_auto_approve_description); + GridData descData = new GridData(SWT.FILL, SWT.TOP, true, false); + descData.widthHint = 400; + description.setLayoutData(descData); + + Composite container = new Composite(group, SWT.NONE); + container.setLayout(new GridLayout(2, false)); + container.setLayoutData(new GridData(SWT.FILL, SWT.FILL, true, false)); + + createTable(container); + createButtons(container); + + unmatchedCheckbox = new Button(group, SWT.CHECK); + unmatchedCheckbox.setText( + Messages.preferences_page_terminal_auto_approve_unmatched); + unmatchedCheckbox.setLayoutData( + new GridData(SWT.FILL, SWT.TOP, true, false)); + + new WrappableNoteLabel(group, + Messages.preferences_page_note_prefix + " ", + Messages.preferences_page_terminal_auto_approve_unmatched_note); + } + + private void createTable(Composite parent) { + tableViewer = new TableViewer(parent, + SWT.BORDER | SWT.FULL_SELECTION | SWT.SINGLE); + Table table = tableViewer.getTable(); + GridData tableData = new GridData(SWT.FILL, SWT.FILL, true, false); + tableData.heightHint = TABLE_HEIGHT_HINT; + table.setLayoutData(tableData); + table.setHeaderVisible(true); + table.setLinesVisible(true); + + TableViewerColumn commandCol = + new TableViewerColumn(tableViewer, SWT.NONE); + commandCol.getColumn().setText( + Messages.preferences_page_terminal_auto_approve_column_command); + commandCol.getColumn().setWidth(300); + commandCol.setLabelProvider(new ColumnLabelProvider() { + @Override + public String getText(Object element) { + return ((TerminalAutoApproveRule) element).getCommand(); + } + }); + + TableViewerColumn statusCol = + new TableViewerColumn(tableViewer, SWT.NONE); + statusCol.getColumn().setText( + Messages.preferences_page_terminal_auto_approve_column_status); + statusCol.getColumn().setWidth(100); + statusCol.setLabelProvider(new ColumnLabelProvider() { + @Override + public String getText(Object element) { + return ((TerminalAutoApproveRule) element).isAutoApprove() + ? Messages.preferences_page_terminal_auto_approve_allow + : Messages.preferences_page_terminal_auto_approve_deny; + } + }); + + tableViewer.setContentProvider(ArrayContentProvider.getInstance()); + tableViewer.addSelectionChangedListener(e -> updateButtonState()); + } + + private void createButtons(Composite parent) { + Composite btnGroup = new Composite(parent, SWT.NONE); + btnGroup.setLayout(new GridLayout(1, false)); + btnGroup.setLayoutData( + new GridData(SWT.BEGINNING, SWT.BEGINNING, false, false)); + + Button addButton = new Button(btnGroup, SWT.PUSH); + addButton.setText(Messages.preferences_page_terminal_auto_approve_add); + addButton.setLayoutData(new GridData(SWT.FILL, SWT.TOP, true, false)); + addButton.addListener(SWT.Selection, e -> onAdd()); + + removeButton = new Button(btnGroup, SWT.PUSH); + removeButton.setText( + Messages.preferences_page_terminal_auto_approve_remove); + removeButton.setLayoutData( + new GridData(SWT.FILL, SWT.TOP, true, false)); + removeButton.setEnabled(false); + removeButton.addListener(SWT.Selection, e -> onRemove()); + + toggleButton = new Button(btnGroup, SWT.PUSH); + toggleButton.setText( + Messages.preferences_page_terminal_auto_approve_allow); + toggleButton.setLayoutData( + new GridData(SWT.FILL, SWT.TOP, true, false)); + toggleButton.setEnabled(false); + toggleButton.addListener(SWT.Selection, e -> onToggle()); + + resetButton = new Button(btnGroup, SWT.PUSH); + resetButton.setText( + Messages.preferences_page_terminal_auto_approve_reset); + resetButton.setLayoutData( + new GridData(SWT.FILL, SWT.TOP, true, false)); + resetButton.addListener(SWT.Selection, e -> onResetToDefaults()); + } + + private void onAdd() { + AddTerminalRuleDialog dialog = new AddTerminalRuleDialog(getShell()); + if (dialog.open() == AddTerminalRuleDialog.OK) { + String command = dialog.getCommand(); + // Remove existing rule with same command, then add at end + rules.removeIf(r -> r.getCommand().equals(command)); + rules.add(new TerminalAutoApproveRule(command, dialog.isAutoApprove())); + tableViewer.refresh(); + updateButtonState(); + } + } + + private void onRemove() { + IStructuredSelection sel = tableViewer.getStructuredSelection(); + if (!sel.isEmpty()) { + rules.remove(sel.getFirstElement()); + tableViewer.refresh(); + updateButtonState(); + } + } + + private void onToggle() { + IStructuredSelection sel = tableViewer.getStructuredSelection(); + if (!sel.isEmpty()) { + TerminalAutoApproveRule rule = + (TerminalAutoApproveRule) sel.getFirstElement(); + rule.setAutoApprove(!rule.isAutoApprove()); + tableViewer.refresh(); + updateButtonState(); + } + } + + private void onResetToDefaults() { + boolean confirmed = MessageDialog.openQuestion(getShell(), + Messages.preferences_page_terminal_auto_approve_reset_title, + Messages.preferences_page_terminal_auto_approve_reset_message); + if (confirmed) { + rules.clear(); + rules.addAll(TerminalConfirmationHandler.DEFAULT_RULES.stream() + .map(r -> new TerminalAutoApproveRule( + r.getCommand(), r.isAutoApprove())) + .toList()); + tableViewer.refresh(); + updateButtonState(); + } + } + + private void updateButtonState() { + boolean hasSelection = + !tableViewer.getStructuredSelection().isEmpty(); + removeButton.setEnabled(hasSelection); + toggleButton.setEnabled(hasSelection); + if (hasSelection) { + TerminalAutoApproveRule rule = (TerminalAutoApproveRule) + tableViewer.getStructuredSelection().getFirstElement(); + toggleButton.setText(rule.isAutoApprove() + ? Messages.preferences_page_terminal_auto_approve_deny + : Messages.preferences_page_terminal_auto_approve_allow); + } else { + toggleButton.setText( + Messages.preferences_page_terminal_auto_approve_allow); + } + resetButton.setEnabled(!isMatchingDefaults()); + } + + private boolean isMatchingDefaults() { + List defaults = TerminalConfirmationHandler.DEFAULT_RULES; + if (rules.size() != defaults.size()) { + return false; + } + for (int i = 0; i < rules.size(); i++) { + if (!rules.get(i).getCommand().equals(defaults.get(i).getCommand()) + || rules.get(i).isAutoApprove() != defaults.get(i).isAutoApprove()) { + return false; + } + } + return true; + } + + /** Loads terminal rules and unmatched-command preference from the store. */ + public void loadFromPreferences(IPreferenceStore store) { + String json = store.getString(Constants.AUTO_APPROVE_TERMINAL_RULES); + rules.clear(); + if (StringUtils.isNotBlank(json) && !"[]".equals(json.trim())) { + try { + List loaded = + new Gson().fromJson(json, TERMINAL_RULES_TYPE); + if (loaded != null) { + rules.addAll(loaded); + } + } catch (Exception e) { + // Invalid JSON, start with empty list + } + } + tableViewer.setInput(rules); + + unmatchedCheckbox.setSelection( + store.getBoolean(Constants.AUTO_APPROVE_UNMATCHED_TERMINAL)); + updateButtonState(); + } + + /** Saves terminal rules and unmatched-command preference to the store. */ + public void saveToPreferences(IPreferenceStore store) { + store.setValue(Constants.AUTO_APPROVE_TERMINAL_RULES, + new Gson().toJson(rules)); + store.setValue(Constants.AUTO_APPROVE_UNMATCHED_TERMINAL, + unmatchedCheckbox.getSelection()); + } +} diff --git a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/preferences/messages.properties b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/preferences/messages.properties index 2b06110a..fb67a60c 100644 --- a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/preferences/messages.properties +++ b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/preferences/messages.properties @@ -144,3 +144,23 @@ preferences_page_agent_max_requests_validation_error=Agent Max Requests must be # enterprise support setting_disabled_by_organization=This setting is disabled by your organization. setting_managed_by_organization=This setting is managed by your organization. + + +# Terminal Auto Approve +preferences_page_terminal_auto_approve_title=Terminal Auto Approve +preferences_page_terminal_auto_approve_description=Controls whether chat-initiated terminal commands are automatically approved. Set to Allow to auto approve matching commands; set to Deny to always require explicit approval. +preferences_page_terminal_auto_approve_column_command=Command +preferences_page_terminal_auto_approve_column_status=Auto Approve +preferences_page_terminal_auto_approve_add=Add... +preferences_page_terminal_auto_approve_remove=Remove +preferences_page_terminal_auto_approve_reset=Reset to Defaults +preferences_page_terminal_auto_approve_reset_title=Reset Terminal Auto Approve Rules +preferences_page_terminal_auto_approve_reset_message=This will replace all current rules with the defaults. Continue? +preferences_page_terminal_auto_approve_allow=Allow +preferences_page_terminal_auto_approve_deny=Deny +preferences_page_terminal_auto_approve_unmatched=Auto approve commands not covered by rules +preferences_page_terminal_auto_approve_unmatched_note=When enabled, terminal commands not covered by the rules above are automatically approved. Use this setting at your own discretion. +preferences_page_terminal_auto_approve_add_dialog_title=Add Terminal Command Rule +preferences_page_terminal_auto_approve_add_dialog_message=Enter the command name or regex pattern (e.g., npm, git, /^apt-get\\b/) +preferences_page_terminal_auto_approve_add_dialog_command=Command or Regex: +preferences_page_terminal_auto_approve_add_dialog_approve=Auto Approve: diff --git a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/dialogs/mcp/DropDownButton.java b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/swt/SplitDropdownButton.java similarity index 88% rename from com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/dialogs/mcp/DropDownButton.java rename to com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/swt/SplitDropdownButton.java index 55aa6301..bda4f7a6 100644 --- a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/dialogs/mcp/DropDownButton.java +++ b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/swt/SplitDropdownButton.java @@ -14,7 +14,7 @@ * The Eclipse Foundation - initial API and implementation *******************************************************************************/ -package com.microsoft.copilot.eclipse.ui.dialogs.mcp; +package com.microsoft.copilot.eclipse.ui.swt; import java.util.ArrayList; import java.util.List; @@ -36,12 +36,18 @@ import org.eclipse.swt.widgets.Shell; /** - * A drop down button from Eclipse Foundation that can show a menu when the arrow is clicked. + * A split button that distinguishes between clicking the label area (primary action) + * and clicking the arrow area (opens a dropdown menu). Originally from Eclipse Foundation. + * + *

Selection listeners receive {@code e.detail == SWT.ARROW} when the arrow + * area is clicked, and {@code e.detail == 0} for the primary area. */ -public class DropDownButton { +public class SplitDropdownButton { private boolean showArrow; + private Color separatorColor; + private Rectangle arrowBounds; private String padding = null; @@ -71,9 +77,10 @@ public void paintControl(PaintEvent e) { try { int inset = 3; int lineX = arrowBounds.x; + Color lineColor = separatorColor != null ? separatorColor : shadowColor; gc.setLineWidth(1); - gc.setForeground(shadowColor); - gc.setBackground(shadowColor); + gc.setForeground(lineColor); + gc.setBackground(lineColor); gc.drawLine(lineX, arrowBounds.y + inset - 1, lineX, e.y + buttonBounds.height - inset); Color arrowColor = button.getForeground(); @@ -93,12 +100,12 @@ public void paintControl(PaintEvent e) { }; /** - * Creates a new DropDownButton. + * Creates a new SplitButton. * * @param parent the parent composite * @param style the style of button */ - public DropDownButton(Composite parent, int style) { + public SplitDropdownButton(Composite parent, int style) { button = new Button(parent, SWT.PUSH); } @@ -125,6 +132,17 @@ public boolean isShowArrow() { return showArrow; } + /** + * Overrides the separator line color. When {@code null} (default), + * the system shadow color is used. + * + * @param color the color for the separator, or {@code null} for the default + */ + public void setSeparatorColor(Color color) { + this.separatorColor = color; + button.redraw(); + } + /** * Sets the text of the button. * @@ -167,7 +185,7 @@ public void setFont(Font font) { private void updatePadding() { String text = getText(); String currentPadding = padding; - String newPadding = showArrow ? calculatePadding(22) : null; + String newPadding = showArrow ? calculatePadding(25) : null; if ((newPadding == null && currentPadding != null) || (newPadding != null && !newPadding.equals(currentPadding))) { this.padding = newPadding; setText(text); @@ -291,7 +309,8 @@ private DropDownSelectionListenerWrapper findWrapper(final SelectionListener lis * @param listener the listener to remove */ public void removeSelectionListener(SelectionListener listener) { - DropDownSelectionListenerWrapper wrapper = findWrapper(listener); + DropDownSelectionListenerWrapper wrapper = + findWrapper(listener); if (wrapper != null) { button.removeSelectionListener(wrapper); } diff --git a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/utils/PreferencesUtils.java b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/utils/PreferencesUtils.java index efd3a7c8..fed12032 100644 --- a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/utils/PreferencesUtils.java +++ b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/utils/PreferencesUtils.java @@ -3,6 +3,7 @@ package com.microsoft.copilot.eclipse.ui.utils; +import com.microsoft.copilot.eclipse.ui.preferences.AutoApprovePreferencePage; import com.microsoft.copilot.eclipse.ui.preferences.ByokPreferencePage; import com.microsoft.copilot.eclipse.ui.preferences.ChatPreferencesPage; import com.microsoft.copilot.eclipse.ui.preferences.CompletionsPreferencesPage; @@ -24,7 +25,7 @@ private PreferencesUtils() { public static String[] getAllPreferenceIds() { return new String[] { CopilotPreferencesPage.ID, GeneralPreferencesPage.ID, ChatPreferencesPage.ID, CompletionsPreferencesPage.ID, CustomInstructionPreferencePage.ID, CustomModesPreferencePage.ID, - McpPreferencePage.ID, ByokPreferencePage.ID }; + McpPreferencePage.ID, ByokPreferencePage.ID, AutoApprovePreferencePage.ID }; } } From d79186e83fe91edd4e4b594fd3257e8d39eb376e Mon Sep 17 00:00:00 2001 From: xinyi-gong Date: Mon, 11 May 2026 14:55:47 +0800 Subject: [PATCH 2/8] filter unapproved terminal commands --- .../terminal-auto-approve.md | 56 ++++++++ .../TerminalConfirmationHandlerTests.java | 90 ++++++++++++ .../TerminalConfirmationHandler.java | 131 +++++++++++++----- 3 files changed, 245 insertions(+), 32 deletions(-) diff --git a/com.microsoft.copilot.eclipse.swtbot.test/test-plans/terminal-auto-approve/terminal-auto-approve.md b/com.microsoft.copilot.eclipse.swtbot.test/test-plans/terminal-auto-approve/terminal-auto-approve.md index f988ec22..21c9491c 100644 --- a/com.microsoft.copilot.eclipse.swtbot.test/test-plans/terminal-auto-approve/terminal-auto-approve.md +++ b/com.microsoft.copilot.eclipse.swtbot.test/test-plans/terminal-auto-approve/terminal-auto-approve.md @@ -363,3 +363,59 @@ reverts - [ ] Preference page after adding custom rules. - [ ] Preference page after reset — only defaults. - [ ] `echo hello` shows confirmation dialog post-reset. + +--- + +## 11. Already-approved names filtered from dropdown + +### TC-011: Session-approved command name hidden from dropdown +actions in multi-command scenario + +**Type:** `Edge Case` +**Priority:** `P1` + +#### Steps +1. In Agent Mode, trigger a command that includes `echo` + (e.g., "run echo hello"). +2. Confirmation dialog appears. +3. Select **"Allow 'echo' in this Session"** from the dropdown. +4. The command executes. +5. In the same conversation, send a prompt that triggers a + multi-command like `echo test && curl https://example.com`. +6. Confirmation dialog appears (because `curl` is a default deny + rule). +7. Click the dropdown arrow. + +#### Expected Result +- The dropdown shows **"Allow 'curl' in this Session"** and + **"Always Allow 'curl'"**. +- `echo` does **NOT** appear in the command-name actions — it was + already session-approved. +- "Allow all commands in this Session" is still shown. +- Exact command actions (if shown) refer to the full multi-command + string, not individual parts. + +#### 📸 Key Screenshots +- [ ] First dialog: approving `echo` in session. +- [ ] Second dialog: dropdown showing only `curl`, not `echo`. + +--- + +### TC-012: Global-approved command name hidden from dropdown + +**Type:** `Edge Case` +**Priority:** `P1` + +#### Steps +1. Open **Preferences → Tool Auto Approve**. +2. Add `echo` as **Allow** rule. Apply and close. +3. In Agent Mode, trigger `echo hello && hostname`. +4. Confirmation dialog appears (because `hostname` has no matching + rule and unmatched is disabled). +5. Click the dropdown arrow. + +#### Expected Result +- The dropdown shows **"Allow 'hostname' in this Session"** and + **"Always Allow 'hostname'"**. +- `echo` does **NOT** appear — it is globally allowed. +- "Allow all commands in this Session" is still shown. diff --git a/com.microsoft.copilot.eclipse.ui.test/src/com/microsoft/copilot/eclipse/ui/chat/confirmation/TerminalConfirmationHandlerTests.java b/com.microsoft.copilot.eclipse.ui.test/src/com/microsoft/copilot/eclipse/ui/chat/confirmation/TerminalConfirmationHandlerTests.java index 18ab98ae..8202847d 100644 --- a/com.microsoft.copilot.eclipse.ui.test/src/com/microsoft/copilot/eclipse/ui/chat/confirmation/TerminalConfirmationHandlerTests.java +++ b/com.microsoft.copilot.eclipse.ui.test/src/com/microsoft/copilot/eclipse/ui/chat/confirmation/TerminalConfirmationHandlerTests.java @@ -475,4 +475,94 @@ private static boolean hasActionType(ConfirmationAction action, && action.getMetadata().get(ConfirmationAction.META_ACTION) .equals(type.name()); } + + // --- Unapproved filtering tests --- + + @Test + void buildContent_filtersSessionApprovedNamesFromActions() { + stubRules(List.of()); + stubUnmatched(false); + + // "echo && curl" — approve echo in session first + InvokeClientToolConfirmationParams approveParams = + buildParams(new String[]{"echo hello"}, new String[]{"echo"}, + "echo hello"); + handler.persistDecision( + buildSessionAction( + TerminalConfirmationHandler.Action.ACCEPT_NAMES_SESSION), + approveParams); + + // Now evaluate "echo hello && curl example.com" + InvokeClientToolConfirmationParams params = + buildParams( + new String[]{"echo hello", "curl example.com"}, + new String[]{"echo", "curl"}, + "echo hello && curl example.com"); + ConfirmationResult result = handler.evaluate(params); + + assertFalse(result.isAutoApproved()); + List actions = result.getContent().getActions(); + + // Command-name actions should only mention "curl", not "echo" + ConfirmationAction namesAction = actions.stream() + .filter(a -> hasActionType(a, + TerminalConfirmationHandler.Action.ACCEPT_NAMES_SESSION)) + .findFirst().orElse(null); + assertNotNull(namesAction); + assertTrue(namesAction.getLabel().contains("curl")); + assertFalse(namesAction.getLabel().contains("echo")); + } + + @Test + void buildContent_filtersGlobalApprovedNamesFromActions() { + // Global allow rule for "echo" + stubRules(List.of(new TerminalAutoApproveRule("echo", true))); + stubUnmatched(false); + + // Evaluate "echo hello && hostname" + InvokeClientToolConfirmationParams params = + buildParams( + new String[]{"echo hello", "hostname"}, + new String[]{"echo", "hostname"}, + "echo hello && hostname"); + ConfirmationResult result = handler.evaluate(params); + + assertFalse(result.isAutoApproved()); + List actions = result.getContent().getActions(); + + // "echo" is globally allowed — only "hostname" in actions + ConfirmationAction namesAction = actions.stream() + .filter(a -> hasActionType(a, + TerminalConfirmationHandler.Action.ACCEPT_NAMES_SESSION)) + .findFirst().orElse(null); + assertNotNull(namesAction); + assertTrue(namesAction.getLabel().contains("hostname")); + assertFalse(namesAction.getLabel().contains("echo")); + } + + @Test + void buildContent_allNamesApproved_autoApproves() { + stubRules(List.of(new TerminalAutoApproveRule("echo", true))); + stubUnmatched(false); + + // Session-approve "curl" + InvokeClientToolConfirmationParams approveParams = + buildParams(new String[]{"curl x"}, new String[]{"curl"}, + "curl x"); + handler.persistDecision( + buildSessionAction( + TerminalConfirmationHandler.Action.ACCEPT_NAMES_SESSION), + approveParams); + + // Evaluate "echo hello && curl example.com" + // echo = global allow, curl = session allow → all approved + InvokeClientToolConfirmationParams params = + buildParams( + new String[]{"echo hello", "curl example.com"}, + new String[]{"echo", "curl"}, + "echo hello && curl example.com"); + ConfirmationResult result = handler.evaluate(params); + + assertTrue(result.isAutoApproved()); + } } diff --git a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/confirmation/TerminalConfirmationHandler.java b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/confirmation/TerminalConfirmationHandler.java index 7b96a127..41352894 100644 --- a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/confirmation/TerminalConfirmationHandler.java +++ b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/confirmation/TerminalConfirmationHandler.java @@ -46,6 +46,19 @@ public enum Action { ACCEPT_ALL_SESSION } + /** Result of evaluating sub-commands against rules. */ + enum RuleVerdict { ALL_APPROVED, DENY_BLOCKED, UNMATCHED } + + private static final class RuleResult { + final RuleVerdict verdict; + final List unapprovedItems; + + RuleResult(RuleVerdict verdict, List unapprovedItems) { + this.verdict = verdict; + this.unapprovedItems = unapprovedItems; + } + } + static final String META_COMMAND_NAMES = "commandNames"; static final String META_COMMAND_LINE = "commandLine"; @@ -129,19 +142,16 @@ public ConfirmationResult evaluate( } } - // 4-6. Global rules from preference store + // 4-6. Global rules String[] subCommands = getSubCommands(params); if (subCommands == null || subCommands.length == 0) { - return ConfirmationResult.needsConfirmation(buildContent(params)); + return ConfirmationResult.needsConfirmation( + buildContent(params, null)); } List rules = loadRules(); - if (rules.isEmpty()) { - return evaluateUnmatched(params); - } - // 4. Global: exact commandLine match (for "Always Allow this - // exact command" — the full commandLine is stored as a rule) + // 4. Exact commandLine match if (commandLine != null) { for (TerminalAutoApproveRule rule : rules) { if (commandLine.trim().equals(rule.getCommand().trim()) @@ -151,47 +161,104 @@ public ConfirmationResult evaluate( } } - // 5. Global: per-subCommand match against regex/prefix rules. - // Any deny match → needs confirmation immediately. - // All subCommands must match an allow rule for auto-approve. - boolean allMatched = true; - for (String subCommand : subCommands) { - boolean matched = false; + // 5. Per-subCommand evaluation + RuleResult result = evaluateSubCommands( + subCommands, cmdNames, rules, namesSet); + + switch (result.verdict) { + case ALL_APPROVED: + return ConfirmationResult.AUTO_APPROVED; + case DENY_BLOCKED: + return ConfirmationResult.needsConfirmation( + buildContent(params, result.unapprovedItems)); + case UNMATCHED: + default: + // 6. Unmatched fallback + if (preferenceStore.getBoolean( + Constants.AUTO_APPROVE_UNMATCHED_TERMINAL)) { + return ConfirmationResult.AUTO_APPROVED; + } + List items = result.unapprovedItems.isEmpty() + ? null : result.unapprovedItems; + return ConfirmationResult.needsConfirmation( + buildContent(params, items)); + } + } + + /** + * Evaluates each sub-command against global rules and session state. + * Returns a verdict and the list of unapproved command names. + */ + private RuleResult evaluateSubCommands(String[] subCommands, + String[] cmdNames, List rules, + Set sessionNames) { + boolean allApproved = true; + boolean hasDeny = false; + List unapproved = new ArrayList<>(); + + for (int i = 0; i < subCommands.length; i++) { + String subCommand = subCommands[i]; + String cmdName = cmdNames != null && i < cmdNames.length + ? cmdNames[i] : null; + boolean hasAllow = false; + boolean denied = false; + + // Session command-name approval + if (cmdName != null && sessionNames != null + && sessionNames.contains(cmdName)) { + hasAllow = true; + } + + // Global rule matching for (TerminalAutoApproveRule rule : rules) { if (matchesRule(subCommand, rule.getCommand())) { - if (!rule.isAutoApprove()) { - return ConfirmationResult.needsConfirmation(buildContent(params)); + if (rule.isAutoApprove()) { + hasAllow = true; + } else { + denied = true; } - matched = true; break; } } - if (!matched) { - allMatched = false; + + if (denied && !hasAllow) { + hasDeny = true; + if (cmdName != null && !unapproved.contains(cmdName)) { + unapproved.add(cmdName); + } + } else if (!hasAllow) { + allApproved = false; + if (cmdName != null && !unapproved.contains(cmdName)) { + unapproved.add(cmdName); + } } } - if (allMatched) { - return ConfirmationResult.AUTO_APPROVED; - } - // 6. No rule matched — defer to unmatched preference - return evaluateUnmatched(params); - } - - private ConfirmationResult evaluateUnmatched( - InvokeClientToolConfirmationParams params) { - if (preferenceStore.getBoolean(Constants.AUTO_APPROVE_UNMATCHED_TERMINAL)) { - return ConfirmationResult.AUTO_APPROVED; + RuleVerdict verdict; + if (hasDeny) { + verdict = RuleVerdict.DENY_BLOCKED; + } else if (allApproved) { + verdict = RuleVerdict.ALL_APPROVED; + } else { + verdict = RuleVerdict.UNMATCHED; } - return ConfirmationResult.needsConfirmation(buildContent(params)); + return new RuleResult(verdict, unapproved); } + /** + * Builds the confirmation dialog content. When {@code unapprovedNames} + * is non-null, only those names appear in the command-name actions + * (already-approved names are filtered out). + */ private ConfirmationContent buildContent( - InvokeClientToolConfirmationParams params) { + InvokeClientToolConfirmationParams params, + List unapprovedNames) { final String[] commandNames = getCommandNames(params); String commandLine = extractCommandLine(params); - List uniqueNames = dedup(commandNames); + // Use unapproved names if available, otherwise all unique names + List uniqueNames = unapprovedNames != null + ? unapprovedNames : dedup(commandNames); String label = !uniqueNames.isEmpty() ? "'" + String.join(", ", uniqueNames) + "'" : "'command'"; String namesValue = String.join(",", uniqueNames); From 32c416b4f56262ced47cb1c207b742acb0e908b9 Mon Sep 17 00:00:00 2001 From: xinyi-gong Date: Mon, 11 May 2026 16:29:23 +0800 Subject: [PATCH 3/8] let subagents inherit all the session rules of main agent --- .../terminal-auto-approve.md | 95 +++++++++++++++++++ .../TerminalConfirmationHandlerTests.java | 58 +++++------ .../confirmation/ConfirmationHandler.java | 12 ++- .../confirmation/ConfirmationService.java | 17 +++- .../TerminalConfirmationHandler.java | 10 +- .../ui/chat/services/AgentToolService.java | 18 +++- 6 files changed, 168 insertions(+), 42 deletions(-) diff --git a/com.microsoft.copilot.eclipse.swtbot.test/test-plans/terminal-auto-approve/terminal-auto-approve.md b/com.microsoft.copilot.eclipse.swtbot.test/test-plans/terminal-auto-approve/terminal-auto-approve.md index 21c9491c..3235dd0f 100644 --- a/com.microsoft.copilot.eclipse.swtbot.test/test-plans/terminal-auto-approve/terminal-auto-approve.md +++ b/com.microsoft.copilot.eclipse.swtbot.test/test-plans/terminal-auto-approve/terminal-auto-approve.md @@ -419,3 +419,98 @@ actions in multi-command scenario **"Always Allow 'hostname'"**. - `echo` does **NOT** appear — it is globally allowed. - "Allow all commands in this Session" is still shown. + +--- + +## 13. Subagent inherits parent session approvals + +### TC-013: Session approval in main agent applies to subagent calls + +**Type:** `Edge Case` +**Priority:** `P1` + +#### Preconditions +- No custom allow rules for `echo` or `cat` (only defaults). +- "Auto approve commands not covered by rules" is **unchecked**. + +#### Steps +1. In Agent Mode, send a prompt that triggers `echo hello`. +2. Confirmation dialog appears. +3. Click dropdown and select **"Allow 'echo' in this Session"**. +4. The command executes. +5. In the **same conversation**, send a complex prompt that causes the + agent to spawn a **subagent** (e.g., `use a subagent to run echo + from subagent`). +6. The subagent invokes `run_in_terminal` with an `echo` command. +7. Observe that **no confirmation dialog appears** — the session + approval from the main agent carries over to the subagent. +8. Still in the subagent context, the subagent invokes a different + command (e.g., `cat somefile.txt`). +9. Observe that a **confirmation dialog appears** — `cat` was not + session-approved. + +#### Expected Result +- Subagent tool calls use the parent conversation's session rules. +- Commands approved in the main agent conversation auto-approve in + subagent context. +- Commands NOT approved still require confirmation in subagent context. + +#### 📸 Key Screenshots +- [ ] Main agent: approving `echo` in session. +- [ ] Subagent: `echo` auto-approved — no dialog. +- [ ] Subagent: `cat` shows confirmation dialog. + +--- + +### TC-014: Session approval in subagent carries back to main agent + +**Type:** `Edge Case` +**Priority:** `P1` + +#### Preconditions +- No custom allow rules for `hostname`. +- "Auto approve commands not covered by rules" is **unchecked**. + +#### Steps +1. In Agent Mode, send a complex prompt that triggers a **subagent**. +2. The subagent invokes `run_in_terminal` with `hostname`. +3. Confirmation dialog appears. +4. Click dropdown and select **"Allow 'hostname' in this Session"**. +5. The command executes in the subagent context. +6. After the subagent completes, continue in the **same conversation** + with the main agent. +7. Send a prompt that triggers `hostname` again (e.g., `what is my + hostname?`). +8. Observe that **no confirmation dialog appears** — the session + approval made during the subagent call is shared with the main + conversation. + +#### Expected Result +- Session approvals made during subagent execution are stored under + the parent conversation's session scope. +- The main agent benefits from approvals granted in subagent context. + +#### 📸 Key Screenshots +- [ ] Subagent: approving `hostname` in session. +- [ ] Main agent: `hostname` auto-approved — no dialog. + +--- + +### TC-015: "Allow all commands in this Session" in main agent covers subagent + +**Type:** `Edge Case` +**Priority:** `P2` + +#### Steps +1. In Agent Mode, trigger any terminal command. +2. Confirmation dialog appears. +3. Select **"Allow all commands in this Session"** from the dropdown. +4. In the **same conversation**, send a prompt that spawns a subagent + which runs multiple different terminal commands. +5. Observe that **none of them** show a confirmation dialog — the + blanket session approval covers subagent calls too. + +#### Expected Result +- "Allow all commands in this Session" is a blanket approval that + applies to both main agent and subagent tool calls within the + same conversation. diff --git a/com.microsoft.copilot.eclipse.ui.test/src/com/microsoft/copilot/eclipse/ui/chat/confirmation/TerminalConfirmationHandlerTests.java b/com.microsoft.copilot.eclipse.ui.test/src/com/microsoft/copilot/eclipse/ui/chat/confirmation/TerminalConfirmationHandlerTests.java index 8202847d..1412d0c9 100644 --- a/com.microsoft.copilot.eclipse.ui.test/src/com/microsoft/copilot/eclipse/ui/chat/confirmation/TerminalConfirmationHandlerTests.java +++ b/com.microsoft.copilot.eclipse.ui.test/src/com/microsoft/copilot/eclipse/ui/chat/confirmation/TerminalConfirmationHandlerTests.java @@ -108,7 +108,7 @@ void evaluate_autoApprovedWhenAllSubCommandsMatchAllowRules() { InvokeClientToolConfirmationParams params = buildParams(new String[]{"echo hello"}, new String[]{"echo"}, "echo hello"); - ConfirmationResult result = handler.evaluate(params); + ConfirmationResult result = handler.evaluate(params, CONV_ID); assertTrue(result.isAutoApproved()); } @@ -121,7 +121,7 @@ void evaluate_needsConfirmationWhenDenyRuleMatches() { InvokeClientToolConfirmationParams params = buildParams(new String[]{"rm -rf /"}, new String[]{"rm"}, "rm -rf /"); - ConfirmationResult result = handler.evaluate(params); + ConfirmationResult result = handler.evaluate(params, CONV_ID); assertFalse(result.isAutoApproved()); assertNotNull(result.getContent()); @@ -135,7 +135,7 @@ void evaluate_needsConfirmationWhenNoRulesMatchAndUnmatchedFalse() { InvokeClientToolConfirmationParams params = buildParams(new String[]{"ls -la"}, new String[]{"ls"}, "ls -la"); - ConfirmationResult result = handler.evaluate(params); + ConfirmationResult result = handler.evaluate(params, CONV_ID); assertFalse(result.isAutoApproved()); } @@ -148,7 +148,7 @@ void evaluate_autoApprovedWhenNoRulesMatchAndUnmatchedTrue() { InvokeClientToolConfirmationParams params = buildParams(new String[]{"ls -la"}, new String[]{"ls"}, "ls -la"); - ConfirmationResult result = handler.evaluate(params); + ConfirmationResult result = handler.evaluate(params, CONV_ID); assertTrue(result.isAutoApproved()); } @@ -159,7 +159,7 @@ void evaluate_needsConfirmationWhenSubCommandsNull() { InvokeClientToolConfirmationParams params = buildParams(null, null, "echo hello"); - ConfirmationResult result = handler.evaluate(params); + ConfirmationResult result = handler.evaluate(params, CONV_ID); assertFalse(result.isAutoApproved()); } @@ -170,7 +170,7 @@ void evaluate_needsConfirmationWhenSubCommandsEmpty() { InvokeClientToolConfirmationParams params = buildParams(new String[]{}, null, "echo hello"); - ConfirmationResult result = handler.evaluate(params); + ConfirmationResult result = handler.evaluate(params, CONV_ID); assertFalse(result.isAutoApproved()); } @@ -182,7 +182,7 @@ void evaluate_emptyRulesUsesUnmatchedSetting() { InvokeClientToolConfirmationParams params = buildParams(new String[]{"ls"}, new String[]{"ls"}, "ls"); - ConfirmationResult result = handler.evaluate(params); + ConfirmationResult result = handler.evaluate(params, CONV_ID); assertTrue(result.isAutoApproved()); } @@ -200,9 +200,9 @@ void persistDecision_acceptAllSession_autoApprovesSubsequent() { ConfirmationAction allSession = buildSessionAction( TerminalConfirmationHandler.Action.ACCEPT_ALL_SESSION); - handler.persistDecision(allSession, params); + handler.persistDecision(allSession, params, CONV_ID); - ConfirmationResult result = handler.evaluate(params); + ConfirmationResult result = handler.evaluate(params, CONV_ID); assertTrue(result.isAutoApproved()); } @@ -217,9 +217,9 @@ void persistDecision_acceptNamesSession_autoApprovesMatchingNames() { ConfirmationAction namesSession = buildSessionAction( TerminalConfirmationHandler.Action.ACCEPT_NAMES_SESSION); - handler.persistDecision(namesSession, params); + handler.persistDecision(namesSession, params, CONV_ID); - ConfirmationResult result = handler.evaluate(params); + ConfirmationResult result = handler.evaluate(params, CONV_ID); assertTrue(result.isAutoApproved()); } @@ -234,9 +234,9 @@ void persistDecision_acceptExactSession_autoApprovesMatchingCommand() { ConfirmationAction exactSession = buildSessionAction( TerminalConfirmationHandler.Action.ACCEPT_EXACT_SESSION); - handler.persistDecision(exactSession, params); + handler.persistDecision(exactSession, params, CONV_ID); - ConfirmationResult result = handler.evaluate(params); + ConfirmationResult result = handler.evaluate(params, CONV_ID); assertTrue(result.isAutoApproved()); } @@ -251,11 +251,11 @@ void clearSession_removesApprovalsForConversation() { ConfirmationAction allSession = buildSessionAction( TerminalConfirmationHandler.Action.ACCEPT_ALL_SESSION); - handler.persistDecision(allSession, params); + handler.persistDecision(allSession, params, CONV_ID); handler.clearSession(CONV_ID); - ConfirmationResult result = handler.evaluate(params); + ConfirmationResult result = handler.evaluate(params, CONV_ID); assertFalse(result.isAutoApproved()); } @@ -270,11 +270,11 @@ void clearSession_doesNotAffectOtherConversation() { ConfirmationAction allSession = buildSessionAction( TerminalConfirmationHandler.Action.ACCEPT_ALL_SESSION); - handler.persistDecision(allSession, params); + handler.persistDecision(allSession, params, CONV_ID); handler.clearSession("other-conv"); - ConfirmationResult result = handler.evaluate(params); + ConfirmationResult result = handler.evaluate(params, CONV_ID); assertTrue(result.isAutoApproved()); } @@ -288,7 +288,7 @@ void buildContent_alwaysHasAllowOnceAsPrimary() { InvokeClientToolConfirmationParams params = buildParams(new String[]{"echo"}, new String[]{"echo"}, "echo hello"); - ConfirmationResult result = handler.evaluate(params); + ConfirmationResult result = handler.evaluate(params, CONV_ID); ConfirmationContent content = result.getContent(); assertNotNull(content); @@ -306,7 +306,7 @@ void buildContent_alwaysHasSkipAsDismiss() { InvokeClientToolConfirmationParams params = buildParams(new String[]{"echo"}, new String[]{"echo"}, "echo hello"); - ConfirmationResult result = handler.evaluate(params); + ConfirmationResult result = handler.evaluate(params, CONV_ID); List actions = result.getContent().getActions(); ConfirmationAction last = actions.get(actions.size() - 1); @@ -321,7 +321,7 @@ void buildContent_hasAllowAllSessionAction() { InvokeClientToolConfirmationParams params = buildParams(new String[]{"echo"}, new String[]{"echo"}, "echo hello"); - ConfirmationResult result = handler.evaluate(params); + ConfirmationResult result = handler.evaluate(params, CONV_ID); List actions = result.getContent().getActions(); boolean hasAllSession = actions.stream().anyMatch(a -> @@ -341,7 +341,7 @@ void buildContent_hasCommandNameActionsWhenNamesPresent() { InvokeClientToolConfirmationParams params = buildParams(new String[]{"echo"}, new String[]{"echo"}, "echo hello"); - ConfirmationResult result = handler.evaluate(params); + ConfirmationResult result = handler.evaluate(params, CONV_ID); List actions = result.getContent().getActions(); boolean hasNamesSession = actions.stream().anyMatch(a -> @@ -363,7 +363,7 @@ void buildContent_hasExactCommandActionsWhenDifferentFromName() { InvokeClientToolConfirmationParams params = buildParams(new String[]{"echo"}, new String[]{"echo"}, "echo hello"); - ConfirmationResult result = handler.evaluate(params); + ConfirmationResult result = handler.evaluate(params, CONV_ID); List actions = result.getContent().getActions(); boolean hasExactSession = actions.stream().anyMatch(a -> @@ -384,7 +384,7 @@ void buildContent_noExactActionsWhenSingleSubCommandEqualsName() { // commandLine equals the single commandName InvokeClientToolConfirmationParams params = buildParams(new String[]{"echo"}, new String[]{"echo"}, "echo"); - ConfirmationResult result = handler.evaluate(params); + ConfirmationResult result = handler.evaluate(params, CONV_ID); List actions = result.getContent().getActions(); boolean hasExact = actions.stream().anyMatch(a -> @@ -403,7 +403,7 @@ void buildContent_actionScopesAreCorrect() { InvokeClientToolConfirmationParams params = buildParams(new String[]{"echo"}, new String[]{"echo"}, "echo hello"); - ConfirmationResult result = handler.evaluate(params); + ConfirmationResult result = handler.evaluate(params, CONV_ID); List actions = result.getContent().getActions(); @@ -490,7 +490,7 @@ void buildContent_filtersSessionApprovedNamesFromActions() { handler.persistDecision( buildSessionAction( TerminalConfirmationHandler.Action.ACCEPT_NAMES_SESSION), - approveParams); + approveParams, CONV_ID); // Now evaluate "echo hello && curl example.com" InvokeClientToolConfirmationParams params = @@ -498,7 +498,7 @@ void buildContent_filtersSessionApprovedNamesFromActions() { new String[]{"echo hello", "curl example.com"}, new String[]{"echo", "curl"}, "echo hello && curl example.com"); - ConfirmationResult result = handler.evaluate(params); + ConfirmationResult result = handler.evaluate(params, CONV_ID); assertFalse(result.isAutoApproved()); List actions = result.getContent().getActions(); @@ -525,7 +525,7 @@ void buildContent_filtersGlobalApprovedNamesFromActions() { new String[]{"echo hello", "hostname"}, new String[]{"echo", "hostname"}, "echo hello && hostname"); - ConfirmationResult result = handler.evaluate(params); + ConfirmationResult result = handler.evaluate(params, CONV_ID); assertFalse(result.isAutoApproved()); List actions = result.getContent().getActions(); @@ -552,7 +552,7 @@ void buildContent_allNamesApproved_autoApproves() { handler.persistDecision( buildSessionAction( TerminalConfirmationHandler.Action.ACCEPT_NAMES_SESSION), - approveParams); + approveParams, CONV_ID); // Evaluate "echo hello && curl example.com" // echo = global allow, curl = session allow → all approved @@ -561,7 +561,7 @@ void buildContent_allNamesApproved_autoApproves() { new String[]{"echo hello", "curl example.com"}, new String[]{"echo", "curl"}, "echo hello && curl example.com"); - ConfirmationResult result = handler.evaluate(params); + ConfirmationResult result = handler.evaluate(params, CONV_ID); assertTrue(result.isAutoApproved()); } diff --git a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/confirmation/ConfirmationHandler.java b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/confirmation/ConfirmationHandler.java index a389e184..2bd4d25a 100644 --- a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/confirmation/ConfirmationHandler.java +++ b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/confirmation/ConfirmationHandler.java @@ -18,9 +18,13 @@ public interface ConfirmationHandler { * Implementations should check both global rules and session memory. * * @param params the confirmation request parameters from CLS + * @param sessionConversationId the conversation ID to use for session-scoped + * lookups (may differ from params.getConversationId() when called from a + * subagent context) * @return the confirmation result */ - ConfirmationResult evaluate(InvokeClientToolConfirmationParams params); + ConfirmationResult evaluate(InvokeClientToolConfirmationParams params, + String sessionConversationId); /** * Persists a user's decision based on the action scope. @@ -28,10 +32,12 @@ public interface ConfirmationHandler { * GLOBAL actions are written to preferences. * * @param action the user's selected action with metadata - * @param params the original confirmation params (for conversationId etc.) + * @param params the original confirmation params (for command data etc.) + * @param sessionConversationId the conversation ID to use for session storage */ default void persistDecision(ConfirmationAction action, - InvokeClientToolConfirmationParams params) { + InvokeClientToolConfirmationParams params, + String sessionConversationId) { // no-op by default } diff --git a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/confirmation/ConfirmationService.java b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/confirmation/ConfirmationService.java index 66a725b6..247ad1c3 100644 --- a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/confirmation/ConfirmationService.java +++ b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/confirmation/ConfirmationService.java @@ -74,9 +74,13 @@ public ConfirmationService(IPreferenceStore preferenceStore) { /** * Evaluates whether a tool confirmation request should be auto-approved. + * + * @param params the confirmation request parameters + * @param sessionConversationId the conversation ID for session-scoped lookups */ public ConfirmationResult evaluate( - InvokeClientToolConfirmationParams params) { + InvokeClientToolConfirmationParams params, + String sessionConversationId) { ToolCategory category = classify(params); if (category == ToolCategory.SAFE_TOOL || category == ToolCategory.UNKNOWN) { @@ -85,16 +89,21 @@ public ConfirmationResult evaluate( ConfirmationHandler handler = handlers.get(category); if (handler != null) { - return handler.evaluate(params); + return handler.evaluate(params, sessionConversationId); } return ConfirmationResult.AUTO_APPROVED; } /** * Dispatches a persist call to the handler that owns this tool category. + * + * @param action the user's selected action + * @param params the original confirmation params + * @param sessionConversationId the conversation ID for session storage */ public void persistDecision(ConfirmationAction action, - InvokeClientToolConfirmationParams params) { + InvokeClientToolConfirmationParams params, + String sessionConversationId) { if (action == null || action.getScope() == null || action.getScope() == ConfirmationActionScope.ONCE) { return; @@ -102,7 +111,7 @@ public void persistDecision(ConfirmationAction action, ToolCategory category = classify(params); ConfirmationHandler handler = handlers.get(category); if (handler != null) { - handler.persistDecision(action, params); + handler.persistDecision(action, params, sessionConversationId); } } diff --git a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/confirmation/TerminalConfirmationHandler.java b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/confirmation/TerminalConfirmationHandler.java index 41352894..6f18e034 100644 --- a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/confirmation/TerminalConfirmationHandler.java +++ b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/confirmation/TerminalConfirmationHandler.java @@ -110,8 +110,9 @@ public TerminalConfirmationHandler(IPreferenceStore preferenceStore) { */ @Override public ConfirmationResult evaluate( - InvokeClientToolConfirmationParams params) { - String convId = params.getConversationId(); + InvokeClientToolConfirmationParams params, + String sessionConversationId) { + String convId = sessionConversationId; String commandLine = extractCommandLine(params); // 1. Session: all commands allowed for this conversation @@ -411,7 +412,8 @@ List loadRules() { @Override public void persistDecision(ConfirmationAction confirmAction, - InvokeClientToolConfirmationParams params) { + InvokeClientToolConfirmationParams params, + String sessionConversationId) { String actionName = confirmAction.getMetadata() .get(ConfirmationAction.META_ACTION); if (actionName == null) { @@ -424,7 +426,7 @@ public void persistDecision(ConfirmationAction confirmAction, return; } - String convId = params.getConversationId(); + String convId = sessionConversationId; String[] cmdNames = getCommandNames(params); String commandLine = extractCommandLine(params); 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 a4be4a68..a770e70d 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 @@ -252,8 +252,20 @@ public CompletableFuture onToolConfirmation return CompletableFuture.completedFuture(result); } + // Resolve the session conversation ID: map subagent conversations to the + // parent so that session-scoped approvals apply to the whole chat. + String sessionConversationId = params.getConversationId(); + if (boundChatView != null + && !Objects.equals(sessionConversationId, + boundChatView.getConversationId()) + && Objects.equals(sessionConversationId, + boundChatView.getSubagentConversationId())) { + sessionConversationId = boundChatView.getConversationId(); + } + // Auto-approve evaluation - ConfirmationResult autoApproveResult = confirmationService.evaluate(params); + ConfirmationResult autoApproveResult = + confirmationService.evaluate(params, sessionConversationId); if (autoApproveResult.isAutoApproved()) { return CompletableFuture.completedFuture( new LanguageModelToolConfirmationResult(ToolConfirmationResult.ACCEPT)); @@ -282,11 +294,13 @@ public CompletableFuture onToolConfirmation // Capture dialog reference before it can be reset by a new request final InvokeToolConfirmationDialog dialog = activeTurnWidget.getConfirmDialog(); + final String sessConvId = sessionConversationId; future = future.thenApply(result -> { ConfirmationAction selected = dialog != null ? dialog.getSelectedAction() : null; if (selected != null && selected.isAccept()) { - confirmationService.persistDecision(selected, params); + confirmationService.persistDecision(selected, params, + sessConvId); } return result; }); From 69e3a172ed70f6c5a303deff7eecdaaad73f4992 Mon Sep 17 00:00:00 2001 From: xinyi-gong Date: Mon, 11 May 2026 17:15:08 +0800 Subject: [PATCH 4/8] resolve comments --- .../terminal-auto-approve.md | 35 ++++++++ .../TerminalConfirmationHandlerTests.java | 56 +++++++++++++ com.microsoft.copilot.eclipse.ui/plugin.xml | 8 +- .../ui/chat/InvokeToolConfirmationDialog.java | 6 +- .../confirmation/ConfirmationService.java | 8 +- .../TerminalConfirmationHandler.java | 84 +++++++++++++++---- .../ui/preferences/AddTerminalRuleDialog.java | 2 +- .../eclipse/ui/preferences/Messages.java | 1 + .../TerminalAutoApproveSection.java | 4 +- .../ui/preferences/messages.properties | 1 + 10 files changed, 176 insertions(+), 29 deletions(-) diff --git a/com.microsoft.copilot.eclipse.swtbot.test/test-plans/terminal-auto-approve/terminal-auto-approve.md b/com.microsoft.copilot.eclipse.swtbot.test/test-plans/terminal-auto-approve/terminal-auto-approve.md index 3235dd0f..180783a4 100644 --- a/com.microsoft.copilot.eclipse.swtbot.test/test-plans/terminal-auto-approve/terminal-auto-approve.md +++ b/com.microsoft.copilot.eclipse.swtbot.test/test-plans/terminal-auto-approve/terminal-auto-approve.md @@ -514,3 +514,38 @@ actions in multi-command scenario - "Allow all commands in this Session" is a blanket approval that applies to both main agent and subagent tool calls within the same conversation. + +--- + +## 14. Edge Cases + +### TC-016: "Always Allow" overrides existing deny rule in preferences + +**Type:** `Edge Case` +**Priority:** `P1` + +#### Steps +1. Open **Preferences → Tool Auto Approve**. +2. Click **"Add..."**, enter `curl`, select **"Deny"**, click OK. +3. Click **"Apply and Close"**. +4. In Agent Mode, type a prompt that triggers `curl` (e.g., `fetch + https://example.com using curl`). +5. Confirmation dialog appears (deny rule blocks it). +6. Click dropdown and select **"Always Allow curl"**. +7. The command executes. +8. Open **Preferences → Tool Auto Approve** again. +9. Verify the `curl` rule has been changed from **Deny → Allow**. +10. Start a **new conversation**, trigger `curl` again. +11. Observe **auto-approved** — the global allow rule now applies. + +#### Expected Result +- Clicking "Always Allow" in the dialog overrides an existing deny + rule in preferences, changing it from deny to allow. +- The updated rule persists across conversations. +- The preferences table reflects the updated rule. + +#### 📸 Key Screenshots +- [ ] Preferences: `curl → Deny` rule before override. +- [ ] Confirmation dialog: showing "Always Allow curl" action. +- [ ] Preferences: `curl → Allow` rule after override. +- [ ] New conversation: `curl` auto-approved without dialog. diff --git a/com.microsoft.copilot.eclipse.ui.test/src/com/microsoft/copilot/eclipse/ui/chat/confirmation/TerminalConfirmationHandlerTests.java b/com.microsoft.copilot.eclipse.ui.test/src/com/microsoft/copilot/eclipse/ui/chat/confirmation/TerminalConfirmationHandlerTests.java index 1412d0c9..ffc8892b 100644 --- a/com.microsoft.copilot.eclipse.ui.test/src/com/microsoft/copilot/eclipse/ui/chat/confirmation/TerminalConfirmationHandlerTests.java +++ b/com.microsoft.copilot.eclipse.ui.test/src/com/microsoft/copilot/eclipse/ui/chat/confirmation/TerminalConfirmationHandlerTests.java @@ -565,4 +565,60 @@ void buildContent_allNamesApproved_autoApproves() { assertTrue(result.isAutoApproved()); } + + @Test + void sessionRules_surviveConversationSwitch() { + // Approve "echo" in conversation A + InvokeClientToolConfirmationParams approveParams = + buildParams(new String[]{"echo hello"}, new String[]{"echo"}, + "echo hello"); + handler.persistDecision( + buildSessionAction( + TerminalConfirmationHandler.Action.ACCEPT_NAMES_SESSION), + approveParams, "conv-A"); + + // Switch to conversation B, then back to A — rule should survive + InvokeClientToolConfirmationParams params = + buildParams(new String[]{"echo world"}, new String[]{"echo"}, + "echo world"); + ConfirmationResult result = handler.evaluate(params, "conv-A"); + assertTrue(result.isAutoApproved()); + } + + @Test + void sessionRules_evictOldestWhenCapExceeded() { + // Fill up to MAX_SESSION_CONVERSATIONS with unique conversation IDs + for (int i = 0; i < TerminalConfirmationHandler.MAX_SESSION_CONVERSATIONS; + i++) { + InvokeClientToolConfirmationParams p = + buildParams(new String[]{"echo"}, new String[]{"echo"}, "echo"); + handler.persistDecision( + buildSessionAction( + TerminalConfirmationHandler.Action.ACCEPT_NAMES_SESSION), + p, "conv-" + i); + } + + // Add one more — should evict the oldest (conv-0) + InvokeClientToolConfirmationParams p = + buildParams(new String[]{"echo"}, new String[]{"echo"}, "echo"); + handler.persistDecision( + buildSessionAction( + TerminalConfirmationHandler.Action.ACCEPT_NAMES_SESSION), + p, "conv-new"); + + // conv-0 should have been evicted + InvokeClientToolConfirmationParams params = + buildParams(new String[]{"echo test"}, new String[]{"echo"}, + "echo test"); + ConfirmationResult evicted = handler.evaluate(params, "conv-0"); + assertFalse(evicted.isAutoApproved()); + + // conv-new should still work + ConfirmationResult kept = handler.evaluate(params, "conv-new"); + assertTrue(kept.isAutoApproved()); + + // conv-1 (second oldest, not evicted) should still work + ConfirmationResult second = handler.evaluate(params, "conv-1"); + assertTrue(second.isAutoApproved()); + } } diff --git a/com.microsoft.copilot.eclipse.ui/plugin.xml b/com.microsoft.copilot.eclipse.ui/plugin.xml index 3d75c0b6..a3d4a20a 100644 --- a/com.microsoft.copilot.eclipse.ui/plugin.xml +++ b/com.microsoft.copilot.eclipse.ui/plugin.xml @@ -1054,12 +1054,10 @@ + name="com.microsoft.copilot.eclipse.ui.nes.change"> + name="com.microsoft.copilot.eclipse.ui.nes.delete"> @@ -1119,4 +1117,4 @@ %theme.chatFont.description - \ No newline at end of file + diff --git a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/InvokeToolConfirmationDialog.java b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/InvokeToolConfirmationDialog.java index fc4634ea..ff797e29 100644 --- a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/InvokeToolConfirmationDialog.java +++ b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/InvokeToolConfirmationDialog.java @@ -265,6 +265,9 @@ public void widgetSelected(SelectionEvent e) { item.addListener(SWT.Selection, ev -> acceptAndDispose(action)); } + menu.addListener(SWT.Hide, ev -> { + ev.display.asyncExec(menu::dispose); + }); Rectangle bounds = primaryBtn.getBounds(); Point loc = primaryBtn.getParent() .toDisplay(bounds.x, bounds.y + bounds.height); @@ -281,7 +284,8 @@ public void widgetSelected(SelectionEvent e) { dismissBtn.setLayoutData( new GridData(SWT.BEGINNING, SWT.CENTER, false, false)); dismissBtn.setText( - dismissAction != null ? dismissAction.getLabel() : "Cancel"); + dismissAction != null ? dismissAction.getLabel() + : Messages.confirmation_action_skip); registerControlForFontUpdates(dismissBtn); final ConfirmationAction dismissRef = dismissAction; diff --git a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/confirmation/ConfirmationService.java b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/confirmation/ConfirmationService.java index 247ad1c3..744dbc4a 100644 --- a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/confirmation/ConfirmationService.java +++ b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/confirmation/ConfirmationService.java @@ -8,7 +8,6 @@ import org.eclipse.jface.preference.IPreferenceStore; -import com.microsoft.copilot.eclipse.core.Constants; import com.microsoft.copilot.eclipse.core.chat.ConfirmationAction; import com.microsoft.copilot.eclipse.core.chat.ConfirmationActionScope; import com.microsoft.copilot.eclipse.core.chat.ConfirmationResult; @@ -82,8 +81,7 @@ public ConfirmationResult evaluate( InvokeClientToolConfirmationParams params, String sessionConversationId) { ToolCategory category = classify(params); - if (category == ToolCategory.SAFE_TOOL - || category == ToolCategory.UNKNOWN) { + if (category == ToolCategory.SAFE_TOOL) { return ConfirmationResult.AUTO_APPROVED; } @@ -91,7 +89,9 @@ public ConfirmationResult evaluate( if (handler != null) { return handler.evaluate(params, sessionConversationId); } - return ConfirmationResult.AUTO_APPROVED; + // No handler registered for this category — fall through to + // the default confirmation dialog rather than silently approving. + return ConfirmationResult.needsConfirmation(null); } /** diff --git a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/confirmation/TerminalConfirmationHandler.java b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/confirmation/TerminalConfirmationHandler.java index 6f18e034..16dd63a3 100644 --- a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/confirmation/TerminalConfirmationHandler.java +++ b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/confirmation/TerminalConfirmationHandler.java @@ -6,6 +6,7 @@ import java.lang.reflect.Type; import java.util.ArrayList; import java.util.Collections; +import java.util.LinkedHashMap; import java.util.LinkedHashSet; import java.util.List; import java.util.Map; @@ -13,6 +14,7 @@ import java.util.concurrent.ConcurrentHashMap; import java.util.regex.Pattern; import java.util.regex.PatternSyntaxException; +import java.util.stream.Collectors; import com.google.gson.Gson; import com.google.gson.reflect.TypeToken; @@ -81,13 +83,21 @@ private static final class RuleResult { private static final Type RULES_TYPE = new TypeToken>() { }.getType(); + /** + * Maximum number of conversations whose session-scoped approvals are kept + * in memory. When exceeded, the oldest conversation's data is evicted. + */ + static final int MAX_SESSION_CONVERSATIONS = 50; + private final IPreferenceStore preferenceStore; - // Session-scoped in-memory storage keyed by conversationId - private final ConcurrentHashMap> allowedCommandNames = - new ConcurrentHashMap<>(); - private final ConcurrentHashMap> allowedExactCommands = - new ConcurrentHashMap<>(); + // Session-scoped in-memory storage keyed by conversationId. + // Uses insertion-ordered maps so we can evict the oldest entry when the + // map grows beyond MAX_SESSION_CONVERSATIONS. + private final Map> allowedCommandNames = + Collections.synchronizedMap(new LinkedHashMap<>()); + private final Map> allowedExactCommands = + Collections.synchronizedMap(new LinkedHashMap<>()); private final Set allowAllConversations = ConcurrentHashMap.newKeySet(); /** @@ -427,12 +437,22 @@ public void persistDecision(ConfirmationAction confirmAction, } String convId = sessionConversationId; - String[] cmdNames = getCommandNames(params); - String commandLine = extractCommandLine(params); + + // Prefer command data from action metadata (set by buildContent) + // so we persist exactly what the user chose, not the full params. + Map meta = confirmAction.getMetadata(); + String metaNames = meta.get(META_COMMAND_NAMES); + String metaLine = meta.get(META_COMMAND_LINE); + + String[] cmdNames = metaNames != null && !metaNames.isBlank() + ? metaNames.split(",") : getCommandNames(params); + String commandLine = metaLine != null && !metaLine.isBlank() + ? metaLine : extractCommandLine(params); switch (type) { case ACCEPT_NAMES_SESSION: if (cmdNames != null) { + evictOldestIfNeeded(allowedCommandNames); Set nameSet = allowedCommandNames.computeIfAbsent( convId, k -> ConcurrentHashMap.newKeySet()); Collections.addAll(nameSet, cmdNames); @@ -440,6 +460,7 @@ public void persistDecision(ConfirmationAction confirmAction, break; case ACCEPT_EXACT_SESSION: if (commandLine != null && !commandLine.isBlank()) { + evictOldestIfNeeded(allowedExactCommands); allowedExactCommands.computeIfAbsent( convId, k -> ConcurrentHashMap.newKeySet()) .add(commandLine.trim()); @@ -447,6 +468,10 @@ public void persistDecision(ConfirmationAction confirmAction, break; case ACCEPT_ALL_SESSION: allowAllConversations.add(convId); + // Cap allowAllConversations the same way + while (allowAllConversations.size() > MAX_SESSION_CONVERSATIONS) { + allowAllConversations.iterator().remove(); + } break; case ACCEPT_NAMES_GLOBAL: if (cmdNames != null) { @@ -463,6 +488,22 @@ public void persistDecision(ConfirmationAction confirmAction, } } + /** + * Evicts the oldest entry from a LinkedHashMap when it reaches the + * maximum number of tracked conversations. + */ + private static void evictOldestIfNeeded(Map map) { + synchronized (map) { + while (map.size() >= MAX_SESSION_CONVERSATIONS) { + var it = map.entrySet().iterator(); + if (it.hasNext()) { + it.next(); + it.remove(); + } + } + } + } + @Override public void clearSession(String conversationId) { allowedCommandNames.remove(conversationId); @@ -471,17 +512,26 @@ public void clearSession(String conversationId) { } private void addGlobalRules(List commands) { - List rules = new ArrayList<>(loadRules()); - boolean changed = false; - for (String cmd : commands) { - if (rules.stream().noneMatch(r -> r.getCommand().equals(cmd))) { - rules.add(new TerminalAutoApproveRule(cmd, true)); - changed = true; - } - } - if (changed) { + List original = loadRules(); + Set existing = original.stream() + .map(TerminalAutoApproveRule::getCommand) + .collect(Collectors.toSet()); + + // Override existing deny rules → allow + List updated = original.stream() + .map(r -> commands.contains(r.getCommand()) && !r.isAutoApprove() + ? new TerminalAutoApproveRule(r.getCommand(), true) : r) + .collect(Collectors.toCollection(ArrayList::new)); + + // Append new rules for commands not yet present + commands.stream() + .filter(cmd -> !existing.contains(cmd)) + .map(cmd -> new TerminalAutoApproveRule(cmd, true)) + .forEach(updated::add); + + if (!updated.equals(original)) { preferenceStore.setValue(Constants.AUTO_APPROVE_TERMINAL_RULES, - new Gson().toJson(rules)); + new Gson().toJson(updated)); } } } diff --git a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/preferences/AddTerminalRuleDialog.java b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/preferences/AddTerminalRuleDialog.java index 1f64fbc5..b686c914 100644 --- a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/preferences/AddTerminalRuleDialog.java +++ b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/preferences/AddTerminalRuleDialog.java @@ -55,7 +55,7 @@ protected Control createDialogArea(Composite parent) { GridData commandData = new GridData(SWT.FILL, SWT.CENTER, true, false); commandData.widthHint = 300; commandText.setLayoutData(commandData); - commandText.setMessage("e.g., npm, git, /^apt-get\\b/"); + commandText.setMessage(Messages.preferences_page_terminal_auto_approve_add_dialog_placeholder); commandText.addModifyListener(e -> updateOkButton()); // Allow / Deny diff --git a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/preferences/Messages.java b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/preferences/Messages.java index acea2e53..ce2fa550 100644 --- a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/preferences/Messages.java +++ b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/preferences/Messages.java @@ -172,6 +172,7 @@ public class Messages extends NLS { public static String preferences_page_terminal_auto_approve_add_dialog_message; public static String preferences_page_terminal_auto_approve_add_dialog_command; public static String preferences_page_terminal_auto_approve_add_dialog_approve; + public static String preferences_page_terminal_auto_approve_add_dialog_placeholder; static { // initialize resource bundle diff --git a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/preferences/TerminalAutoApproveSection.java b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/preferences/TerminalAutoApproveSection.java index 50265936..45fdc75d 100644 --- a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/preferences/TerminalAutoApproveSection.java +++ b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/preferences/TerminalAutoApproveSection.java @@ -27,6 +27,7 @@ import org.eclipse.swt.widgets.Table; import com.microsoft.copilot.eclipse.core.Constants; +import com.microsoft.copilot.eclipse.core.CopilotCore; import com.microsoft.copilot.eclipse.core.chat.TerminalAutoApproveRule; import com.microsoft.copilot.eclipse.ui.chat.confirmation.TerminalConfirmationHandler; @@ -253,7 +254,8 @@ public void loadFromPreferences(IPreferenceStore store) { rules.addAll(loaded); } } catch (Exception e) { - // Invalid JSON, start with empty list + CopilotCore.LOGGER.error( + "Failed to parse terminal auto-approve rules from preferences", e); } } tableViewer.setInput(rules); diff --git a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/preferences/messages.properties b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/preferences/messages.properties index fb67a60c..b6c8f4ed 100644 --- a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/preferences/messages.properties +++ b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/preferences/messages.properties @@ -164,3 +164,4 @@ preferences_page_terminal_auto_approve_add_dialog_title=Add Terminal Command Rul preferences_page_terminal_auto_approve_add_dialog_message=Enter the command name or regex pattern (e.g., npm, git, /^apt-get\\b/) preferences_page_terminal_auto_approve_add_dialog_command=Command or Regex: preferences_page_terminal_auto_approve_add_dialog_approve=Auto Approve: +preferences_page_terminal_auto_approve_add_dialog_placeholder=e.g., npm, git, /^apt-get\\b/ From 9dd1ef25cdb00173b5a58e4b2ddc846f0895171c Mon Sep 17 00:00:00 2001 From: xinyi-gong Date: Mon, 11 May 2026 18:11:06 +0800 Subject: [PATCH 5/8] fix tests --- .../TerminalConfirmationHandlerTests.java | 78 ++++++++++++------- 1 file changed, 48 insertions(+), 30 deletions(-) diff --git a/com.microsoft.copilot.eclipse.ui.test/src/com/microsoft/copilot/eclipse/ui/chat/confirmation/TerminalConfirmationHandlerTests.java b/com.microsoft.copilot.eclipse.ui.test/src/com/microsoft/copilot/eclipse/ui/chat/confirmation/TerminalConfirmationHandlerTests.java index ffc8892b..133aabf2 100644 --- a/com.microsoft.copilot.eclipse.ui.test/src/com/microsoft/copilot/eclipse/ui/chat/confirmation/TerminalConfirmationHandlerTests.java +++ b/com.microsoft.copilot.eclipse.ui.test/src/com/microsoft/copilot/eclipse/ui/chat/confirmation/TerminalConfirmationHandlerTests.java @@ -46,56 +46,74 @@ void setUp() { handler = new TerminalConfirmationHandler(preferenceStore); } - // --- matchesRule --- + // --- rule matching (tested through evaluate) --- @Test - void matchesRule_simpleRuleMatchesCommandAtStart() { - assertTrue(TerminalConfirmationHandler.matchesRule( - "rm -rf /tmp", "rm")); - } - - @Test - void matchesRule_simpleRuleDoesNotMatchMiddleOfWord() { - assertFalse(TerminalConfirmationHandler.matchesRule( - "remove something", "rm")); - } - - @Test - void matchesRule_regexCaseInsensitive() { - assertTrue(TerminalConfirmationHandler.matchesRule( - "Git status", "/^git\\b/i")); + void ruleMatching_simpleRuleMatchesCommandAtStart() { + stubRules(List.of(new TerminalAutoApproveRule("rm", true))); + stubUnmatched(false); + InvokeClientToolConfirmationParams params = + buildParams(new String[]{"rm -rf /tmp"}, new String[]{"rm"}, + "rm -rf /tmp"); + assertTrue(handler.evaluate(params, CONV_ID).isAutoApproved()); } @Test - void matchesRule_regexDotallMatchesSubshell() { - assertTrue(TerminalConfirmationHandler.matchesRule( - "(echo hello)", "/(\\(.+\\))/s")); + void ruleMatching_simpleRuleDoesNotMatchMiddleOfWord() { + stubRules(List.of(new TerminalAutoApproveRule("rm", true))); + stubUnmatched(false); + InvokeClientToolConfirmationParams params = + buildParams(new String[]{"remove something"}, + new String[]{"remove"}, "remove something"); + assertFalse(handler.evaluate(params, CONV_ID).isAutoApproved()); } @Test - void matchesRule_nullSubCommandReturnsFalse() { - assertFalse(TerminalConfirmationHandler.matchesRule(null, "rm")); + void ruleMatching_regexCaseInsensitive() { + stubRules(List.of(new TerminalAutoApproveRule("/^git\\b/i", true))); + stubUnmatched(false); + InvokeClientToolConfirmationParams params = + buildParams(new String[]{"Git status"}, new String[]{"Git"}, + "Git status"); + assertTrue(handler.evaluate(params, CONV_ID).isAutoApproved()); } @Test - void matchesRule_emptySubCommandReturnsFalse() { - assertFalse(TerminalConfirmationHandler.matchesRule("", "rm")); + void ruleMatching_regexDotallMatchesSubshell() { + stubRules(List.of( + new TerminalAutoApproveRule("/(\\(.+\\))/s", true))); + stubUnmatched(false); + InvokeClientToolConfirmationParams params = + buildParams(new String[]{"(echo hello)"}, + new String[]{"(echo"}, "(echo hello)"); + assertTrue(handler.evaluate(params, CONV_ID).isAutoApproved()); } @Test - void matchesRule_nullRuleReturnsFalse() { - assertFalse(TerminalConfirmationHandler.matchesRule("rm -rf", null)); + void ruleMatching_noMatchWhenSubCommandsNull() { + stubRules(List.of(new TerminalAutoApproveRule("rm", true))); + stubUnmatched(false); + InvokeClientToolConfirmationParams params = + buildParams(null, null, "rm -rf"); + assertFalse(handler.evaluate(params, CONV_ID).isAutoApproved()); } @Test - void matchesRule_emptyRuleReturnsFalse() { - assertFalse(TerminalConfirmationHandler.matchesRule("rm -rf", "")); + void ruleMatching_noMatchWhenSubCommandsEmpty() { + stubRules(List.of(new TerminalAutoApproveRule("rm", true))); + stubUnmatched(false); + InvokeClientToolConfirmationParams params = + buildParams(new String[]{}, new String[]{}, "rm -rf"); + assertFalse(handler.evaluate(params, CONV_ID).isAutoApproved()); } @Test - void matchesRule_blankInputsReturnFalse() { - assertFalse(TerminalConfirmationHandler.matchesRule(" ", "rm")); - assertFalse(TerminalConfirmationHandler.matchesRule("rm", " ")); + void ruleMatching_noMatchWhenSubCommandBlank() { + stubRules(List.of(new TerminalAutoApproveRule("rm", true))); + stubUnmatched(false); + InvokeClientToolConfirmationParams params = + buildParams(new String[]{" "}, new String[]{" "}, " "); + assertFalse(handler.evaluate(params, CONV_ID).isAutoApproved()); } // --- evaluate --- From 67a1cf75877e7f19d723c8a2249002472a82eaf0 Mon Sep 17 00:00:00 2001 From: xinyi-gong Date: Tue, 12 May 2026 11:27:53 +0800 Subject: [PATCH 6/8] extract shared messages --- .../ui/preferences/AddTerminalRuleDialog.java | 6 ++--- .../eclipse/ui/preferences/Messages.java | 18 ++++++++------- .../TerminalAutoApproveSection.java | 22 +++++++++---------- .../ui/preferences/messages.properties | 18 ++++++++------- 4 files changed, 34 insertions(+), 30 deletions(-) diff --git a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/preferences/AddTerminalRuleDialog.java b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/preferences/AddTerminalRuleDialog.java index b686c914..aa4af5de 100644 --- a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/preferences/AddTerminalRuleDialog.java +++ b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/preferences/AddTerminalRuleDialog.java @@ -61,14 +61,14 @@ protected Control createDialogArea(Composite parent) { // Allow / Deny Label approveLabel = new Label(container, SWT.NONE); approveLabel.setText( - Messages.preferences_page_terminal_auto_approve_add_dialog_approve); + Messages.preferences_page_auto_approve_add_dialog_approve); Composite radioGroup = new Composite(container, SWT.NONE); radioGroup.setLayout(new GridLayout(2, false)); allowRadio = new Button(radioGroup, SWT.RADIO); - allowRadio.setText(Messages.preferences_page_terminal_auto_approve_allow); + allowRadio.setText(Messages.preferences_page_auto_approve_allow); allowRadio.setSelection(true); Button denyRadio = new Button(radioGroup, SWT.RADIO); - denyRadio.setText(Messages.preferences_page_terminal_auto_approve_deny); + denyRadio.setText(Messages.preferences_page_auto_approve_deny); return area; } diff --git a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/preferences/Messages.java b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/preferences/Messages.java index ce2fa550..cfb55ccb 100644 --- a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/preferences/Messages.java +++ b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/preferences/Messages.java @@ -154,24 +154,26 @@ public class Messages extends NLS { public static String setting_managed_by_organization; public static String setting_disabled_by_organization; + // Shared Auto-Approve strings + public static String preferences_page_auto_approve_column_status; + public static String preferences_page_auto_approve_add; + public static String preferences_page_auto_approve_remove; + public static String preferences_page_auto_approve_reset; + public static String preferences_page_auto_approve_reset_message; + public static String preferences_page_auto_approve_allow; + public static String preferences_page_auto_approve_deny; + public static String preferences_page_auto_approve_add_dialog_approve; + // Terminal Auto-Approve public static String preferences_page_terminal_auto_approve_title; public static String preferences_page_terminal_auto_approve_description; public static String preferences_page_terminal_auto_approve_column_command; - public static String preferences_page_terminal_auto_approve_column_status; - public static String preferences_page_terminal_auto_approve_add; - public static String preferences_page_terminal_auto_approve_remove; - public static String preferences_page_terminal_auto_approve_reset; public static String preferences_page_terminal_auto_approve_reset_title; - public static String preferences_page_terminal_auto_approve_reset_message; - public static String preferences_page_terminal_auto_approve_allow; - public static String preferences_page_terminal_auto_approve_deny; public static String preferences_page_terminal_auto_approve_unmatched; public static String preferences_page_terminal_auto_approve_unmatched_note; public static String preferences_page_terminal_auto_approve_add_dialog_title; public static String preferences_page_terminal_auto_approve_add_dialog_message; public static String preferences_page_terminal_auto_approve_add_dialog_command; - public static String preferences_page_terminal_auto_approve_add_dialog_approve; public static String preferences_page_terminal_auto_approve_add_dialog_placeholder; static { diff --git a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/preferences/TerminalAutoApproveSection.java b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/preferences/TerminalAutoApproveSection.java index 45fdc75d..db41cb10 100644 --- a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/preferences/TerminalAutoApproveSection.java +++ b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/preferences/TerminalAutoApproveSection.java @@ -113,14 +113,14 @@ public String getText(Object element) { TableViewerColumn statusCol = new TableViewerColumn(tableViewer, SWT.NONE); statusCol.getColumn().setText( - Messages.preferences_page_terminal_auto_approve_column_status); + Messages.preferences_page_auto_approve_column_status); statusCol.getColumn().setWidth(100); statusCol.setLabelProvider(new ColumnLabelProvider() { @Override public String getText(Object element) { return ((TerminalAutoApproveRule) element).isAutoApprove() - ? Messages.preferences_page_terminal_auto_approve_allow - : Messages.preferences_page_terminal_auto_approve_deny; + ? Messages.preferences_page_auto_approve_allow + : Messages.preferences_page_auto_approve_deny; } }); @@ -135,13 +135,13 @@ private void createButtons(Composite parent) { new GridData(SWT.BEGINNING, SWT.BEGINNING, false, false)); Button addButton = new Button(btnGroup, SWT.PUSH); - addButton.setText(Messages.preferences_page_terminal_auto_approve_add); + addButton.setText(Messages.preferences_page_auto_approve_add); addButton.setLayoutData(new GridData(SWT.FILL, SWT.TOP, true, false)); addButton.addListener(SWT.Selection, e -> onAdd()); removeButton = new Button(btnGroup, SWT.PUSH); removeButton.setText( - Messages.preferences_page_terminal_auto_approve_remove); + Messages.preferences_page_auto_approve_remove); removeButton.setLayoutData( new GridData(SWT.FILL, SWT.TOP, true, false)); removeButton.setEnabled(false); @@ -149,7 +149,7 @@ private void createButtons(Composite parent) { toggleButton = new Button(btnGroup, SWT.PUSH); toggleButton.setText( - Messages.preferences_page_terminal_auto_approve_allow); + Messages.preferences_page_auto_approve_allow); toggleButton.setLayoutData( new GridData(SWT.FILL, SWT.TOP, true, false)); toggleButton.setEnabled(false); @@ -157,7 +157,7 @@ private void createButtons(Composite parent) { resetButton = new Button(btnGroup, SWT.PUSH); resetButton.setText( - Messages.preferences_page_terminal_auto_approve_reset); + Messages.preferences_page_auto_approve_reset); resetButton.setLayoutData( new GridData(SWT.FILL, SWT.TOP, true, false)); resetButton.addListener(SWT.Selection, e -> onResetToDefaults()); @@ -198,7 +198,7 @@ private void onToggle() { private void onResetToDefaults() { boolean confirmed = MessageDialog.openQuestion(getShell(), Messages.preferences_page_terminal_auto_approve_reset_title, - Messages.preferences_page_terminal_auto_approve_reset_message); + Messages.preferences_page_auto_approve_reset_message); if (confirmed) { rules.clear(); rules.addAll(TerminalConfirmationHandler.DEFAULT_RULES.stream() @@ -219,11 +219,11 @@ private void updateButtonState() { TerminalAutoApproveRule rule = (TerminalAutoApproveRule) tableViewer.getStructuredSelection().getFirstElement(); toggleButton.setText(rule.isAutoApprove() - ? Messages.preferences_page_terminal_auto_approve_deny - : Messages.preferences_page_terminal_auto_approve_allow); + ? Messages.preferences_page_auto_approve_deny + : Messages.preferences_page_auto_approve_allow); } else { toggleButton.setText( - Messages.preferences_page_terminal_auto_approve_allow); + Messages.preferences_page_auto_approve_allow); } resetButton.setEnabled(!isMatchingDefaults()); } diff --git a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/preferences/messages.properties b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/preferences/messages.properties index b6c8f4ed..b9534b13 100644 --- a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/preferences/messages.properties +++ b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/preferences/messages.properties @@ -146,22 +146,24 @@ setting_disabled_by_organization=This setting is disabled by your organization. setting_managed_by_organization=This setting is managed by your organization. +# Shared Auto Approve strings (reusable by all auto-approve sections) +preferences_page_auto_approve_column_status=Auto Approve +preferences_page_auto_approve_add=Add... +preferences_page_auto_approve_remove=Remove +preferences_page_auto_approve_reset=Reset to Defaults +preferences_page_auto_approve_reset_message=This will replace all current rules with the defaults. Continue? +preferences_page_auto_approve_allow=Allow +preferences_page_auto_approve_deny=Deny +preferences_page_auto_approve_add_dialog_approve=Auto Approve: + # Terminal Auto Approve preferences_page_terminal_auto_approve_title=Terminal Auto Approve preferences_page_terminal_auto_approve_description=Controls whether chat-initiated terminal commands are automatically approved. Set to Allow to auto approve matching commands; set to Deny to always require explicit approval. preferences_page_terminal_auto_approve_column_command=Command -preferences_page_terminal_auto_approve_column_status=Auto Approve -preferences_page_terminal_auto_approve_add=Add... -preferences_page_terminal_auto_approve_remove=Remove -preferences_page_terminal_auto_approve_reset=Reset to Defaults preferences_page_terminal_auto_approve_reset_title=Reset Terminal Auto Approve Rules -preferences_page_terminal_auto_approve_reset_message=This will replace all current rules with the defaults. Continue? -preferences_page_terminal_auto_approve_allow=Allow -preferences_page_terminal_auto_approve_deny=Deny preferences_page_terminal_auto_approve_unmatched=Auto approve commands not covered by rules preferences_page_terminal_auto_approve_unmatched_note=When enabled, terminal commands not covered by the rules above are automatically approved. Use this setting at your own discretion. preferences_page_terminal_auto_approve_add_dialog_title=Add Terminal Command Rule preferences_page_terminal_auto_approve_add_dialog_message=Enter the command name or regex pattern (e.g., npm, git, /^apt-get\\b/) preferences_page_terminal_auto_approve_add_dialog_command=Command or Regex: -preferences_page_terminal_auto_approve_add_dialog_approve=Auto Approve: preferences_page_terminal_auto_approve_add_dialog_placeholder=e.g., npm, git, /^apt-get\\b/ From 28cb436485735512126898810e09e20e00d50de3 Mon Sep 17 00:00:00 2001 From: xinyi-gong Date: Tue, 12 May 2026 17:19:17 +0800 Subject: [PATCH 7/8] resolve comments --- .../TerminalConfirmationHandlerTests.java | 28 +++++++++---------- .../ui/chat/InvokeToolConfirmationDialog.java | 2 +- .../confirmation/ConfirmationHandler.java | 4 +-- .../confirmation/ConfirmationService.java | 6 ++-- .../TerminalConfirmationHandler.java | 20 +++++++++---- .../ui/chat/services/AgentToolService.java | 2 +- 6 files changed, 35 insertions(+), 27 deletions(-) diff --git a/com.microsoft.copilot.eclipse.ui.test/src/com/microsoft/copilot/eclipse/ui/chat/confirmation/TerminalConfirmationHandlerTests.java b/com.microsoft.copilot.eclipse.ui.test/src/com/microsoft/copilot/eclipse/ui/chat/confirmation/TerminalConfirmationHandlerTests.java index 133aabf2..65c9e551 100644 --- a/com.microsoft.copilot.eclipse.ui.test/src/com/microsoft/copilot/eclipse/ui/chat/confirmation/TerminalConfirmationHandlerTests.java +++ b/com.microsoft.copilot.eclipse.ui.test/src/com/microsoft/copilot/eclipse/ui/chat/confirmation/TerminalConfirmationHandlerTests.java @@ -205,10 +205,10 @@ void evaluate_emptyRulesUsesUnmatchedSetting() { assertTrue(result.isAutoApproved()); } - // --- Session memory via persistDecision --- + // --- Session memory via cacheDecision --- @Test - void persistDecision_acceptAllSession_autoApprovesSubsequent() { + void cacheDecision_acceptAllSession_autoApprovesSubsequent() { stubRules(List.of()); stubUnmatched(false); @@ -218,14 +218,14 @@ void persistDecision_acceptAllSession_autoApprovesSubsequent() { ConfirmationAction allSession = buildSessionAction( TerminalConfirmationHandler.Action.ACCEPT_ALL_SESSION); - handler.persistDecision(allSession, params, CONV_ID); + handler.cacheDecision(allSession, params, CONV_ID); ConfirmationResult result = handler.evaluate(params, CONV_ID); assertTrue(result.isAutoApproved()); } @Test - void persistDecision_acceptNamesSession_autoApprovesMatchingNames() { + void cacheDecision_acceptNamesSession_autoApprovesMatchingNames() { stubRules(List.of()); stubUnmatched(false); @@ -235,14 +235,14 @@ void persistDecision_acceptNamesSession_autoApprovesMatchingNames() { ConfirmationAction namesSession = buildSessionAction( TerminalConfirmationHandler.Action.ACCEPT_NAMES_SESSION); - handler.persistDecision(namesSession, params, CONV_ID); + handler.cacheDecision(namesSession, params, CONV_ID); ConfirmationResult result = handler.evaluate(params, CONV_ID); assertTrue(result.isAutoApproved()); } @Test - void persistDecision_acceptExactSession_autoApprovesMatchingCommand() { + void cacheDecision_acceptExactSession_autoApprovesMatchingCommand() { stubRules(List.of()); stubUnmatched(false); @@ -252,7 +252,7 @@ void persistDecision_acceptExactSession_autoApprovesMatchingCommand() { ConfirmationAction exactSession = buildSessionAction( TerminalConfirmationHandler.Action.ACCEPT_EXACT_SESSION); - handler.persistDecision(exactSession, params, CONV_ID); + handler.cacheDecision(exactSession, params, CONV_ID); ConfirmationResult result = handler.evaluate(params, CONV_ID); assertTrue(result.isAutoApproved()); @@ -269,7 +269,7 @@ void clearSession_removesApprovalsForConversation() { ConfirmationAction allSession = buildSessionAction( TerminalConfirmationHandler.Action.ACCEPT_ALL_SESSION); - handler.persistDecision(allSession, params, CONV_ID); + handler.cacheDecision(allSession, params, CONV_ID); handler.clearSession(CONV_ID); @@ -288,7 +288,7 @@ void clearSession_doesNotAffectOtherConversation() { ConfirmationAction allSession = buildSessionAction( TerminalConfirmationHandler.Action.ACCEPT_ALL_SESSION); - handler.persistDecision(allSession, params, CONV_ID); + handler.cacheDecision(allSession, params, CONV_ID); handler.clearSession("other-conv"); @@ -505,7 +505,7 @@ void buildContent_filtersSessionApprovedNamesFromActions() { InvokeClientToolConfirmationParams approveParams = buildParams(new String[]{"echo hello"}, new String[]{"echo"}, "echo hello"); - handler.persistDecision( + handler.cacheDecision( buildSessionAction( TerminalConfirmationHandler.Action.ACCEPT_NAMES_SESSION), approveParams, CONV_ID); @@ -567,7 +567,7 @@ void buildContent_allNamesApproved_autoApproves() { InvokeClientToolConfirmationParams approveParams = buildParams(new String[]{"curl x"}, new String[]{"curl"}, "curl x"); - handler.persistDecision( + handler.cacheDecision( buildSessionAction( TerminalConfirmationHandler.Action.ACCEPT_NAMES_SESSION), approveParams, CONV_ID); @@ -590,7 +590,7 @@ void sessionRules_surviveConversationSwitch() { InvokeClientToolConfirmationParams approveParams = buildParams(new String[]{"echo hello"}, new String[]{"echo"}, "echo hello"); - handler.persistDecision( + handler.cacheDecision( buildSessionAction( TerminalConfirmationHandler.Action.ACCEPT_NAMES_SESSION), approveParams, "conv-A"); @@ -610,7 +610,7 @@ void sessionRules_evictOldestWhenCapExceeded() { i++) { InvokeClientToolConfirmationParams p = buildParams(new String[]{"echo"}, new String[]{"echo"}, "echo"); - handler.persistDecision( + handler.cacheDecision( buildSessionAction( TerminalConfirmationHandler.Action.ACCEPT_NAMES_SESSION), p, "conv-" + i); @@ -619,7 +619,7 @@ void sessionRules_evictOldestWhenCapExceeded() { // Add one more — should evict the oldest (conv-0) InvokeClientToolConfirmationParams p = buildParams(new String[]{"echo"}, new String[]{"echo"}, "echo"); - handler.persistDecision( + handler.cacheDecision( buildSessionAction( TerminalConfirmationHandler.Action.ACCEPT_NAMES_SESSION), p, "conv-new"); diff --git a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/InvokeToolConfirmationDialog.java b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/InvokeToolConfirmationDialog.java index ff797e29..48540e14 100644 --- a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/InvokeToolConfirmationDialog.java +++ b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/InvokeToolConfirmationDialog.java @@ -114,7 +114,7 @@ public void cancelConfirmation() { new LanguageModelToolConfirmationResult(ToolConfirmationResult.DISMISS)); Composite parent = this.getParent(); - SwtUtils.invokeOnDisplayThread(() -> { + SwtUtils.invokeOnDisplayThreadAsync(() -> { if (StringUtils.isNotEmpty(this.cancelMessage)) { new AgentToolCancelLabel(this.getParent(), SWT.NONE, this.cancelMessage); } diff --git a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/confirmation/ConfirmationHandler.java b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/confirmation/ConfirmationHandler.java index 2bd4d25a..83c73f78 100644 --- a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/confirmation/ConfirmationHandler.java +++ b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/confirmation/ConfirmationHandler.java @@ -27,7 +27,7 @@ ConfirmationResult evaluate(InvokeClientToolConfirmationParams params, String sessionConversationId); /** - * Persists a user's decision based on the action scope. + * Caches a user's decision based on the action scope. * SESSION actions are stored in-memory per conversation; * GLOBAL actions are written to preferences. * @@ -35,7 +35,7 @@ ConfirmationResult evaluate(InvokeClientToolConfirmationParams params, * @param params the original confirmation params (for command data etc.) * @param sessionConversationId the conversation ID to use for session storage */ - default void persistDecision(ConfirmationAction action, + default void cacheDecision(ConfirmationAction action, InvokeClientToolConfirmationParams params, String sessionConversationId) { // no-op by default diff --git a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/confirmation/ConfirmationService.java b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/confirmation/ConfirmationService.java index 744dbc4a..941e95c6 100644 --- a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/confirmation/ConfirmationService.java +++ b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/confirmation/ConfirmationService.java @@ -95,13 +95,13 @@ public ConfirmationResult evaluate( } /** - * Dispatches a persist call to the handler that owns this tool category. + * Caches the user's decision for future auto-approve lookups. * * @param action the user's selected action * @param params the original confirmation params * @param sessionConversationId the conversation ID for session storage */ - public void persistDecision(ConfirmationAction action, + public void cacheDecision(ConfirmationAction action, InvokeClientToolConfirmationParams params, String sessionConversationId) { if (action == null || action.getScope() == null @@ -111,7 +111,7 @@ public void persistDecision(ConfirmationAction action, ToolCategory category = classify(params); ConfirmationHandler handler = handlers.get(category); if (handler != null) { - handler.persistDecision(action, params, sessionConversationId); + handler.cacheDecision(action, params, sessionConversationId); } } diff --git a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/confirmation/TerminalConfirmationHandler.java b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/confirmation/TerminalConfirmationHandler.java index 16dd63a3..9a137d1e 100644 --- a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/confirmation/TerminalConfirmationHandler.java +++ b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/confirmation/TerminalConfirmationHandler.java @@ -6,6 +6,7 @@ import java.lang.reflect.Type; import java.util.ArrayList; import java.util.Collections; +import java.util.HashMap; import java.util.LinkedHashMap; import java.util.LinkedHashSet; import java.util.List; @@ -41,10 +42,15 @@ public class TerminalConfirmationHandler implements ConfirmationHandler { /** Action types matching IntelliJ's TerminalAutoApproveAction enum. */ public enum Action { + /** Allow specific command names for the current session/conversation. */ ACCEPT_NAMES_SESSION, + /** Always allow specific command names (persisted as a global rule). */ ACCEPT_NAMES_GLOBAL, + /** Allow this exact command line for the current session/conversation. */ ACCEPT_EXACT_SESSION, + /** Always allow this exact command line (persisted as a global rule). */ ACCEPT_EXACT_GLOBAL, + /** Allow all terminal commands for the current session/conversation. */ ACCEPT_ALL_SESSION } @@ -98,7 +104,9 @@ private static final class RuleResult { Collections.synchronizedMap(new LinkedHashMap<>()); private final Map> allowedExactCommands = Collections.synchronizedMap(new LinkedHashMap<>()); - private final Set allowAllConversations = ConcurrentHashMap.newKeySet(); + private final Set allowAllConversations = + Collections.newSetFromMap( + Collections.synchronizedMap(new LinkedHashMap<>())); /** * Creates a new TerminalConfirmationHandler. @@ -202,7 +210,7 @@ public ConfirmationResult evaluate( */ private RuleResult evaluateSubCommands(String[] subCommands, String[] cmdNames, List rules, - Set sessionNames) { + Set sessionApprovedNames) { boolean allApproved = true; boolean hasDeny = false; List unapproved = new ArrayList<>(); @@ -215,8 +223,8 @@ private RuleResult evaluateSubCommands(String[] subCommands, boolean denied = false; // Session command-name approval - if (cmdName != null && sessionNames != null - && sessionNames.contains(cmdName)) { + if (cmdName != null && sessionApprovedNames != null + && sessionApprovedNames.contains(cmdName)) { hasAllow = true; } @@ -314,7 +322,7 @@ private ConfirmationContent buildContent( private static ConfirmationAction action(Action type, String label, ConfirmationActionScope scope, Map extra) { - Map meta = new java.util.HashMap<>(extra); + Map meta = new HashMap<>(extra); meta.put(ConfirmationAction.META_ACTION, type.name()); return new ConfirmationAction(label, true, scope, meta, false); } @@ -421,7 +429,7 @@ List loadRules() { } @Override - public void persistDecision(ConfirmationAction confirmAction, + public void cacheDecision(ConfirmationAction confirmAction, InvokeClientToolConfirmationParams params, String sessionConversationId) { String actionName = confirmAction.getMetadata() 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 a770e70d..3eee71f0 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 @@ -299,7 +299,7 @@ public CompletableFuture onToolConfirmation ConfirmationAction selected = dialog != null ? dialog.getSelectedAction() : null; if (selected != null && selected.isAccept()) { - confirmationService.persistDecision(selected, params, + confirmationService.cacheDecision(selected, params, sessConvId); } return result; From 4c8dbbe08795c2efbc7b60d5ae87df8cdba0c58e Mon Sep 17 00:00:00 2001 From: xinyi-gong Date: Tue, 12 May 2026 17:38:47 +0800 Subject: [PATCH 8/8] add fallbackConfirmationHandler --- .../copilot/eclipse/ui/chat/Messages.java | 1 + .../confirmation/ConfirmationService.java | 10 ++--- .../FallbackConfirmationHandler.java | 37 +++++++++++++++++++ .../eclipse/ui/chat/messages.properties | 1 + 4 files changed, 44 insertions(+), 5 deletions(-) create mode 100644 com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/confirmation/FallbackConfirmationHandler.java diff --git a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/Messages.java b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/Messages.java index 97ee8c5e..2a35a47b 100644 --- a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/Messages.java +++ b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/Messages.java @@ -50,6 +50,7 @@ public final class Messages extends NLS { // Confirmation dialog titles public static String confirmation_title_terminal; + public static String confirmation_title_fallback; // Misc public static String confirmation_autoApprovedDescription; diff --git a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/confirmation/ConfirmationService.java b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/confirmation/ConfirmationService.java index 941e95c6..3ddf6a93 100644 --- a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/confirmation/ConfirmationService.java +++ b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/confirmation/ConfirmationService.java @@ -58,6 +58,8 @@ public static ToolCategory fromValue(String value) { private final Map handlers = new EnumMap<>(ToolCategory.class); + private final ConfirmationHandler fallbackHandler = + new FallbackConfirmationHandler(); private final IPreferenceStore preferenceStore; /** @@ -86,12 +88,10 @@ public ConfirmationResult evaluate( } ConfirmationHandler handler = handlers.get(category); - if (handler != null) { - return handler.evaluate(params, sessionConversationId); + if (handler == null) { + handler = fallbackHandler; } - // No handler registered for this category — fall through to - // the default confirmation dialog rather than silently approving. - return ConfirmationResult.needsConfirmation(null); + return handler.evaluate(params, sessionConversationId); } /** diff --git a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/confirmation/FallbackConfirmationHandler.java b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/confirmation/FallbackConfirmationHandler.java new file mode 100644 index 00000000..9fd99b25 --- /dev/null +++ b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/confirmation/FallbackConfirmationHandler.java @@ -0,0 +1,37 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + +package com.microsoft.copilot.eclipse.ui.chat.confirmation; + +import java.util.List; + +import org.eclipse.osgi.util.NLS; + +import com.microsoft.copilot.eclipse.core.chat.ConfirmationAction; +import com.microsoft.copilot.eclipse.core.chat.ConfirmationContent; +import com.microsoft.copilot.eclipse.core.chat.ConfirmationResult; +import com.microsoft.copilot.eclipse.core.lsp.protocol.InvokeClientToolConfirmationParams; +import com.microsoft.copilot.eclipse.ui.chat.Messages; + +/** + * Catch-all handler for tool types that have no dedicated handler registered. + * Always shows a simple confirmation dialog with Allow Once / Skip. + */ +public class FallbackConfirmationHandler implements ConfirmationHandler { + + @Override + public ConfirmationResult evaluate( + InvokeClientToolConfirmationParams params, + String sessionConversationId) { + String title = params.getTitle() != null + ? params.getTitle() + : NLS.bind(Messages.confirmation_title_fallback, params.getName()); + return ConfirmationResult.needsConfirmation( + new ConfirmationContent(title, params.getMessage(), + List.of( + ConfirmationAction.allowOnce( + Messages.confirmation_action_allowOnce), + ConfirmationAction.skip( + Messages.confirmation_action_skip)))); + } +} diff --git a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/messages.properties b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/messages.properties index 9fcf2413..6024e6ad 100644 --- a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/messages.properties +++ b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/messages.properties @@ -44,6 +44,7 @@ confirmation_action_alwaysAllow=Always Allow # Confirmation dialog titles confirmation_title_terminal=Run command in terminal +confirmation_title_fallback=Allow {0}? # Misc confirmation_autoApprovedDescription=Auto-approved from dialog