mirror of
https://github.com/abhigyanpatwari/GitNexus.git
synced 2026-08-28 05:25:25 +00:00
fix: require repo in multi-repo MCP tool schemas (#2717)
* fix: require repo in multi-repo MCP schemas * style(mcp): fix server test formatting * chore(autofix): apply prettier + eslint fixes via /autofix command * test(mcp): cover repository schema policy --------- Co-authored-by: Gergő Magyar <gergomagyar@icloud.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
This commit is contained in:
parent
ee9987fdc5
commit
7be6d29ca0
4 changed files with 115 additions and 18 deletions
|
|
@ -179,6 +179,11 @@ export class McpRepositoryPolicy {
|
|||
});
|
||||
}
|
||||
|
||||
async requiresExplicitRepo(backend: LocalBackend): Promise<boolean> {
|
||||
if (this.defaultRepo) return false;
|
||||
return (await this.listAllowedRepos(backend)).length > 1;
|
||||
}
|
||||
|
||||
private async listReposPage(
|
||||
backend: LocalBackend,
|
||||
params: Record<string, unknown> | undefined,
|
||||
|
|
|
|||
|
|
@ -23,7 +23,7 @@ import {
|
|||
ListPromptsRequestSchema,
|
||||
GetPromptRequestSchema,
|
||||
} from '@modelcontextprotocol/sdk/types.js';
|
||||
import { GITNEXUS_TOOLS } from './tools.js';
|
||||
import { GITNEXUS_TOOLS, REPO_SCOPED_TOOLS } from './tools.js';
|
||||
import { installGlobalStdoutSentinel } from './stdio-context.js';
|
||||
import type { LocalBackend } from './local/local-backend.js';
|
||||
import { getResourceDefinitions, getResourceTemplates, readResource } from './resources.js';
|
||||
|
|
@ -185,21 +185,32 @@ export function createMCPServer(
|
|||
}
|
||||
});
|
||||
|
||||
// Handle list tools request
|
||||
server.setRequestHandler(ListToolsRequestSchema, async () => ({
|
||||
tools: GITNEXUS_TOOLS.filter(
|
||||
(tool) =>
|
||||
(!readOnly || MCP_READ_ONLY_TOOLS.has(tool.name)) &&
|
||||
repositoryPolicy.toolAllowed(tool.name),
|
||||
)
|
||||
.map((tool) => toolForReadOnlyMcp(repositoryPolicy.toolForMcp(tool), readOnly))
|
||||
.map((tool) => ({
|
||||
name: tool.name,
|
||||
description: tool.description,
|
||||
inputSchema: tool.inputSchema,
|
||||
annotations: tool.annotations,
|
||||
})),
|
||||
}));
|
||||
// With multiple visible repositories and no process-wide default, make the
|
||||
// routing requirement machine-readable. Agents then supply `repo` before the
|
||||
// call instead of discovering the ambiguity through a failed tool response.
|
||||
server.setRequestHandler(ListToolsRequestSchema, async () => {
|
||||
const requireRepo = await repositoryPolicy.requiresExplicitRepo(backend);
|
||||
return {
|
||||
tools: GITNEXUS_TOOLS.filter(
|
||||
(tool) =>
|
||||
(!readOnly || MCP_READ_ONLY_TOOLS.has(tool.name)) &&
|
||||
repositoryPolicy.toolAllowed(tool.name),
|
||||
)
|
||||
.map((tool) => toolForReadOnlyMcp(repositoryPolicy.toolForMcp(tool), readOnly))
|
||||
.map((tool) => ({
|
||||
name: tool.name,
|
||||
description: tool.description,
|
||||
inputSchema:
|
||||
requireRepo && REPO_SCOPED_TOOLS.has(tool.name)
|
||||
? {
|
||||
...tool.inputSchema,
|
||||
required: [...new Set([...tool.inputSchema.required, 'repo'])],
|
||||
}
|
||||
: tool.inputSchema,
|
||||
annotations: tool.annotations,
|
||||
})),
|
||||
};
|
||||
});
|
||||
|
||||
// Handle tool calls — append next-step hints to guide agent workflow
|
||||
server.setRequestHandler(CallToolRequestSchema, async (request) => {
|
||||
|
|
|
|||
|
|
@ -910,7 +910,7 @@ DESTINATION TRACE (cross-repo): for an "@groupName" trace, OMIT to/to_uid/to_fil
|
|||
* `list_repos` and the `group_*` tools are intentionally excluded — they are
|
||||
* not single-repo, single-branch operations.
|
||||
*/
|
||||
const BRANCH_SCOPED_TOOLS = new Set([
|
||||
export const REPO_SCOPED_TOOLS = new Set([
|
||||
'query',
|
||||
'cypher',
|
||||
'context',
|
||||
|
|
@ -928,7 +928,7 @@ const BRANCH_SCOPED_TOOLS = new Set([
|
|||
]);
|
||||
|
||||
for (const tool of GITNEXUS_TOOLS) {
|
||||
if (!BRANCH_SCOPED_TOOLS.has(tool.name)) continue;
|
||||
if (!REPO_SCOPED_TOOLS.has(tool.name)) continue;
|
||||
if (tool.inputSchema.properties.branch) continue;
|
||||
// Optional — `required` is left unchanged so omitting `branch` keeps today's
|
||||
// workspace-index behavior. Ignored in group mode (repo starts "@").
|
||||
|
|
|
|||
|
|
@ -23,6 +23,7 @@ import {
|
|||
SHUTDOWN_EXIT_CODES,
|
||||
} from '../../src/mcp/server.js';
|
||||
import { GITNEXUS_TOOLS } from '../../src/mcp/tools.js';
|
||||
import { createMcpRepositoryPolicy } from '../../src/mcp/repository-policy.js';
|
||||
|
||||
// ─── Mock backend ──────────────────────────────────────────────────
|
||||
|
||||
|
|
@ -104,6 +105,86 @@ describe('createMCPServer', () => {
|
|||
await server.close();
|
||||
}
|
||||
});
|
||||
it('requires repo in repo-scoped tool schemas when multiple repos are visible', async () => {
|
||||
const backend = createMockBackend({
|
||||
listRepos: vi.fn().mockResolvedValue([
|
||||
{ name: 'alpha', path: '/tmp/alpha' },
|
||||
{ name: 'beta', path: '/tmp/beta' },
|
||||
]),
|
||||
});
|
||||
const server = createMCPServer(backend);
|
||||
const client = new Client({ name: 'multi-repo-client', version: '0.0.0' });
|
||||
const [clientTransport, serverTransport] = InMemoryTransport.createLinkedPair();
|
||||
|
||||
try {
|
||||
await Promise.all([server.connect(serverTransport), client.connect(clientTransport)]);
|
||||
const tools = await client.listTools();
|
||||
const query = tools.tools.find((tool) => tool.name === 'query');
|
||||
const listRepos = tools.tools.find((tool) => tool.name === 'list_repos');
|
||||
|
||||
expect(query?.inputSchema.required).toContain('repo');
|
||||
expect(listRepos?.inputSchema.required).not.toContain('repo');
|
||||
expect(
|
||||
GITNEXUS_TOOLS.find((tool) => tool.name === 'query')?.inputSchema.required,
|
||||
).not.toContain('repo');
|
||||
} finally {
|
||||
await client.close();
|
||||
await server.close();
|
||||
}
|
||||
});
|
||||
|
||||
it('keeps repo optional when a default repo is configured', async () => {
|
||||
const backend = createMockBackend({
|
||||
listRepos: vi.fn().mockResolvedValue([
|
||||
{ name: 'alpha', path: '/tmp/alpha' },
|
||||
{ name: 'beta', path: '/tmp/beta' },
|
||||
]),
|
||||
});
|
||||
const repositoryPolicy = await createMcpRepositoryPolicy(backend, {
|
||||
GITNEXUS_MCP_DEFAULT_REPO: 'alpha',
|
||||
});
|
||||
const server = createMCPServer(backend, { repositoryPolicy });
|
||||
const client = new Client({ name: 'default-repo-client', version: '0.0.0' });
|
||||
const [clientTransport, serverTransport] = InMemoryTransport.createLinkedPair();
|
||||
|
||||
try {
|
||||
await Promise.all([server.connect(serverTransport), client.connect(clientTransport)]);
|
||||
const tools = await client.listTools();
|
||||
const query = tools.tools.find((tool) => tool.name === 'query');
|
||||
|
||||
expect(query?.inputSchema.required).not.toContain('repo');
|
||||
} finally {
|
||||
await client.close();
|
||||
await server.close();
|
||||
}
|
||||
});
|
||||
|
||||
it('requires repo when multiple allowed repositories are visible without a default', async () => {
|
||||
const backend = createMockBackend({
|
||||
listRepos: vi.fn().mockResolvedValue([
|
||||
{ name: 'alpha', path: '/tmp/alpha' },
|
||||
{ name: 'beta', path: '/tmp/beta' },
|
||||
{ name: 'gamma', path: '/tmp/gamma' },
|
||||
]),
|
||||
});
|
||||
const repositoryPolicy = await createMcpRepositoryPolicy(backend, {
|
||||
GITNEXUS_MCP_ALLOWED_REPOS: 'alpha,beta',
|
||||
});
|
||||
const server = createMCPServer(backend, { repositoryPolicy });
|
||||
const client = new Client({ name: 'allowlisted-repos-client', version: '0.0.0' });
|
||||
const [clientTransport, serverTransport] = InMemoryTransport.createLinkedPair();
|
||||
|
||||
try {
|
||||
await Promise.all([server.connect(serverTransport), client.connect(clientTransport)]);
|
||||
const tools = await client.listTools();
|
||||
const query = tools.tools.find((tool) => tool.name === 'query');
|
||||
|
||||
expect(query?.inputSchema.required).toContain('repo');
|
||||
} finally {
|
||||
await client.close();
|
||||
await server.close();
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
// ─── getNextStepHint (tested indirectly via server tool handler) ──────
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue