diff --git a/src/renderer/components/ChatPane.tsx b/src/renderer/components/ChatPane.tsx index 5010c45..3395bc8 100644 --- a/src/renderer/components/ChatPane.tsx +++ b/src/renderer/components/ChatPane.tsx @@ -30,6 +30,7 @@ import { type PatternDefinition, type ReasoningEffort } from '@shared/domain/pat import { isScratchpadProject, type ProjectRecord } from '@shared/domain/project'; import { resolveSessionToolingSelection, type SessionBranchOriginAction, type SessionRecord } from '@shared/domain/session'; import { + countApprovedToolsInGroups, groupApprovalToolsByProvider, listApprovalToolDefinitions, type RuntimeToolDefinition, @@ -162,17 +163,7 @@ export function ChatPane({ ); const effectiveAutoApprovedCount = useMemo(() => { const groups = groupApprovalToolsByProvider(approvalTools, toolingSettings); - const counted = new Set(); - for (const group of groups) { - if (group.serverApprovalKey && effectiveAutoApproved.has(group.serverApprovalKey)) { - for (const tool of group.tools) counted.add(tool.id); - } else { - for (const tool of group.tools) { - if (effectiveAutoApproved.has(tool.id)) counted.add(tool.id); - } - } - } - return counted.size; + return countApprovedToolsInGroups(groups, effectiveAutoApproved); }, [approvalTools, effectiveAutoApproved, toolingSettings]); const isProbingMcp = (mcpProbingServerIds?.length ?? 0) > 0; const hasApprovalContent = approvalTools.length > 0 || isProbingMcp; diff --git a/src/shared/domain/tooling.ts b/src/shared/domain/tooling.ts index a31fa92..91f006a 100644 --- a/src/shared/domain/tooling.ts +++ b/src/shared/domain/tooling.ts @@ -395,6 +395,34 @@ function resolveApprovalToolGroupKey( return { id: 'other', label: 'Other', kind: 'mixed' }; } +/** + * Count how many tools are effectively auto-approved across all groups. + * + * This must use per-group counting (not unique-tool-ID deduplication) to stay + * consistent with the total-item formula used in InlineApprovalPill, which + * counts each group occurrence independently. Without this, shared tool names + * across MCP servers produce a numerator smaller than the denominator even when + * everything is approved. + */ +export function countApprovedToolsInGroups( + groups: ReadonlyArray, + approved: ReadonlySet, +): number { + let count = 0; + for (const group of groups) { + if (group.serverApprovalKey && approved.has(group.serverApprovalKey)) { + // Server-level approval covers all tools. Servers with 0 tools still + // count as 1 (matching the totalItemCount formula). + count += Math.max(group.tools.length, 1); + } else { + for (const tool of group.tools) { + if (approved.has(tool.id)) count += 1; + } + } + } + return count; +} + export function validateMcpServerDefinition(server: McpServerDefinition): string | undefined { if (!server.name.trim()) { return 'MCP server name is required.'; diff --git a/tests/shared/tooling.test.ts b/tests/shared/tooling.test.ts index d55924c..a81e1b2 100644 --- a/tests/shared/tooling.test.ts +++ b/tests/shared/tooling.test.ts @@ -2,6 +2,7 @@ import { describe, expect, test } from 'bun:test'; import { buildMcpServerApprovalKey, + countApprovedToolsInGroups, groupApprovalToolsByProvider, listApprovalToolDefinitions, listApprovalToolNames, @@ -658,3 +659,101 @@ describe('probed tools', () => { expect(names).toContain('mcp_server:Probed Keys'); }); }); + +describe('countApprovedToolsInGroups', () => { + const TS = '2026-03-28T00:00:00.000Z'; + + function makeTooling( + mcpServers: McpServerDefinition[] = [], + lspProfiles: LspProfileDefinition[] = [], + ): WorkspaceToolingSettings { + return { mcpServers, lspProfiles }; + } + + function makeMcpServer(id: string, name: string, tools: string[]): McpServerDefinition { + return { id, name, transport: 'local', command: 'node', args: [], tools, createdAt: TS, updatedAt: TS }; + } + + test('counts all tools approved when all server keys are set', () => { + const tooling = makeTooling([ + makeMcpServer('git', 'Git MCP', ['git.status', 'git.diff']), + makeMcpServer('fs', 'Filesystem', ['fs.read']), + ]); + const tools = listApprovalToolDefinitions(tooling); + const groups = groupApprovalToolsByProvider(tools, tooling); + const mcpGroups = groups.filter((g) => g.kind === 'mcp'); + const approved = new Set(['mcp_server:Git MCP', 'mcp_server:Filesystem']); + + const mcpTotal = mcpGroups.reduce( + (sum, g) => sum + Math.max(g.tools.length, g.serverApprovalKey ? 1 : 0), 0, + ); + const count = countApprovedToolsInGroups(mcpGroups, approved); + expect(count).toBe(mcpTotal); + }); + + test('counts shared tool names per group, not as unique IDs', () => { + const tooling = makeTooling([ + makeMcpServer('git-a', 'Git A', ['git.status', 'git.diff']), + makeMcpServer('git-b', 'Git B', ['git.status', 'git.diff']), + ]); + const tools = listApprovalToolDefinitions(tooling); + const groups = groupApprovalToolsByProvider(tools, tooling); + + const mcpGroups = groups.filter((g) => g.kind === 'mcp'); + expect(mcpGroups.length).toBe(2); + // Both servers share the same tool names — MCP total should be 4 (2 per group) + const mcpTotal = mcpGroups.reduce( + (sum, g) => sum + Math.max(g.tools.length, g.serverApprovalKey ? 1 : 0), 0, + ); + expect(mcpTotal).toBe(4); + + // Approve all via server keys — count must match total + const approved = new Set(['mcp_server:Git A', 'mcp_server:Git B']); + const count = countApprovedToolsInGroups(mcpGroups, approved); + expect(count).toBe(mcpTotal); + }); + + test('counts individual tool approvals per group when no server key', () => { + const tooling = makeTooling([ + makeMcpServer('git-a', 'Git A', ['git.status']), + makeMcpServer('git-b', 'Git B', ['git.status']), + ]); + const tools = listApprovalToolDefinitions(tooling); + const groups = groupApprovalToolsByProvider(tools, tooling); + + // Approve only the individual tool ID (shared across both groups) + const approved = new Set(['git.status']); + const mcpGroups = groups.filter((g) => g.kind === 'mcp'); + expect(mcpGroups.length).toBe(2); + + // Each group has the same tool → count should be 2 (once per group) + const mcpCount = countApprovedToolsInGroups(mcpGroups, approved); + expect(mcpCount).toBe(2); + }); + + test('counts empty server with approved key as 1', () => { + const tooling = makeTooling([makeMcpServer('empty', 'Empty Server', [])]); + const tools = listApprovalToolDefinitions(tooling); + const groups = groupApprovalToolsByProvider(tools, tooling); + const approved = new Set(['mcp_server:Empty Server']); + + const mcpGroups = groups.filter((g) => g.kind === 'mcp'); + expect(mcpGroups.length).toBe(1); + expect(mcpGroups[0].tools.length).toBe(0); + + const count = countApprovedToolsInGroups(mcpGroups, approved); + expect(count).toBe(1); + }); + + test('returns 0 when nothing is approved', () => { + const tooling = makeTooling([ + makeMcpServer('git', 'Git MCP', ['git.status']), + ]); + const tools = listApprovalToolDefinitions(tooling); + const groups = groupApprovalToolsByProvider(tools, tooling); + const approved = new Set(); + + const count = countApprovedToolsInGroups(groups, approved); + expect(count).toBe(0); + }); +});