Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions integration-tests/extensions-reload.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ describe('extension reloading', () => {
return (
output.includes(
'test-server (from test-extension) - Ready (1 tool)',
) && output.includes('- hello')
) && output.includes('- mcp_test-server_hello')
);
},
30000, // 30s timeout
Expand Down Expand Up @@ -148,7 +148,7 @@ describe('extension reloading', () => {
return (
output.includes(
'test-server (from test-extension) - Ready (1 tool)',
) && output.includes('- goodbye')
) && output.includes('- mcp_test-server_goodbye')
);
},
30000,
Expand Down
73 changes: 42 additions & 31 deletions packages/cli/src/config/policy-engine.integration.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -89,45 +89,45 @@ describe('Policy Engine Integration Tests', () => {
// Tools from allowed server should be allowed
// Tools from allowed server should be allowed
expect(
(await engine.check({ name: 'allowed-server__tool1' }, undefined))
(await engine.check({ name: 'mcp_allowed-server_tool1' }, undefined))
.decision,
).toBe(PolicyDecision.ALLOW);
expect(
(
await engine.check(
{ name: 'allowed-server__another_tool' },
{ name: 'mcp_allowed-server_another_tool' },
undefined,
)
).decision,
).toBe(PolicyDecision.ALLOW);

// Tools from trusted server should be allowed
expect(
(await engine.check({ name: 'trusted-server__tool1' }, undefined))
(await engine.check({ name: 'mcp_trusted-server_tool1' }, undefined))
.decision,
).toBe(PolicyDecision.ALLOW);
expect(
(
await engine.check(
{ name: 'trusted-server__special_tool' },
{ name: 'mcp_trusted-server_special_tool' },
undefined,
)
).decision,
).toBe(PolicyDecision.ALLOW);

// Tools from blocked server should be denied
expect(
(await engine.check({ name: 'blocked-server__tool1' }, undefined))
(await engine.check({ name: 'mcp_blocked-server_tool1' }, undefined))
.decision,
).toBe(PolicyDecision.DENY);
expect(
(await engine.check({ name: 'blocked-server__any_tool' }, undefined))
(await engine.check({ name: 'mcp_blocked-server_any_tool' }, undefined))
.decision,
).toBe(PolicyDecision.DENY);

// Tools from unknown servers should use default
expect(
(await engine.check({ name: 'unknown-server__tool' }, undefined))
(await engine.check({ name: 'mcp_unknown-server_tool' }, undefined))
.decision,
).toBe(PolicyDecision.ASK_USER);
});
Expand All @@ -147,12 +147,16 @@ describe('Policy Engine Integration Tests', () => {

// ANY tool with a server name should be allowed
expect(
(await engine.check({ name: 'mcp-server__tool' }, 'mcp-server'))
(await engine.check({ name: 'mcp_mcp-server_tool' }, 'mcp-server'))
.decision,
).toBe(PolicyDecision.ALLOW);
expect(
(await engine.check({ name: 'another-server__tool' }, 'another-server'))
.decision,
(
await engine.check(
{ name: 'mcp_another-server_tool' },
'another-server',
)
).decision,
).toBe(PolicyDecision.ALLOW);

// Built-in tools should NOT be allowed by the MCP wildcard
Expand All @@ -167,7 +171,7 @@ describe('Policy Engine Integration Tests', () => {
allowed: ['my-server'],
},
tools: {
exclude: ['my-server__dangerous-tool'],
exclude: ['mcp_my-server_dangerous-tool'],
},
};

Expand All @@ -180,20 +184,24 @@ describe('Policy Engine Integration Tests', () => {
// MCP server allowed (priority 4.1) provides general allow for server
// MCP server allowed (priority 4.1) provides general allow for server
expect(
(await engine.check({ name: 'my-server__safe-tool' }, undefined))
(await engine.check({ name: 'mcp_my-server_safe-tool' }, undefined))
.decision,
).toBe(PolicyDecision.ALLOW);
// But specific tool exclude (priority 4.4) wins over server allow
expect(
(await engine.check({ name: 'my-server__dangerous-tool' }, undefined))
.decision,
(
await engine.check(
{ name: 'mcp_my-server_dangerous-tool' },
undefined,
)
).decision,
).toBe(PolicyDecision.DENY);
});

it('should handle complex mixed configurations', async () => {
const settings: Settings = {
tools: {
allowed: ['custom-tool', 'my-server__special-tool'],
allowed: ['custom-tool', 'mcp_my-server_special-tool'],
exclude: ['glob', 'dangerous-tool'],
},
mcp: {
Expand Down Expand Up @@ -238,21 +246,21 @@ describe('Policy Engine Integration Tests', () => {
(await engine.check({ name: 'custom-tool' }, undefined)).decision,
).toBe(PolicyDecision.ALLOW);
expect(
(await engine.check({ name: 'my-server__special-tool' }, undefined))
(await engine.check({ name: 'mcp_my-server_special-tool' }, undefined))
.decision,
).toBe(PolicyDecision.ALLOW);

// MCP server tools
expect(
(await engine.check({ name: 'allowed-server__tool' }, undefined))
(await engine.check({ name: 'mcp_allowed-server_tool' }, undefined))
.decision,
).toBe(PolicyDecision.ALLOW);
expect(
(await engine.check({ name: 'trusted-server__tool' }, undefined))
(await engine.check({ name: 'mcp_trusted-server_tool' }, undefined))
.decision,
).toBe(PolicyDecision.ALLOW);
expect(
(await engine.check({ name: 'blocked-server__tool' }, undefined))
(await engine.check({ name: 'mcp_blocked-server_tool' }, undefined))
.decision,
).toBe(PolicyDecision.DENY);

Expand Down Expand Up @@ -479,7 +487,7 @@ describe('Policy Engine Integration Tests', () => {
expect(blockedToolRule?.priority).toBe(4.4); // Command line exclude

const blockedServerRule = rules.find(
(r) => r.toolName === 'blocked-server__*',
(r) => r.toolName === 'mcp_blocked-server_*',
);
expect(blockedServerRule?.priority).toBe(4.9); // MCP server exclude

Expand All @@ -489,11 +497,13 @@ describe('Policy Engine Integration Tests', () => {
expect(specificToolRule?.priority).toBe(4.3); // Command line allow

const trustedServerRule = rules.find(
(r) => r.toolName === 'trusted-server__*',
(r) => r.toolName === 'mcp_trusted-server_*',
);
expect(trustedServerRule?.priority).toBe(4.2); // MCP trusted server

const mcpServerRule = rules.find((r) => r.toolName === 'mcp-server__*');
const mcpServerRule = rules.find(
(r) => r.toolName === 'mcp_mcp-server_*',
);
expect(mcpServerRule?.priority).toBe(4.1); // MCP allowed server

const readOnlyToolRule = rules.find((r) => r.toolName === 'glob');
Expand All @@ -505,18 +515,19 @@ describe('Policy Engine Integration Tests', () => {
(await engine.check({ name: 'blocked-tool' }, undefined)).decision,
).toBe(PolicyDecision.DENY);
expect(
(await engine.check({ name: 'blocked-server__any' }, undefined))
(await engine.check({ name: 'mcp_blocked-server_any' }, undefined))
.decision,
).toBe(PolicyDecision.DENY);
expect(
(await engine.check({ name: 'specific-tool' }, undefined)).decision,
).toBe(PolicyDecision.ALLOW);
expect(
(await engine.check({ name: 'trusted-server__any' }, undefined))
(await engine.check({ name: 'mcp_trusted-server_any' }, undefined))
.decision,
).toBe(PolicyDecision.ALLOW);
expect(
(await engine.check({ name: 'mcp-server__any' }, undefined)).decision,
(await engine.check({ name: 'mcp_mcp-server_any' }, undefined))
.decision,
).toBe(PolicyDecision.ALLOW);
expect((await engine.check({ name: 'glob' }, undefined)).decision).toBe(
PolicyDecision.ALLOW,
Expand Down Expand Up @@ -545,7 +556,7 @@ describe('Policy Engine Integration Tests', () => {

// Exclusion (195) should win over trust (90)
expect(
(await engine.check({ name: 'conflicted-server__tool' }, undefined))
(await engine.check({ name: 'mcp_conflicted-server_tool' }, undefined))
.decision,
).toBe(PolicyDecision.DENY);
});
Expand All @@ -556,7 +567,7 @@ describe('Policy Engine Integration Tests', () => {
excluded: ['my-server'], // Priority 195 - DENY
},
tools: {
allowed: ['my-server__special-tool'], // Priority 100 - ALLOW
allowed: ['mcp_my-server_special-tool'], // Priority 100 - ALLOW
},
};

Expand All @@ -569,11 +580,11 @@ describe('Policy Engine Integration Tests', () => {
// Server exclusion (195) wins over specific tool allow (100)
// This might be counterintuitive but follows the priority system
expect(
(await engine.check({ name: 'my-server__special-tool' }, undefined))
(await engine.check({ name: 'mcp_my-server_special-tool' }, undefined))
.decision,
).toBe(PolicyDecision.DENY);
expect(
(await engine.check({ name: 'my-server__other-tool' }, undefined))
(await engine.check({ name: 'mcp_my-server_other-tool' }, undefined))
.decision,
).toBe(PolicyDecision.DENY);
});
Expand Down Expand Up @@ -643,13 +654,13 @@ describe('Policy Engine Integration Tests', () => {
const tool3Rule = rules.find((r) => r.toolName === 'tool3');
expect(tool3Rule?.priority).toBe(4.4); // Excluded tools (user tier)

const server2Rule = rules.find((r) => r.toolName === 'server2__*');
const server2Rule = rules.find((r) => r.toolName === 'mcp_server2_*');
expect(server2Rule?.priority).toBe(4.9); // Excluded servers (user tier)

const tool1Rule = rules.find((r) => r.toolName === 'tool1');
expect(tool1Rule?.priority).toBe(4.3); // Allowed tools (user tier)

const server1Rule = rules.find((r) => r.toolName === 'server1__*');
const server1Rule = rules.find((r) => r.toolName === 'mcp_server1_*');
expect(server1Rule?.priority).toBe(4.1); // Allowed servers (user tier)

const globRule = rules.find((r) => r.toolName === 'glob');
Expand Down
7 changes: 2 additions & 5 deletions packages/core/src/agents/local-executor.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,7 @@ import { debugLogger } from '../utils/debugLogger.js';
import { LocalAgentExecutor, type ActivityCallback } from './local-executor.js';
import { makeFakeConfig } from '../test-utils/config.js';
import { ToolRegistry } from '../tools/tool-registry.js';
import {
DiscoveredMCPTool,
MCP_QUALIFIED_NAME_SEPARATOR,
} from '../tools/mcp-tool.js';
import { DiscoveredMCPTool } from '../tools/mcp-tool.js';
import { LSTool } from '../tools/ls.js';
import { LS_TOOL_NAME, READ_FILE_TOOL_NAME } from '../tools/tool-names.js';
import {
Expand Down Expand Up @@ -504,7 +501,7 @@ describe('LocalAgentExecutor', () => {
it('should automatically qualify MCP tools in agent definitions', async () => {
const serverName = 'mcp-server';
const toolName = 'mcp-tool';
const qualifiedName = `${serverName}${MCP_QUALIFIED_NAME_SEPARATOR}${toolName}`;
const qualifiedName = `mcp_${serverName}_${toolName}`;

const mockMcpTool = {
tool: vi.fn(),
Expand Down
6 changes: 3 additions & 3 deletions packages/core/src/core/__snapshots__/prompts.test.ts.snap
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ The following tools are available in Plan Mode:
<tool>\`exit_plan_mode\`</tool>
<tool>\`write_file\`</tool>
<tool>\`replace\`</tool>
<tool>\`read_data\` (readonly-server)</tool>
<tool>\`mcp_readonly-server_read_data\` (readonly-server)</tool>
</available_tools>

## Rules
Expand Down Expand Up @@ -271,7 +271,7 @@ The following tools are available in Plan Mode:
<tool>\`exit_plan_mode\`</tool>
<tool>\`write_file\`</tool>
<tool>\`replace\`</tool>
<tool>\`read_data\` (readonly-server)</tool>
<tool>\`mcp_readonly-server_read_data\` (readonly-server)</tool>
</available_tools>

## Rules
Expand Down Expand Up @@ -558,7 +558,7 @@ The following tools are available in Plan Mode:
<tool>\`exit_plan_mode\`</tool>
<tool>\`write_file\`</tool>
<tool>\`replace\`</tool>
<tool>\`read_data\` (readonly-server)</tool>
<tool>\`mcp_readonly-server_read_data\` (readonly-server)</tool>
</available_tools>

## Rules
Expand Down
19 changes: 9 additions & 10 deletions packages/core/src/core/loggingContentGenerator.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -709,7 +709,7 @@ describe('estimateContextBreakdown', () => {
{
functionDeclarations: [
{
name: 'myserver__search',
name: 'mcp_myserver_search',
description: 'Search via MCP',
parameters: {},
},
Expand Down Expand Up @@ -747,8 +747,7 @@ describe('estimateContextBreakdown', () => {
expect(builtinOnly.mcp_servers).toBe(0);
});

it('should not classify tools with __ in the middle of a segment as MCP', () => {
// "__" at start or end (not a valid server__tool pattern) should not be MCP
it('should not classify tools without mcp_ prefix as MCP', () => {
const config = {
tools: [
{
Expand Down Expand Up @@ -842,7 +841,7 @@ describe('estimateContextBreakdown', () => {
functionDeclarations: [
{ name: 'read_file', description: 'Read', parameters: {} },
{
name: 'myserver__search',
name: 'mcp_myserver_search',
description: 'MCP search',
parameters: {},
},
Expand All @@ -858,7 +857,7 @@ describe('estimateContextBreakdown', () => {
expect(result.history).toBeGreaterThan(0);
// tool_calls should only contain non-MCP tools
expect(result.tool_calls['read_file']).toBeGreaterThan(0);
expect(result.tool_calls['myserver__search']).toBeUndefined();
expect(result.tool_calls['mcp_myserver_search']).toBeUndefined();
// MCP tokens are only in mcp_servers
expect(result.mcp_servers).toBeGreaterThan(0);
});
Expand All @@ -870,7 +869,7 @@ describe('estimateContextBreakdown', () => {
parts: [
{
functionCall: {
name: 'myserver__search',
name: 'mcp_myserver_search',
args: { query: 'test' },
},
},
Expand All @@ -881,7 +880,7 @@ describe('estimateContextBreakdown', () => {
parts: [
{
functionResponse: {
name: 'myserver__search',
name: 'mcp_myserver_search',
response: { results: [] },
},
},
Expand All @@ -890,7 +889,7 @@ describe('estimateContextBreakdown', () => {
];
const result = estimateContextBreakdown(contents);
// MCP tool calls should NOT appear in tool_calls
expect(result.tool_calls['myserver__search']).toBeUndefined();
expect(result.tool_calls['mcp_myserver_search']).toBeUndefined();
// MCP call tokens should only be counted in mcp_servers
expect(result.mcp_servers).toBeGreaterThan(0);
});
Expand All @@ -908,7 +907,7 @@ describe('estimateContextBreakdown', () => {
},
{
functionCall: {
name: 'myserver__search',
name: 'mcp_myserver_search',
args: { q: 'hello' },
},
},
Expand All @@ -919,7 +918,7 @@ describe('estimateContextBreakdown', () => {
// Non-MCP tools should be in tool_calls
expect(result.tool_calls['read_file']).toBeGreaterThan(0);
// MCP tools should NOT be in tool_calls
expect(result.tool_calls['myserver__search']).toBeUndefined();
expect(result.tool_calls['mcp_myserver_search']).toBeUndefined();
// MCP tool calls should only be in mcp_servers
expect(result.mcp_servers).toBeGreaterThan(0);
});
Expand Down
Loading
Loading