From d5c538eed91146203424ddcb688980c76082d2ee Mon Sep 17 00:00:00 2001 From: David Kaya Date: Tue, 24 Mar 2026 20:02:29 +0100 Subject: [PATCH] feat: add tool-specific approval overrides Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- ARCHITECTURE.md | 4 +- HANDOVER.md | 412 ++++++++++-------- README.md | 4 +- .../Contracts/ProtocolModels.cs | 1 + .../Services/CopilotWorkflowRunner.cs | 71 ++- .../CopilotWorkflowRunnerTests.cs | 95 ++++ src/main/EryxAppService.ts | 71 ++- src/main/ipc/registerIpcHandlers.ts | 6 + src/main/persistence/workspaceRepository.ts | 22 +- src/preload/index.ts | 2 + src/shared/contracts/channels.ts | 1 + src/shared/contracts/ipc.ts | 6 + src/shared/domain/approval.ts | 120 ++++- src/shared/domain/pattern.ts | 6 +- src/shared/domain/session.ts | 28 +- src/shared/domain/sessionLibrary.ts | 5 + src/shared/domain/tooling.ts | 108 +++++ tests/shared/approval.test.ts | 46 +- tests/shared/pattern.test.ts | 20 + tests/shared/session.test.ts | 29 ++ tests/shared/sessionLibrary.test.ts | 6 + tests/shared/tooling.test.ts | 55 +++ 22 files changed, 897 insertions(+), 221 deletions(-) diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index dff1b18..f58d174 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -176,6 +176,7 @@ Typical examples: - send message - update theme - toggle session tooling +- update session approval overrides The renderer does not reach into Electron or the filesystem directly. It talks through a constrained API surface. @@ -246,7 +247,8 @@ This is a good example of a cross-cutting concern that spans multiple layers wit Tooling is deliberately split into two levels: - **global definitions** for MCP servers and LSP profiles -- **per-session enablement** for which tools are active in a given run +- **pattern defaults** for which known tools can bypass manual approval +- **per-session overrides** for both tool enablement and tool auto-approval This lets the application treat tooling as reusable workspace capability while still preserving session-level control and safety. diff --git a/HANDOVER.md b/HANDOVER.md index 2a0c490..94cf9ae 100644 --- a/HANDOVER.md +++ b/HANDOVER.md @@ -1,226 +1,256 @@ -# Approval checkpoints UX handover +# Tool auto-approval backend handover -## What changed in the backend +## What changed -Approval checkpoints are now **queued per session** instead of failing when a second approval request arrives before the first one is resolved. +The backend now supports **tool-specific auto-approval defaults on patterns** plus **per-session overrides**. -This fixes the previous backend limitation where multi-agent patterns such as `Sequential Trio Review` could throw: +This builds on the existing approval-checkpoint system: -```text -Session "" already has a pending approval. -``` +- `final-response` approvals still work the same way +- `tool-call` checkpoints still decide **whether a tool call needs approval at all** +- the new feature decides **which known tools can be auto-approved instead of surfacing a manual approval** -The queue is backend-only in this change. The current frontend can keep working because the active approval is still exposed through the existing `session.pendingApproval` field. +The approval queue work from earlier is unchanged and still applies. -## Queue semantics +## Shared model -- A session still exposes **one active approval** through `session.pendingApproval`. -- Additional approvals waiting behind it are exposed through `session.pendingApprovalQueue`. -- `session.pendingApprovalQueue` does **not** include the active approval. -- Queue order is the order in which approvals were requested. -- `resolveSessionApproval(...)` still resolves **only the active approval**. -- When the active approval is resolved, the next queued approval automatically becomes the new `session.pendingApproval`. -- If the run fails while approvals remain queued, the backend rejects and clears the remaining queued approvals. -- If Eryx restarts while approvals are pending or queued, the backend rejects all of them and marks the session/run as errored. +### Pattern-level defaults -## Current shared model - -### Pattern approval policy - -This is unchanged from the earlier backend work: +Pattern defaults now live on `pattern.approvalPolicy.autoApprovedToolNames`. ```ts -type ApprovalCheckpointKind = 'tool-call' | 'final-response'; - -interface ApprovalCheckpointRule { - kind: ApprovalCheckpointKind; - agentIds?: string[]; // omitted or empty = all agents -} - interface ApprovalPolicy { rules: ApprovalCheckpointRule[]; + autoApprovedToolNames?: string[]; } ``` -### Session approval state - -The active approval + queued approvals are now represented as: - -```ts -interface PendingApprovalMessageRecord { - id: string; - authorName: string; - content: string; -} - -interface PendingApprovalRecord { - id: string; - kind: 'tool-call' | 'final-response'; - status: 'pending' | 'approved' | 'rejected'; - requestedAt: string; - resolvedAt?: string; - agentId?: string; - agentName?: string; - toolName?: string; - permissionKind?: string; - title: string; - detail?: string; - messages?: PendingApprovalMessageRecord[]; -} -``` - -Attached to `SessionRecord` as: - -```ts -pendingApproval?: PendingApprovalRecord; -pendingApprovalQueue?: PendingApprovalRecord[]; -``` - -Interpretation: - -- `pendingApproval` = the approval the user can act on **right now** -- `pendingApprovalQueue` = additional pending approvals waiting behind it - -For `final-response`, `pendingApproval.messages` contains the unpublished assistant output preview. - -For `tool-call`, `messages` is usually absent and the useful context is `agentName`, `toolName`, and `permissionKind`. - -## IPC surface - -No new IPC was added for queueing. - -The renderer still resolves approvals through: - -```ts -resolveSessionApproval({ - sessionId: string; - approvalId: string; - decision: 'approved' | 'rejected'; -}): Promise -``` - -Important behavior: - -- this resolves the **active** approval only -- if the user somehow attempts to resolve a queued approval directly, the backend rejects it -- after a successful resolution, the next queued approval may immediately become active in the returned workspace state - -## Timeline behavior - -The run timeline still uses `approval` events. - -Each approval request gets its own event keyed by `approvalId`. - -That means queued approvals now show up as multiple `approval` events in the same run when applicable. The active/queued distinction is in session state, not in the timeline event type. - -## Backend files changed for queue support - -- `src/shared/domain/approval.ts` - - added queue-state helpers and normalization -- `src/shared/domain/session.ts` - - added `pendingApprovalQueue` -- `src/shared/domain/sessionLibrary.ts` - - duplicated sessions now clear queued approvals too -- `src/main/persistence/workspaceRepository.ts` - - normalizes legacy single-approval data into active + queue state -- `src/main/EryxAppService.ts` - - enqueues approval requests instead of throwing - - advances the queue on resolution - - rejects all queued approvals on restart / failure cleanup - -## What the current frontend already does - -The current UI should remain functionally compatible because it already reads `session.pendingApproval` and renders a single active approval banner/card. - -That means: - -- users can still resolve approvals one at a time -- the next queued approval should appear automatically after resolving the current one -- no urgent UI fix is required just to keep the app working - -## Next UX steps for the frontend agent - -These are the next improvements the UX agent should make on top of the new backend queue. - -### 1. Show queue size next to the active approval - -Files to inspect: - -- `src/renderer/components/ChatPane.tsx` -- `src/renderer/components/ActivityPanel.tsx` -- `src/renderer/components/Sidebar.tsx` - -Recommended UX: - -- if `session.pendingApprovalQueue?.length > 0`, show a compact badge like: - - `1 queued` - - `2 queued` -- keep the active approval clearly distinguished from queued approvals - -### 2. Clarify that the visible approval is only the active one - -In the active approval card/banner, add copy such as: - -- `Approval 1 of 3` -- `Next approval will appear after this one is resolved` - -This will make automatic queue advancement feel intentional instead of surprising. - -### 3. Add an optional queued-approvals preview - -Files to inspect: - -- `src/renderer/components/ChatPane.tsx` -- `src/renderer/components/ActivityPanel.tsx` - -Recommended UX: - -- an accordion or compact list for `session.pendingApprovalQueue` -- each queued item can show: - - title - - kind - - agent name - - permission kind / tool name when available - Important: -- queued approvals are **read-only** -- only the active `pendingApproval` should show Approve / Reject buttons +- this list stores **runtime tool identifiers**, not display labels +- it is independent from the `rules` list +- a pattern may store tool auto-approval defaults even if `tool-call` checkpoints are currently disabled -### 4. Handle queue transitions smoothly +If `tool-call` approvals are not enabled for the relevant agent, the tool auto-approval list has no runtime effect. -When the active approval is resolved and the next one becomes active: +### Per-session override -- avoid jarring layout jumps -- keep scroll position stable -- consider a subtle transition so the user understands the queue advanced +Sessions now support an optional override object: -### 5. Improve error copy for queued-approval edge cases +```ts +interface SessionApprovalSettings { + autoApprovedToolNames: string[]; +} -Possible backend outcomes the UI should explain well: +interface SessionRecord { + approvalSettings?: SessionApprovalSettings; +} +``` -- active approval was resolved, then the run failed before the queue fully drained -- app restarted and the whole queue was rejected/interrupted -- user tried to resolve an approval that is no longer active +Semantics: -Recommended UX: +- `session.approvalSettings === undefined` → inherit the pattern defaults +- `session.approvalSettings.autoApprovedToolNames = []` → explicit session override that auto-approves **nothing** +- non-empty `autoApprovedToolNames` → explicit session override list -- inline error or toast -- refresh from returned workspace state -- preserve visibility into which approval is now active +Session overrides replace the pattern's tool auto-approval list for that session. They do **not** merge. -### 6. Optionally surface queue state in the timeline +## Runtime behavior -The timeline already records all approval events, but the UI could add: +The main process now applies session approval overrides before sending the pattern to the sidecar. -- an “active” vs “queued” distinction when an approval event is still pending -- badges or copy that explain multiple approval checkpoints exist in the same run +That means the sidecar sees the **effective** approval policy for the session: -This is optional because the core queue behavior is already represented in backend state. +- pattern rules +- pattern defaults when no session override exists +- session override list when present + +Actual decision flow for a tool call is now: + +1. Is `tool-call` approval enabled for this agent by `approvalPolicy.rules`? +2. If not, auto-approve as before. +3. If yes, is the runtime `toolName` in the effective `autoApprovedToolNames` list? +4. If yes, auto-approve without creating a pending approval. +5. Otherwise, emit a normal approval request and pause for user action. + +Unknown or non-tool-specific permission requests still require manual approval when `tool-call` checkpoints are active. + +## Tool identity contract + +Use the shared helper in `src/shared/domain/tooling.ts`: + +```ts +listApprovalToolDefinitions(workspace.settings.tooling) +``` + +This is the canonical source for UI rendering and validation. + +Each returned item includes: + +- `id` → the runtime tool identifier used by approvals +- `label` → human-readable label for the UI +- `kind` → `mcp`, `lsp`, or `mixed` +- `providerIds` +- `providerNames` + +Do **not** re-derive tool IDs in the renderer. + +### MCP tool IDs + +For MCP tools, the runtime ID is the raw tool name from `server.tools`. + +Example: + +```ts +{ + id: "git.status", + label: "git.status", + kind: "mcp" +} +``` + +### LSP tool IDs + +For LSP tools, the runtime ID is derived from the profile ID and operation name. + +Current operations: + +- `workspace_symbols` +- `document_symbols` +- `definition` +- `hover` +- `references` + +Example for profile ID `ts`: + +- `lsp_ts_workspace_symbols` +- `lsp_ts_document_symbols` +- `lsp_ts_definition` +- `lsp_ts_hover` +- `lsp_ts_references` + +Again: the renderer should consume the helper output instead of rebuilding these strings manually. + +## New IPC + +Renderer access is now available through: + +```ts +updateSessionApprovalSettings({ + sessionId: string; + autoApprovedToolNames?: string[]; +}): Promise +``` + +Semantics: + +- omit `autoApprovedToolNames` to clear the override and inherit the pattern defaults +- pass `[]` to keep an explicit session override with no auto-approved tools +- pass a populated array to set an explicit session override list + +The existing `savePattern(...)` path is still used for pattern defaults. + +## Validation and repair rules + +The backend now enforces and maintains tool references consistently: + +- saving a pattern rejects unknown `autoApprovedToolNames` +- updating session approval settings rejects unknown tool IDs +- scratchpad sessions reject non-empty session tool auto-approval overrides +- workspace load prunes stale tool IDs from patterns and sessions +- saving or deleting MCP/LSP definitions also prunes stale tool IDs from patterns and sessions + +This is intentional: stored approval defaults should only reference tools that still exist. + +## Approval request payload changes + +When the sidecar can identify the specific tool, `approval-requested` events now include `toolName`. + +Tool-call approval titles/details are also more specific when the tool is known. + +Examples: + +- title: `Approve lsp_ts_hover` +- detail: `Primary requested custom tool permission for tool "lsp_ts_hover" in Copilot session ...` + +Existing UI can keep working without changes, but the UX agent can now surface better tool context. + +## Files changed + +- `src/shared/domain/approval.ts` + - pattern defaults, session override model, effective-policy helpers, pruning/validation helpers +- `src/shared/domain/tooling.ts` + - canonical `listApprovalToolDefinitions(...)` helper +- `src/shared/domain/session.ts` + - session approval settings + effective pattern merge helper +- `src/main/EryxAppService.ts` + - pattern validation, session override IPC handler, tooling cleanup, effective-pattern merge +- `src/main/persistence/workspaceRepository.ts` + - load-time normalization + pruning for stale tool IDs +- `src/shared/contracts/ipc.ts` +- `src/shared/contracts/channels.ts` +- `src\preload\index.ts` +- `src\main\ipc\registerIpcHandlers.ts` +- `sidecar/src/Eryx.AgentHost/Contracts/ProtocolModels.cs` +- `sidecar/src/Eryx.AgentHost/Services/CopilotWorkflowRunner.cs` + - tool-name extraction from permission requests + - tool-specific auto-approval decision + +## UX work for the frontend agent + +### 1. Pattern editor: default tool auto-approval + +In `PatternEditor`, add a section that renders `listApprovalToolDefinitions(workspace.settings.tooling)`. + +Recommended behavior: + +- show every available approval tool with a toggle +- write the selected tool IDs to `pattern.approvalPolicy.autoApprovedToolNames` +- keep this separate from the existing `tool-call` / `final-response` checkpoint toggles +- if there are no tools, show an empty state instead of an interactive list + +### 2. Activity panel: per-session override + +In the right-side Activity panel, add a per-session auto-approval section for tools. + +Recommended behavior: + +- base the list on `listApprovalToolDefinitions(workspace.settings.tooling)` +- show the current **effective** state +- distinguish inherited vs overridden state in the UI +- allow resetting the session to inherit the pattern defaults +- call `updateSessionApprovalSettings(...)` when the user changes the override + +Suggested UX: + +- `Inheriting pattern defaults` badge/state when `session.approvalSettings` is `undefined` +- `Custom for this session` badge/state when the override object exists +- `Reset to pattern` action that calls `updateSessionApprovalSettings({ sessionId })` + +### 3. Disable while running + +Match the current tools behavior: + +- disable the per-session override controls while `session.status === 'running'` +- scratchpad sessions should show a non-interactive explanation because tool auto-approval does not apply there + +### 4. Surface tool context in approval UI + +Optional but recommended: + +- show `approval.toolName` in the active approval banner / queued approval list when present +- this is now available for tool-specific approval requests + +### 5. Avoid ID duplication logic in the renderer + +Important: + +- do not manually rebuild LSP tool IDs in UI code +- use the shared helper output and persist the `id` values from it ## Validation commands -Backend queue changes should be validated with: +Backend changes were validated with: - `bun run typecheck` - `bun test` diff --git a/README.md b/README.md index d3b30e1..6b7cd4c 100644 --- a/README.md +++ b/README.md @@ -47,6 +47,8 @@ You can define MCP servers and LSP profiles once in **Settings**, then enable th This keeps machine-wide tooling reusable while still letting each session decide which external tools the agent can use. +Patterns can also store default auto-approval for known MCP and LSP tools, and each session can override those defaults from the Activity panel before a run starts. + ### Watch runs as they happen You can follow agent activity while a session is running, which makes longer or more complex workflows easier to trust and understand. @@ -82,7 +84,7 @@ Eryx includes connection status in the app so you can quickly tell whether Copil Use a simple single-agent setup to begin, or choose a saved multi-agent pattern when you want a more structured workflow. 5. **Configure optional tooling** - If you want MCP or LSP support, add the global definitions in settings and then enable the ones you want for the current session from the Activity panel. + If you want MCP or LSP support, add the global definitions in settings and then enable the ones you want for the current session from the Activity panel. You can also set pattern-level tool auto-approval defaults and override them per session. 6. **Start working** Ask a question, describe a task, or explore a project. As the run progresses, you can watch the participating agents and keep the session for later. diff --git a/sidecar/src/Eryx.AgentHost/Contracts/ProtocolModels.cs b/sidecar/src/Eryx.AgentHost/Contracts/ProtocolModels.cs index a0b3698..e76ab00 100644 --- a/sidecar/src/Eryx.AgentHost/Contracts/ProtocolModels.cs +++ b/sidecar/src/Eryx.AgentHost/Contracts/ProtocolModels.cs @@ -30,6 +30,7 @@ public sealed class PatternDefinitionDto public sealed class ApprovalPolicyDto { public IReadOnlyList Rules { get; init; } = []; + public IReadOnlyList AutoApprovedToolNames { get; init; } = []; } public sealed class ApprovalCheckpointRuleDto diff --git a/sidecar/src/Eryx.AgentHost/Services/CopilotWorkflowRunner.cs b/sidecar/src/Eryx.AgentHost/Services/CopilotWorkflowRunner.cs index 2992f0c..c52ae50 100644 --- a/sidecar/src/Eryx.AgentHost/Services/CopilotWorkflowRunner.cs +++ b/sidecar/src/Eryx.AgentHost/Services/CopilotWorkflowRunner.cs @@ -242,7 +242,9 @@ public sealed class CopilotWorkflowRunner : ITurnWorkflowRunner Func onApproval, CancellationToken cancellationToken) { - if (!RequiresApproval(command.Pattern.ApprovalPolicy, "tool-call", agent.Id)) + TryGetApprovalToolName(request, out string? toolName); + + if (!RequiresToolCallApproval(command.Pattern.ApprovalPolicy, agent.Id, toolName)) { return new PermissionRequestResult { @@ -266,7 +268,7 @@ public sealed class CopilotWorkflowRunner : ITurnWorkflowRunner try { - await onApproval(BuildPermissionApprovalEvent(command, agent, request, invocation, approvalId)) + await onApproval(BuildPermissionApprovalEvent(command, agent, request, invocation, approvalId, toolName)) .ConfigureAwait(false); using CancellationTokenRegistration registration = cancellationToken.Register( @@ -383,12 +385,13 @@ public sealed class CopilotWorkflowRunner : ITurnWorkflowRunner return false; } - private static ApprovalRequestedEventDto BuildPermissionApprovalEvent( + internal static ApprovalRequestedEventDto BuildPermissionApprovalEvent( RunTurnCommandDto command, PatternAgentDefinitionDto agent, PermissionRequest request, PermissionInvocation invocation, - string approvalId) + string approvalId, + string? toolName) { string permissionKind = string.IsNullOrWhiteSpace(request.Kind) ? "tool access" @@ -397,6 +400,19 @@ public sealed class CopilotWorkflowRunner : ITurnWorkflowRunner string? sessionId = string.IsNullOrWhiteSpace(invocation.SessionId) ? null : invocation.SessionId.Trim(); + string? normalizedToolName = string.IsNullOrWhiteSpace(toolName) + ? null + : toolName.Trim(); + string title = normalizedToolName is null + ? $"Approve {permissionKind}" + : $"Approve {normalizedToolName}"; + string detail = normalizedToolName is null + ? sessionId is null + ? $"{agentName} requested {permissionKind} permission." + : $"{agentName} requested {permissionKind} permission for Copilot session {sessionId}." + : sessionId is null + ? $"{agentName} requested {permissionKind} permission for tool \"{normalizedToolName}\"." + : $"{agentName} requested {permissionKind} permission for tool \"{normalizedToolName}\" in Copilot session {sessionId}."; return new ApprovalRequestedEventDto { @@ -407,44 +423,69 @@ public sealed class CopilotWorkflowRunner : ITurnWorkflowRunner ApprovalKind = "tool-call", AgentId = string.IsNullOrWhiteSpace(agent.Id) ? null : agent.Id, AgentName = string.IsNullOrWhiteSpace(agentName) ? null : agentName, + ToolName = normalizedToolName, PermissionKind = permissionKind, - Title = $"Approve {permissionKind}", - Detail = sessionId is null - ? $"{agentName} requested {permissionKind} permission." - : $"{agentName} requested {permissionKind} permission for Copilot session {sessionId}.", + Title = title, + Detail = detail, }; } - private static bool RequiresApproval( + internal static bool RequiresToolCallApproval( ApprovalPolicyDto? approvalPolicy, - string checkpointKind, - string agentId) + string agentId, + string? toolName) { if (approvalPolicy?.Rules is null || approvalPolicy.Rules.Count == 0) { return false; } + bool matchesCheckpoint = false; foreach (ApprovalCheckpointRuleDto rule in approvalPolicy.Rules) { - if (!string.Equals(rule.Kind, checkpointKind, StringComparison.OrdinalIgnoreCase)) + if (!string.Equals(rule.Kind, "tool-call", StringComparison.OrdinalIgnoreCase)) { continue; } if (rule.AgentIds.Count == 0) { - return true; + matchesCheckpoint = true; + break; } if (rule.AgentIds.Any(candidate => string.Equals(candidate, agentId, StringComparison.OrdinalIgnoreCase))) { - return true; + matchesCheckpoint = true; + break; } } - return false; + if (!matchesCheckpoint) + { + return false; + } + + if (string.IsNullOrWhiteSpace(toolName)) + { + return true; + } + + return !approvalPolicy.AutoApprovedToolNames.Any(candidate => + string.Equals(candidate, toolName, StringComparison.OrdinalIgnoreCase)); + } + + internal static bool TryGetApprovalToolName(PermissionRequest request, out string? toolName) + { + toolName = request switch + { + PermissionRequestMcp mcp when !string.IsNullOrWhiteSpace(mcp.ToolName) => mcp.ToolName.Trim(), + PermissionRequestCustomTool customTool when !string.IsNullOrWhiteSpace(customTool.ToolName) => customTool.ToolName.Trim(), + _ => null, + }; + + return !string.IsNullOrWhiteSpace(toolName); } private static string CreateApprovalRequestId() diff --git a/sidecar/tests/Eryx.AgentHost.Tests/CopilotWorkflowRunnerTests.cs b/sidecar/tests/Eryx.AgentHost.Tests/CopilotWorkflowRunnerTests.cs index 4455c10..fa05ef9 100644 --- a/sidecar/tests/Eryx.AgentHost.Tests/CopilotWorkflowRunnerTests.cs +++ b/sidecar/tests/Eryx.AgentHost.Tests/CopilotWorkflowRunnerTests.cs @@ -1,5 +1,6 @@ using Eryx.AgentHost.Contracts; using Eryx.AgentHost.Services; +using GitHub.Copilot.SDK; using Microsoft.Extensions.AI; namespace Eryx.AgentHost.Tests; @@ -233,6 +234,100 @@ public sealed class CopilotWorkflowRunnerTests Assert.Equal("Real content", message.Content); } + [Fact] + public void RequiresToolCallApproval_HonorsAutoApprovedToolNames() + { + ApprovalPolicyDto policy = new() + { + Rules = + [ + new ApprovalCheckpointRuleDto + { + Kind = "tool-call", + AgentIds = ["agent-1"], + }, + ], + AutoApprovedToolNames = ["lsp_ts_hover"], + }; + + Assert.False(CopilotWorkflowRunner.RequiresToolCallApproval(policy, "agent-1", "lsp_ts_hover")); + Assert.True(CopilotWorkflowRunner.RequiresToolCallApproval(policy, "agent-1", "lsp_ts_definition")); + Assert.True(CopilotWorkflowRunner.RequiresToolCallApproval(policy, "agent-1", null)); + Assert.False(CopilotWorkflowRunner.RequiresToolCallApproval(policy, "agent-2", "lsp_ts_definition")); + } + + [Fact] + public void TryGetApprovalToolName_ReadsMcpAndCustomToolRequests() + { + Assert.True( + CopilotWorkflowRunner.TryGetApprovalToolName( + new PermissionRequestMcp + { + Kind = "mcp", + ServerName = "Git MCP", + ToolName = "git.status", + ToolTitle = "Git Status", + ReadOnly = true, + }, + out string? mcpToolName)); + Assert.Equal("git.status", mcpToolName); + + Assert.True( + CopilotWorkflowRunner.TryGetApprovalToolName( + new PermissionRequestCustomTool + { + Kind = "custom tool", + ToolName = "lsp_ts_hover", + ToolDescription = "Hover information", + }, + out string? customToolName)); + Assert.Equal("lsp_ts_hover", customToolName); + + Assert.False( + CopilotWorkflowRunner.TryGetApprovalToolName( + new PermissionRequestShell + { + Kind = "shell", + FullCommandText = "git status", + Intention = "Inspect repository state", + Commands = [], + PossiblePaths = [], + PossibleUrls = [], + HasWriteFileRedirection = false, + CanOfferSessionApproval = false, + }, + out string? shellToolName)); + Assert.Null(shellToolName); + } + + [Fact] + public void BuildPermissionApprovalEvent_IncludesToolContextWhenKnown() + { + ApprovalRequestedEventDto approvalEvent = CopilotWorkflowRunner.BuildPermissionApprovalEvent( + new RunTurnCommandDto + { + RequestId = "turn-1", + SessionId = "session-1", + }, + CreateAgent("agent-1", "Primary"), + new PermissionRequestCustomTool + { + Kind = "custom tool", + ToolName = "lsp_ts_hover", + ToolDescription = "Hover information", + }, + new PermissionInvocation + { + SessionId = "copilot-session-1", + }, + "approval-1", + "lsp_ts_hover"); + + Assert.Equal("lsp_ts_hover", approvalEvent.ToolName); + Assert.Equal("Approve lsp_ts_hover", approvalEvent.Title); + Assert.Contains("tool \"lsp_ts_hover\"", approvalEvent.Detail); + } + private static PatternAgentDefinitionDto CreateAgent(string id, string name) { return new PatternAgentDefinitionDto diff --git a/src/main/EryxAppService.ts b/src/main/EryxAppService.ts index 36436e0..174a478 100644 --- a/src/main/EryxAppService.ts +++ b/src/main/EryxAppService.ts @@ -30,6 +30,9 @@ import { enqueuePendingApprovalState, listPendingApprovals, normalizeApprovalPolicy, + normalizeSessionApprovalSettings, + pruneApprovalPolicyTools, + pruneSessionApprovalSettings, resolvePendingApproval, type ApprovalDecision, type PendingApprovalMessageRecord, @@ -45,6 +48,7 @@ import { } from '@shared/domain/sessionLibrary'; import type { SessionEventRecord } from '@shared/domain/event'; import { + applySessionApprovalSettings, applyScratchpadSessionConfig, resolveSessionToolingSelection, createScratchpadSessionConfig, @@ -64,6 +68,7 @@ import { } from '@shared/domain/runTimeline'; import { createSessionToolingSelection, + listApprovalToolNames, normalizeTheme, type AppearanceTheme, type LspProfileDefinition, @@ -195,7 +200,10 @@ export class EryxAppService extends EventEmitter { async savePattern(pattern: PatternDefinition): Promise { const workspace = await this.loadWorkspace(); - const issues = validatePatternDefinition(pattern).filter((issue) => issue.level === 'error'); + const issues = validatePatternDefinition( + pattern, + this.listKnownApprovalToolNames(workspace), + ).filter((issue) => issue.level === 'error'); if (issues.length > 0) { throw new Error(issues[0].message); } @@ -273,6 +281,7 @@ export class EryxAppService extends EventEmitter { workspace.settings.tooling.mcpServers.push(candidate); } + this.pruneUnavailableApprovalTools(workspace); return this.persistAndBroadcast(workspace); } @@ -290,6 +299,7 @@ export class EryxAppService extends EventEmitter { }; } + this.pruneUnavailableApprovalTools(workspace); return this.persistAndBroadcast(workspace); } @@ -318,6 +328,7 @@ export class EryxAppService extends EventEmitter { workspace.settings.tooling.lspProfiles.push(candidate); } + this.pruneUnavailableApprovalTools(workspace); return this.persistAndBroadcast(workspace); } @@ -335,6 +346,7 @@ export class EryxAppService extends EventEmitter { }; } + this.pruneUnavailableApprovalTools(workspace); return this.persistAndBroadcast(workspace); } @@ -667,6 +679,41 @@ export class EryxAppService extends EventEmitter { return this.persistAndBroadcast(workspace); } + async updateSessionApprovalSettings( + sessionId: string, + autoApprovedToolNames?: string[], + ): Promise { + const workspace = await this.loadWorkspace(); + const session = this.requireSession(workspace, sessionId); + const project = this.requireProject(workspace, session.projectId); + + if (session.status === 'running') { + throw new Error('Wait for the current response to finish before changing session approval settings.'); + } + + const settings = normalizeSessionApprovalSettings( + autoApprovedToolNames === undefined ? undefined : { autoApprovedToolNames }, + ); + + if ( + isScratchpadProject(project) + && settings + && settings.autoApprovedToolNames.length > 0 + ) { + throw new Error('Scratchpad sessions do not support tool auto-approval settings.'); + } + + const knownToolNames = new Set(this.listKnownApprovalToolNames(workspace)); + const unknownToolName = settings?.autoApprovedToolNames.find((toolName) => !knownToolNames.has(toolName)); + if (unknownToolName) { + throw new Error(`Unknown approval tool "${unknownToolName}".`); + } + + session.approvalSettings = settings; + session.updatedAt = nowIso(); + return this.persistAndBroadcast(workspace); + } + async querySessions(input: QuerySessionsInput): Promise { const workspace = await this.loadWorkspace(); return queryWorkspaceSessions(workspace, input); @@ -1119,9 +1166,29 @@ export class EryxAppService extends EventEmitter { const patternWithSessionConfig = isScratchpadProject(project) ? applyScratchpadSessionConfig(pattern, session) : pattern; + const patternWithApprovalSettings = applySessionApprovalSettings(patternWithSessionConfig, session); const modelCatalog = await this.loadAvailableModelCatalog(); - return normalizePatternModels(patternWithSessionConfig, modelCatalog); + return normalizePatternModels(patternWithApprovalSettings, modelCatalog); + } + + private listKnownApprovalToolNames(workspace: WorkspaceState): string[] { + return listApprovalToolNames(workspace.settings.tooling); + } + + private pruneUnavailableApprovalTools(workspace: WorkspaceState): void { + const knownToolNames = this.listKnownApprovalToolNames(workspace); + + for (const pattern of workspace.patterns) { + pattern.approvalPolicy = pruneApprovalPolicyTools(pattern.approvalPolicy, knownToolNames); + } + + for (const session of workspace.sessions) { + session.approvalSettings = pruneSessionApprovalSettings( + session.approvalSettings, + knownToolNames, + ); + } } private buildRunTurnToolingConfig( diff --git a/src/main/ipc/registerIpcHandlers.ts b/src/main/ipc/registerIpcHandlers.ts index 8a24477..38775da 100644 --- a/src/main/ipc/registerIpcHandlers.ts +++ b/src/main/ipc/registerIpcHandlers.ts @@ -13,6 +13,7 @@ import type { SetPatternFavoriteInput, SetSessionArchivedInput, SetSessionPinnedInput, + UpdateSessionApprovalSettingsInput, UpdateSessionToolingInput, UpdateScratchpadSessionConfigInput, } from '@shared/contracts/ipc'; @@ -60,6 +61,11 @@ export function registerIpcHandlers(window: BrowserWindow, service: EryxAppServi input.enabledLspProfileIds, ), ); + ipcMain.handle( + ipcChannels.updateSessionApprovalSettings, + (_event, input: UpdateSessionApprovalSettingsInput) => + service.updateSessionApprovalSettings(input.sessionId, input.autoApprovedToolNames), + ); ipcMain.handle(ipcChannels.createSession, (_event, input: CreateSessionInput) => service.createSession(input.projectId, input.patternId), ); diff --git a/src/main/persistence/workspaceRepository.ts b/src/main/persistence/workspaceRepository.ts index 7422574..63834a4 100644 --- a/src/main/persistence/workspaceRepository.ts +++ b/src/main/persistence/workspaceRepository.ts @@ -4,10 +4,17 @@ import { createBuiltinPatterns } from '@shared/domain/pattern'; import type { PatternDefinition } from '@shared/domain/pattern'; import { mergeScratchpadProject } from '@shared/domain/project'; import { normalizeSessionRunRecords } from '@shared/domain/runTimeline'; -import { normalizeSessionToolingSelection, normalizeWorkspaceSettings } from '@shared/domain/tooling'; +import { + listApprovalToolNames, + normalizeSessionToolingSelection, + normalizeWorkspaceSettings, +} from '@shared/domain/tooling'; import { normalizeApprovalPolicy, normalizePendingApprovalState, + normalizeSessionApprovalSettings, + pruneApprovalPolicyTools, + pruneSessionApprovalSettings, } from '@shared/domain/approval'; import { createWorkspaceSeed, type WorkspaceState } from '@shared/domain/workspace'; import { nowIso } from '@shared/utils/ids'; @@ -60,24 +67,33 @@ export class WorkspaceRepository { } const projects = mergeScratchpadProject(stored.projects ?? [], this.scratchpadPath); + const settings = normalizeWorkspaceSettings(stored.settings); + const knownToolNames = listApprovalToolNames(settings.tooling); const workspace: WorkspaceState = { ...stored, patterns: mergePatterns(stored.patterns ?? []).map((pattern) => ({ ...pattern, - approvalPolicy: normalizeApprovalPolicy(pattern.approvalPolicy), + approvalPolicy: pruneApprovalPolicyTools( + normalizeApprovalPolicy(pattern.approvalPolicy), + knownToolNames, + ), })), projects, sessions: (stored.sessions ?? []).map((session) => ({ ...session, runs: normalizeSessionRunRecords(session.runs), tooling: normalizeSessionToolingSelection(session.tooling), + approvalSettings: pruneSessionApprovalSettings( + normalizeSessionApprovalSettings(session.approvalSettings), + knownToolNames, + ), ...normalizePendingApprovalState({ pendingApproval: session.pendingApproval, pendingApprovalQueue: session.pendingApprovalQueue, }), })), - settings: normalizeWorkspaceSettings(stored.settings), + settings, selectedProjectId: projects.some((project) => project.id === stored.selectedProjectId) ? stored.selectedProjectId : projects[0]?.id, diff --git a/src/preload/index.ts b/src/preload/index.ts index cc52507..9ede59f 100644 --- a/src/preload/index.ts +++ b/src/preload/index.ts @@ -19,6 +19,8 @@ const api: ElectronApi = { saveLspProfile: (input) => ipcRenderer.invoke(ipcChannels.saveLspProfile, input), deleteLspProfile: (profileId) => ipcRenderer.invoke(ipcChannels.deleteLspProfile, profileId), updateSessionTooling: (input) => ipcRenderer.invoke(ipcChannels.updateSessionTooling, input), + updateSessionApprovalSettings: (input) => + ipcRenderer.invoke(ipcChannels.updateSessionApprovalSettings, input), createSession: (input) => ipcRenderer.invoke(ipcChannels.createSession, input), duplicateSession: (input) => ipcRenderer.invoke(ipcChannels.duplicateSession, input), renameSession: (input) => ipcRenderer.invoke(ipcChannels.renameSession, input), diff --git a/src/shared/contracts/channels.ts b/src/shared/contracts/channels.ts index f854049..72af03f 100644 --- a/src/shared/contracts/channels.ts +++ b/src/shared/contracts/channels.ts @@ -14,6 +14,7 @@ export const ipcChannels = { saveLspProfile: 'tooling:lsp:save', deleteLspProfile: 'tooling:lsp:delete', updateSessionTooling: 'sessions:update-tooling', + updateSessionApprovalSettings: 'sessions:update-approval-settings', createSession: 'sessions:create', duplicateSession: 'sessions:duplicate', renameSession: 'sessions:rename', diff --git a/src/shared/contracts/ipc.ts b/src/shared/contracts/ipc.ts index 11a956f..c48db84 100644 --- a/src/shared/contracts/ipc.ts +++ b/src/shared/contracts/ipc.ts @@ -74,6 +74,11 @@ export interface UpdateSessionToolingInput extends SessionToolingSelection { sessionId: string; } +export interface UpdateSessionApprovalSettingsInput { + sessionId: string; + autoApprovedToolNames?: string[]; +} + export interface ElectronApi { describeSidecarCapabilities(): Promise; refreshSidecarCapabilities(): Promise; @@ -88,6 +93,7 @@ export interface ElectronApi { saveLspProfile(input: SaveLspProfileInput): Promise; deleteLspProfile(profileId: string): Promise; updateSessionTooling(input: UpdateSessionToolingInput): Promise; + updateSessionApprovalSettings(input: UpdateSessionApprovalSettingsInput): Promise; createSession(input: CreateSessionInput): Promise; duplicateSession(input: DuplicateSessionInput): Promise; renameSession(input: RenameSessionInput): Promise; diff --git a/src/shared/domain/approval.ts b/src/shared/domain/approval.ts index 868032d..8eb1cdf 100644 --- a/src/shared/domain/approval.ts +++ b/src/shared/domain/approval.ts @@ -9,6 +9,11 @@ export interface ApprovalCheckpointRule { export interface ApprovalPolicy { rules: ApprovalCheckpointRule[]; + autoApprovedToolNames?: string[]; +} + +export interface SessionApprovalSettings { + autoApprovedToolNames: string[]; } export interface PendingApprovalMessageRecord { @@ -53,6 +58,7 @@ export function normalizeApprovalPolicy(policy?: Partial): Appro const rules = Array.isArray(policy?.rules) ? policy.rules : []; const selectedAgents = new Map>(); const appliesToAllAgents = new Set(); + const autoApprovedToolNames = normalizeStringArray(policy?.autoApprovedToolNames); for (const rule of rules) { if (!isApprovalCheckpointKind(rule?.kind)) { @@ -90,18 +96,39 @@ export function normalizeApprovalPolicy(policy?: Partial): Appro return [{ kind, agentIds }]; }); - return normalizedRules.length > 0 ? { rules: normalizedRules } : undefined; + if (normalizedRules.length === 0 && autoApprovedToolNames.length === 0) { + return undefined; + } + + return { + rules: normalizedRules, + autoApprovedToolNames: autoApprovedToolNames.length > 0 ? autoApprovedToolNames : undefined, + }; +} + +export function normalizeSessionApprovalSettings( + settings?: Partial, +): SessionApprovalSettings | undefined { + if (settings == null) { + return undefined; + } + + return { + autoApprovedToolNames: normalizeStringArray(settings.autoApprovedToolNames), + }; } export function validateApprovalPolicy( policy: ApprovalPolicy | undefined, knownAgentIds: readonly string[], + knownToolNames?: readonly string[], ): string[] { if (!policy) { return []; } const knownAgents = new Set(normalizeStringArray(knownAgentIds)); + const knownTools = knownToolNames ? new Set(normalizeStringArray(knownToolNames)) : undefined; const issues: string[] = []; for (const rule of policy.rules) { @@ -112,6 +139,14 @@ export function validateApprovalPolicy( } } + if (knownTools) { + for (const toolName of policy.autoApprovedToolNames ?? []) { + if (!knownTools.has(toolName)) { + issues.push(`Approval auto-approve references unknown tool "${toolName}".`); + } + } + } + return issues; } @@ -137,6 +172,81 @@ export function approvalPolicyRequiresCheckpoint( return rule.agentIds.includes(normalizedAgentId); } +export function approvalPolicyAutoApprovesTool( + policy: ApprovalPolicy | undefined, + toolName?: string, +): boolean { + const normalizedToolName = normalizeOptionalString(toolName); + if (!normalizedToolName) { + return false; + } + + return policy?.autoApprovedToolNames?.includes(normalizedToolName) ?? false; +} + +export function approvalPolicyRequiresToolCallApproval( + policy: ApprovalPolicy | undefined, + agentId?: string, + toolName?: string, +): boolean { + if (!approvalPolicyRequiresCheckpoint(policy, 'tool-call', agentId)) { + return false; + } + + return !approvalPolicyAutoApprovesTool(policy, toolName); +} + +export function resolveEffectiveApprovalPolicy( + policy: ApprovalPolicy | undefined, + sessionSettings?: Partial, +): ApprovalPolicy | undefined { + if (sessionSettings === undefined) { + return normalizeApprovalPolicy(policy); + } + + const normalizedPolicy = normalizeApprovalPolicy(policy); + const normalizedSessionSettings = normalizeSessionApprovalSettings(sessionSettings); + return normalizeApprovalPolicy({ + rules: normalizedPolicy?.rules ?? [], + autoApprovedToolNames: normalizedSessionSettings?.autoApprovedToolNames, + }); +} + +export function pruneApprovalPolicyTools( + policy: ApprovalPolicy | undefined, + knownToolNames: readonly string[], +): ApprovalPolicy | undefined { + const normalizedPolicy = normalizeApprovalPolicy(policy); + if (!normalizedPolicy) { + return undefined; + } + + return normalizeApprovalPolicy({ + ...normalizedPolicy, + autoApprovedToolNames: filterKnownToolNames( + normalizedPolicy.autoApprovedToolNames, + knownToolNames, + ), + }); +} + +export function pruneSessionApprovalSettings( + settings: SessionApprovalSettings | undefined, + knownToolNames: readonly string[], +): SessionApprovalSettings | undefined { + const normalizedSettings = normalizeSessionApprovalSettings(settings); + if (normalizedSettings === undefined) { + return undefined; + } + + return { + autoApprovedToolNames: filterKnownToolNames( + normalizedSettings.autoApprovedToolNames, + knownToolNames, + ), + }; +} + export function normalizePendingApproval( approval?: Partial, ): PendingApprovalRecord | undefined { @@ -283,3 +393,11 @@ function normalizeStringArray(values?: ReadonlyArray): string[] { return [...new Set(values.map((value) => value.trim()).filter((value) => value.length > 0))]; } + +function filterKnownToolNames( + toolNames: readonly string[] | undefined, + knownToolNames: readonly string[], +): string[] { + const knownTools = new Set(normalizeStringArray(knownToolNames)); + return normalizeStringArray(toolNames).filter((toolName) => knownTools.has(toolName)); +} diff --git a/src/shared/domain/pattern.ts b/src/shared/domain/pattern.ts index 785d7e1..4bf52e1 100644 --- a/src/shared/domain/pattern.ts +++ b/src/shared/domain/pattern.ts @@ -258,7 +258,10 @@ export function createBuiltinPatterns(timestamp: string): PatternDefinition[] { ]; } -export function validatePatternDefinition(pattern: PatternDefinition): PatternValidationIssue[] { +export function validatePatternDefinition( + pattern: PatternDefinition, + knownToolNames?: readonly string[], +): PatternValidationIssue[] { const issues: PatternValidationIssue[] = []; if (!pattern.name.trim()) { @@ -324,6 +327,7 @@ export function validatePatternDefinition(pattern: PatternDefinition): PatternVa for (const message of validateApprovalPolicy( normalizeApprovalPolicy(pattern.approvalPolicy), pattern.agents.map((agent) => agent.id), + knownToolNames, )) { issues.push({ level: 'error', diff --git a/src/shared/domain/session.ts b/src/shared/domain/session.ts index d1ef80a..1f06231 100644 --- a/src/shared/domain/session.ts +++ b/src/shared/domain/session.ts @@ -4,7 +4,12 @@ import { normalizeSessionToolingSelection, type SessionToolingSelection, } from '@shared/domain/tooling'; -import type { PendingApprovalRecord } from '@shared/domain/approval'; +import { + normalizeSessionApprovalSettings, + resolveEffectiveApprovalPolicy, + type PendingApprovalRecord, + type SessionApprovalSettings, +} from '@shared/domain/approval'; import type { SessionRunRecord } from '@shared/domain/runTimeline'; export type ChatRole = 'system' | 'user' | 'assistant'; @@ -40,6 +45,7 @@ export interface SessionRecord { lastError?: string; scratchpadConfig?: ScratchpadSessionConfig; tooling?: SessionToolingSelection; + approvalSettings?: SessionApprovalSettings; pendingApproval?: PendingApprovalRecord; pendingApprovalQueue?: PendingApprovalRecord[]; runs: SessionRunRecord[]; @@ -77,6 +83,12 @@ export function resolveSessionToolingSelection( return normalizeSessionToolingSelection(session.tooling ?? createSessionToolingSelection()); } +export function resolveSessionApprovalSettings( + session: Pick, +): SessionApprovalSettings | undefined { + return normalizeSessionApprovalSettings(session.approvalSettings); +} + export function resolveScratchpadSessionConfig( session: SessionRecord, pattern: PatternDefinition, @@ -115,3 +127,17 @@ export function applyScratchpadSessionConfig( ], }; } + +export function applySessionApprovalSettings( + pattern: PatternDefinition, + session: Pick, +): PatternDefinition { + if (session.approvalSettings === undefined) { + return pattern; + } + + return { + ...pattern, + approvalPolicy: resolveEffectiveApprovalPolicy(pattern.approvalPolicy, session.approvalSettings), + }; +} diff --git a/src/shared/domain/sessionLibrary.ts b/src/shared/domain/sessionLibrary.ts index b76d8f3..103833b 100644 --- a/src/shared/domain/sessionLibrary.ts +++ b/src/shared/domain/sessionLibrary.ts @@ -181,6 +181,11 @@ export function duplicateSessionRecord( enabledLspProfileIds: [...session.tooling.enabledLspProfileIds], } : undefined, + approvalSettings: session.approvalSettings + ? { + autoApprovedToolNames: [...session.approvalSettings.autoApprovedToolNames], + } + : undefined, pendingApproval: undefined, pendingApprovalQueue: undefined, runs: [], diff --git a/src/shared/domain/tooling.ts b/src/shared/domain/tooling.ts index ecacf2c..a9fb9ec 100644 --- a/src/shared/domain/tooling.ts +++ b/src/shared/domain/tooling.ts @@ -52,6 +52,24 @@ export interface SessionToolingSelection { enabledLspProfileIds: string[]; } +export type ApprovalToolKind = 'mcp' | 'lsp' | 'mixed'; + +export interface ApprovalToolDefinition { + id: string; + label: string; + kind: ApprovalToolKind; + providerIds: string[]; + providerNames: string[]; +} + +const lspApprovalOperations = [ + { suffix: 'workspace_symbols', label: 'Workspace symbols' }, + { suffix: 'document_symbols', label: 'Document symbols' }, + { suffix: 'definition', label: 'Definition' }, + { suffix: 'hover', label: 'Hover' }, + { suffix: 'references', label: 'References' }, +] as const; + export function createWorkspaceSettings(): WorkspaceSettings { return { theme: 'dark', @@ -94,6 +112,44 @@ export function normalizeSessionToolingSelection( }; } +export function listApprovalToolDefinitions( + tooling: WorkspaceToolingSettings, +): ApprovalToolDefinition[] { + const toolsById = new Map(); + + for (const server of tooling.mcpServers) { + for (const toolName of normalizeStringArray(server.tools)) { + registerApprovalTool(toolsById, { + id: toolName, + label: toolName, + kind: 'mcp', + providerId: server.id, + providerName: server.name, + }); + } + } + + for (const profile of tooling.lspProfiles) { + const toolPrefix = buildLspApprovalToolPrefix(profile.id); + for (const operation of lspApprovalOperations) { + registerApprovalTool(toolsById, { + id: `${toolPrefix}_${operation.suffix}`, + label: `${profile.name} · ${operation.label}`, + kind: 'lsp', + providerId: profile.id, + providerName: profile.name, + }); + } + } + + return [...toolsById.values()].sort((left, right) => + left.label.localeCompare(right.label) || left.id.localeCompare(right.id)); +} + +export function listApprovalToolNames(tooling: WorkspaceToolingSettings): string[] { + return listApprovalToolDefinitions(tooling).map((tool) => tool.id); +} + export function validateMcpServerDefinition(server: McpServerDefinition): string | undefined { if (!server.name.trim()) { return 'MCP server name is required.'; @@ -196,6 +252,58 @@ function requiresTypeScriptLanguageServerStdio(command: string): boolean { || executableName === 'typescript-language-server.exe'; } +function buildLspApprovalToolPrefix(value: string): string { + let prefix = ''; + for (const char of value) { + if (/[a-z0-9]/i.test(char)) { + prefix += char.toLowerCase(); + continue; + } + + if (!prefix || prefix.endsWith('_')) { + continue; + } + + prefix += '_'; + } + + const normalized = prefix.replace(/^_+|_+$/g, ''); + return normalized ? `lsp_${normalized}` : 'lsp'; +} + +function registerApprovalTool( + toolsById: Map, + tool: { + id: string; + label: string; + kind: Exclude; + providerId: string; + providerName: string; + }, +): void { + const existing = toolsById.get(tool.id); + if (!existing) { + toolsById.set(tool.id, { + id: tool.id, + label: tool.label, + kind: tool.kind, + providerIds: [tool.providerId], + providerNames: [tool.providerName], + }); + return; + } + + if (!existing.providerIds.includes(tool.providerId)) { + existing.providerIds.push(tool.providerId); + } + if (!existing.providerNames.includes(tool.providerName)) { + existing.providerNames.push(tool.providerName); + } + if (existing.kind !== tool.kind) { + existing.kind = 'mixed'; + } +} + function normalizeStringArray(values?: ReadonlyArray): string[] { if (!values) { return []; diff --git a/tests/shared/approval.test.ts b/tests/shared/approval.test.ts index 81f124f..1c2650e 100644 --- a/tests/shared/approval.test.ts +++ b/tests/shared/approval.test.ts @@ -1,28 +1,33 @@ import { describe, expect, test } from 'bun:test'; import { + approvalPolicyRequiresToolCallApproval, + approvalPolicyRequiresCheckpoint, + normalizeApprovalPolicy, + normalizePendingApproval, + normalizePendingApprovalState, + normalizeSessionApprovalSettings, dequeuePendingApprovalState, enqueuePendingApprovalState, listPendingApprovals, - approvalPolicyRequiresCheckpoint, - normalizePendingApprovalState, - normalizeApprovalPolicy, - normalizePendingApproval, + resolveEffectiveApprovalPolicy, } from '@shared/domain/approval'; describe('approval helpers', () => { - test('normalizes duplicate checkpoint rules into stable agent-scoped policy entries', () => { + test('normalizes duplicate checkpoint rules and auto-approved tools into stable policy entries', () => { expect(normalizeApprovalPolicy({ rules: [ { kind: 'tool-call', agentIds: ['agent-1', ' agent-1 ', 'agent-2'] }, { kind: 'tool-call', agentIds: ['agent-2', 'agent-3'] }, { kind: 'final-response', agentIds: [] }, ], + autoApprovedToolNames: [' git.status ', 'git.status', 'lsp_ts_hover'], })).toEqual({ rules: [ { kind: 'tool-call', agentIds: ['agent-1', 'agent-2', 'agent-3'] }, { kind: 'final-response' }, ], + autoApprovedToolNames: ['git.status', 'lsp_ts_hover'], }); }); @@ -32,11 +37,42 @@ describe('approval helpers', () => { { kind: 'tool-call', agentIds: ['agent-1'] }, { kind: 'final-response', agentIds: [] }, ], + autoApprovedToolNames: ['git.status'], }); expect(approvalPolicyRequiresCheckpoint(policy, 'tool-call', 'agent-1')).toBe(true); expect(approvalPolicyRequiresCheckpoint(policy, 'tool-call', 'agent-2')).toBe(false); expect(approvalPolicyRequiresCheckpoint(policy, 'final-response', 'agent-2')).toBe(true); + expect(approvalPolicyRequiresToolCallApproval(policy, 'agent-1', 'git.status')).toBe(false); + expect(approvalPolicyRequiresToolCallApproval(policy, 'agent-1', 'git.diff')).toBe(true); + expect(approvalPolicyRequiresToolCallApproval(policy, 'agent-2', 'git.diff')).toBe(false); + }); + + test('resolves session approval settings over pattern auto-approval defaults', () => { + expect(resolveEffectiveApprovalPolicy( + { + rules: [{ kind: 'tool-call' }], + autoApprovedToolNames: ['git.status'], + }, + normalizeSessionApprovalSettings({ + autoApprovedToolNames: ['git.diff'], + }), + )).toEqual({ + rules: [{ kind: 'tool-call' }], + autoApprovedToolNames: ['git.diff'], + }); + + expect(resolveEffectiveApprovalPolicy( + { + rules: [{ kind: 'tool-call' }], + autoApprovedToolNames: ['git.status'], + }, + normalizeSessionApprovalSettings({ + autoApprovedToolNames: [], + }), + )).toEqual({ + rules: [{ kind: 'tool-call' }], + }); }); test('normalizes pending approvals with optional message previews', () => { diff --git a/tests/shared/pattern.test.ts b/tests/shared/pattern.test.ts index 70d80f0..a9e29cc 100644 --- a/tests/shared/pattern.test.ts +++ b/tests/shared/pattern.test.ts @@ -125,4 +125,24 @@ describe('pattern validation', () => { 'Approval checkpoint "tool-call" references unknown agent "agent-missing".', ); }); + + test('approval policy rejects unknown auto-approved tool references when tool names are provided', () => { + const singlePattern = createBuiltinPatterns(BUILTIN_TIMESTAMP).find( + (pattern) => pattern.mode === 'single', + ); + + expect(singlePattern).toBeDefined(); + + const issues = validatePatternDefinition({ + ...singlePattern!, + approvalPolicy: { + rules: [{ kind: 'tool-call' }], + autoApprovedToolNames: ['git.status', 'unknown.tool'], + }, + }, ['git.status']); + + expect(issues.find((issue) => issue.field === 'approvalPolicy')?.message).toBe( + 'Approval auto-approve references unknown tool "unknown.tool".', + ); + }); }); diff --git a/tests/shared/session.test.ts b/tests/shared/session.test.ts index d6b7d86..7686e67 100644 --- a/tests/shared/session.test.ts +++ b/tests/shared/session.test.ts @@ -2,8 +2,10 @@ import { describe, expect, test } from 'bun:test'; import type { PatternDefinition } from '@shared/domain/pattern'; import { + applySessionApprovalSettings, applyScratchpadSessionConfig, createScratchpadSessionConfig, + resolveSessionApprovalSettings, resolveSessionToolingSelection, resolveSessionTitle, resolveScratchpadSessionConfig, @@ -143,3 +145,30 @@ describe('session tooling helpers', () => { }); }); }); + +describe('session approval helpers', () => { + test('normalizes session approval overrides and applies them over pattern defaults', () => { + const pattern = { + ...createPattern(), + approvalPolicy: { + rules: [{ kind: 'tool-call' as const }], + autoApprovedToolNames: ['git.status'], + }, + }; + + expect(resolveSessionApprovalSettings(createSession())).toBeUndefined(); + expect( + applySessionApprovalSettings( + pattern, + createSession({ + approvalSettings: { + autoApprovedToolNames: ['git.diff', ' git.diff '], + }, + }), + ).approvalPolicy, + ).toEqual({ + rules: [{ kind: 'tool-call' }], + autoApprovedToolNames: ['git.diff'], + }); + }); +}); diff --git a/tests/shared/sessionLibrary.test.ts b/tests/shared/sessionLibrary.test.ts index 4bfd9ed..8022a98 100644 --- a/tests/shared/sessionLibrary.test.ts +++ b/tests/shared/sessionLibrary.test.ts @@ -122,6 +122,9 @@ describe('session library helpers', () => { isPinned: true, isArchived: true, lastError: 'sidecar crashed', + approvalSettings: { + autoApprovedToolNames: ['git.status'], + }, pendingApproval: { id: 'approval-1', kind: 'tool-call', @@ -165,6 +168,9 @@ describe('session library helpers', () => { updatedAt: '2026-03-23T00:07:00.000Z', }); expect(session.messages[0]?.pending).toBe(false); + expect(session.approvalSettings).toEqual({ + autoApprovedToolNames: ['git.status'], + }); expect(session.pendingApproval).toBeUndefined(); expect(session.pendingApprovalQueue).toBeUndefined(); expect(session.runs).toEqual([]); diff --git a/tests/shared/tooling.test.ts b/tests/shared/tooling.test.ts index ffb26cf..0538943 100644 --- a/tests/shared/tooling.test.ts +++ b/tests/shared/tooling.test.ts @@ -1,6 +1,7 @@ import { describe, expect, test } from 'bun:test'; import { + listApprovalToolDefinitions, normalizeWorkspaceSettings, validateLspProfileDefinition, validateMcpServerDefinition, @@ -155,4 +156,58 @@ describe('tooling settings helpers', () => { }), ).toBe('LSP profile "Typescript LSP" needs the "--stdio" argument.'); }); + + test('lists approval tools from MCP and LSP definitions using runtime tool identifiers', () => { + const tools = listApprovalToolDefinitions({ + mcpServers: [ + { + id: 'mcp-git', + name: 'Git MCP', + transport: 'local', + command: 'node', + args: ['server.js'], + tools: ['git.status', 'git.diff'], + createdAt: TIMESTAMP, + updatedAt: TIMESTAMP, + }, + { + id: 'mcp-extra', + name: 'Extra MCP', + transport: 'local', + command: 'node', + args: ['extra.js'], + tools: ['git.status'], + createdAt: TIMESTAMP, + updatedAt: TIMESTAMP, + }, + ], + lspProfiles: [ + { + id: 'ts', + name: 'TypeScript', + command: 'typescript-language-server', + args: ['--stdio'], + languageId: 'typescript', + fileExtensions: ['.ts', '.tsx'], + createdAt: TIMESTAMP, + updatedAt: TIMESTAMP, + }, + ], + }); + + expect(tools).toContainEqual({ + id: 'git.status', + label: 'git.status', + kind: 'mcp', + providerIds: ['mcp-git', 'mcp-extra'], + providerNames: ['Git MCP', 'Extra MCP'], + }); + expect(tools).toContainEqual({ + id: 'lsp_ts_hover', + label: 'TypeScript · Hover', + kind: 'lsp', + providerIds: ['ts'], + providerNames: ['TypeScript'], + }); + }); });