Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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: 3 additions & 1 deletion .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,9 @@ jobs:
- name: 'Link Checker'
uses: 'lycheeverse/lychee-action@885c65f3dc543b57c898c8099f4e08c8afd178a2' # ratchet: lycheeverse/lychee-action@v2.6.1
with:
args: '--verbose --accept 200,503 ./**/*.md'
# Exclude GEMINI.md because the absolute GitHub URL in CONTRIBUTING.md (which is symlinked)
# causes intermittent 429 Too Many Requests errors from GitHub API rate limits.
args: '--verbose --accept 200,503 --exclude "GEMINI\\.md" ./**/*.md'
fail: true
test_linux:
name: 'Test (Linux) - ${{ matrix.node-version }}, ${{ matrix.shard }}'
Expand Down
4 changes: 3 additions & 1 deletion .github/workflows/links.yml
Original file line number Diff line number Diff line change
Expand Up @@ -22,4 +22,6 @@ jobs:
id: 'lychee'
uses: 'lycheeverse/lychee-action@885c65f3dc543b57c898c8099f4e08c8afd178a2' # ratchet: lycheeverse/lychee-action@v2.6.1
with:
args: '--verbose --no-progress --accept 200,503 ./**/*.md'
# Exclude GEMINI.md because the absolute GitHub URL in CONTRIBUTING.md (which is symlinked)
# causes intermittent 429 Too Many Requests errors from GitHub API rate limits.
args: '--verbose --no-progress --accept 200,503 --exclude "GEMINI\\.md" ./**/*.md'
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