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
102 changes: 82 additions & 20 deletions gitnexus/src/mcp/local/local-backend.ts
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,61 @@ function resolveAliasString(canonical: unknown, legacy: unknown): string | undef
return undefined;
}

interface StringAliasDefinition {
canonical: string;
aliases: readonly string[];
}

const TOOL_STRING_ALIASES: Readonly<Record<string, readonly StringAliasDefinition[]>> = {
impact: [{ canonical: 'target', aliases: ['name', 'symbol'] }],
context: [{ canonical: 'file_path', aliases: ['file'] }],
};

function normalizeToolParams(
method: string,
params: unknown,
): { params: Record<string, unknown> } | { error: string } {
const input = params && typeof params === 'object' ? (params as Record<string, unknown>) : {};
const definitions = TOOL_STRING_ALIASES[method];
if (!definitions) return { params: input };

const normalized = { ...input };
for (const { canonical, aliases } of definitions) {
const keys = [canonical, ...aliases];
const supplied: Array<{ key: string; value: string }> = [];
for (const key of keys) {
if (!Object.prototype.hasOwnProperty.call(input, key)) continue;
const value = input[key];
// Internal CLI callers materialize omitted optional flags as undefined.
if (value === undefined) continue;
if (typeof value !== 'string' || !value.trim()) {
return { error: `MCP parameter ${method}.${key} must be a non-empty string.` };
}
supplied.push({ key, value: value.trim() });
Comment thread
magyargergo marked this conversation as resolved.
}
const distinctValues = new Set(supplied.map(({ value }) => value));
if (distinctValues.size > 1) {
return {
error: `Conflicting MCP parameters for ${method}.${canonical}: ${supplied
.map(({ key }) => key)
.join(', ')} must agree.`,
};
}

for (const alias of aliases) delete normalized[alias];
if (supplied.length > 0) normalized[canonical] = supplied[0].value;
}

if (
method === 'impact' &&
typeof normalized.target !== 'string' &&
(typeof normalized.target_uid !== 'string' || !normalized.target_uid.trim())
) {
return { error: 'MCP impact requires target, name, symbol, or target_uid.' };
}
return { params: normalized };
}

// AI context generation is CLI-only (gitnexus analyze)
// import { generateAIContextFiles } from '../../cli/ai-context.js';

Expand Down Expand Up @@ -1661,7 +1716,9 @@ export class LocalBackend {
return this.handleGroupTool(method, params || {});
}

const p = params && typeof params === 'object' ? (params as Record<string, unknown>) : {};
const normalized = normalizeToolParams(method, params);
if ('error' in normalized) return { error: normalized.error };
const p = normalized.params;

// #2175: Claude Code drops a tool-call argument named exactly "query", so the
// query/cypher tools advertise "search_query"/"statement" while still accepting the
Expand All @@ -1682,47 +1739,52 @@ export class LocalBackend {

// Resolve repo from optional param (re-reads registry on miss). An optional
// `branch` param scopes the resolved handle to that branch's index (#2106).
const repoParams = params as { repo?: string; branch?: string } | undefined;
const repo = await this.resolveRepo(repoParams?.repo, repoParams?.branch);
const repo = await this.resolveRepo(
p.repo as string | undefined,
p.branch as string | undefined,
);

switch (method) {
case 'query':
return this.query(repo, params);
return this.query(repo, p);
case 'cypher': {
const raw = await this.cypher(repo, params);
const raw = await this.cypher(repo, p);
return this.formatCypherAsMarkdown(raw);
}
case 'context':
return this.context(repo, params);
return this.context(repo, p);
case 'explain':
return this.explain(repo, params);
return this.explain(repo, p);
case 'pdg_query':
return this.pdgQuery(repo, params);
return this.pdgQuery(repo, p);
case 'impact':
return this.impact(repo, params);
return this.impact(repo, p as unknown as ImpactParams);
case 'detect_changes':
return this.detectChanges(repo, params);
return this.detectChanges(repo, p);
case 'check':
return this.check(repo, params);
return this.check(repo, p);
case 'rename':
return this.rename(repo, params);
return this.rename(repo, p as unknown as Parameters<LocalBackend['rename']>[1]);
// Legacy aliases for backwards compatibility
case 'search':
return this.query(repo, params);
return this.query(repo, p);
case 'explore':
return this.context(repo, { name: params?.name, ...params });
return this.context(repo, {
name: typeof p.name === 'string' ? p.name : undefined,
...p,
});
case 'overview':
return this.overview(repo, params);
return this.overview(repo, p);
case 'route_map':
return this.routeMap(repo, params);
return this.routeMap(repo, p);
case 'shape_check':
return this.shapeCheck(repo, params);
return this.shapeCheck(repo, p);
case 'tool_map':
return this.toolMap(repo, params);
return this.toolMap(repo, p);
case 'api_impact':
return this.apiImpact(repo, params);
return this.apiImpact(repo, p);
case 'trace':
return this.trace(repo, params);
return this.trace(repo, p);
default:
throw new Error(`Unknown tool: ${method}`);
}
Expand Down
14 changes: 13 additions & 1 deletion gitnexus/src/mcp/tools.ts
Original file line number Diff line number Diff line change
Expand Up @@ -297,6 +297,10 @@ SERVICE: optional monorepo path prefix (case-sensitive path segments). When "rep
description: 'Direct symbol UID from prior tool results (zero-ambiguity lookup)',
},
file_path: { type: 'string', description: 'File path to disambiguate common names' },
file: {
type: 'string',
description: 'Compatibility alias for file_path; values must agree when both are present',
},
kind: {
type: 'string',
description:
Expand Down Expand Up @@ -461,6 +465,14 @@ SERVICE: optional monorepo path prefix (case-sensitive path segments). When "rep
type: 'object',
properties: {
target: { type: 'string', description: 'Name of function, class, or file to analyze' },
name: {
type: 'string',
description: 'Compatibility alias for target; all supplied target aliases must agree',
},
symbol: {
type: 'string',
description: 'Compatibility alias for target; all supplied target aliases must agree',
},
target_uid: {
type: 'string',
description:
Expand Down Expand Up @@ -578,7 +590,7 @@ SERVICE: optional monorepo path prefix (case-sensitive path segments). When "rep
maximum: 3600000,
},
},
required: ['target', 'direction'],
required: ['direction'],
Comment thread
magyargergo marked this conversation as resolved.
},
},
{
Expand Down
25 changes: 25 additions & 0 deletions gitnexus/test/integration/local-backend-calltool.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,31 @@ withTestLbugDB(
expect(depNames).toContain('login');
});

it.each(['name', 'symbol'] as const)(
'impact tool resolves the %s compatibility alias against a real index',
async (alias) => {
const result = await backend.callTool('impact', {
[alias]: 'validate',
direction: 'upstream',
});
expect(result).not.toHaveProperty('error');
expect(result.target?.name).toBe('validate');
const directDeps = result.byDepth[1] || result.byDepth['1'] || [];
expect(directDeps.map((d: any) => d.name)).toContain('login');
},
);

it('context tool resolves the file compatibility alias against a real index', async () => {
const result = await backend.callTool('context', {
name: 'authenticate',
file: 'src/base.ts',
});
expect(result).not.toHaveProperty('error');
expect(result.status).toBe('found');
expect(result.symbol?.name).toBe('authenticate');
expect(result.symbol?.filePath).toBe('src/base.ts');
});

it('query tool returns results for keyword search', async () => {
const result = await backend.callTool('query', { query: 'login' });
expect(result).not.toHaveProperty('error');
Expand Down
132 changes: 132 additions & 0 deletions gitnexus/test/unit/calltool-dispatch.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -327,6 +327,138 @@ describe('LocalBackend.callTool', () => {
);
});

it.each(['name', 'symbol'] as const)(
'normalizes impact.%s to target once before local dispatch',
async (alias) => {
const impactSpy = vi
.spyOn(backend as any, 'impact')
.mockResolvedValue({ status: 'normalized' });

const result = await backend.callTool('impact', {
[alias]: ' validate ',
direction: 'upstream',
});

expect(result).toEqual({ status: 'normalized' });
const dispatched = impactSpy.mock.calls[0][1] as Record<string, unknown>;
expect(dispatched.target).toBe('validate');
expect(dispatched).not.toHaveProperty('name');
expect(dispatched).not.toHaveProperty('symbol');
},
);

it('normalizes context.file to file_path once before local dispatch', async () => {
const contextSpy = vi
.spyOn(backend as any, 'context')
.mockResolvedValue({ status: 'normalized' });

const result = await backend.callTool('context', {
name: 'validate',
file: ' src/auth.ts ',
});

expect(result).toEqual({ status: 'normalized' });
const dispatched = contextSpy.mock.calls[0][1] as Record<string, unknown>;
expect(dispatched.file_path).toBe('src/auth.ts');
expect(dispatched).not.toHaveProperty('file');
});

it('treats undefined optional alias keys from CLI callers as absent', async () => {
const contextSpy = vi
.spyOn(backend as any, 'context')
.mockResolvedValue({ status: 'normalized' });

const result = await backend.callTool('context', {
name: 'validate',
file_path: undefined,
file: undefined,
});

expect(result).toEqual({ status: 'normalized' });
expect(contextSpy.mock.calls[0][1]).toMatchObject({ name: 'validate' });
});

it('allows agreeing canonical and alias values after trimming', async () => {
const impactSpy = vi
.spyOn(backend as any, 'impact')
.mockResolvedValue({ status: 'normalized' });

await backend.callTool('impact', {
target: 'validate',
name: ' validate ',
symbol: 'validate',
direction: 'upstream',
});

expect(impactSpy.mock.calls[0][1]).toMatchObject({ target: 'validate' });
});

it.each([
['impact', { target: 'validate', name: 'login', direction: 'upstream' }],
['impact', { name: 'validate', symbol: 'login', direction: 'upstream' }],
['context', { name: 'validate', file_path: 'src/auth.ts', file: 'src/login.ts' }],
])('rejects conflicting %s aliases before repository resolution', async (method, params) => {
const resolveSpy = vi.spyOn(backend, 'resolveRepo');

const result = await backend.callTool(method, params);

expect(result.error).toMatch(/conflicting mcp parameters/i);
expect(resolveSpy).not.toHaveBeenCalled();
});

it.each([
['impact', { target: '', direction: 'upstream' }],
['impact', { name: 42, direction: 'upstream' }],
['context', { name: 'validate', file: ' ' }],
['context', { name: 'validate', file: null }],
])('rejects invalid %s aliases before repository resolution', async (method, params) => {
const resolveSpy = vi.spyOn(backend, 'resolveRepo');

const result = await backend.callTool(method, params);

expect(result.error).toMatch(/non-empty string/i);
expect(resolveSpy).not.toHaveBeenCalled();
});

it('rejects a missing impact target before repository resolution', async () => {
const resolveSpy = vi.spyOn(backend, 'resolveRepo');

const result = await backend.callTool('impact', { direction: 'upstream' });

expect(result.error).toMatch(/requires target, name, symbol, or target_uid/i);
expect(resolveSpy).not.toHaveBeenCalled();
});

it('preserves target_uid-only impact dispatch', async () => {
const impactSpy = vi
.spyOn(backend as any, 'impact')
.mockResolvedValue({ status: 'normalized' });

await backend.callTool('impact', {
target_uid: 'Function:src/auth.ts:validate',
direction: 'upstream',
});

expect(impactSpy.mock.calls[0][1]).toMatchObject({
target_uid: 'Function:src/auth.ts:validate',
});
});

it('normalizes impact aliases before @group forwarding', async () => {
resolveAtMemberMock.mockResolvedValue({ ok: true, repoPath: '/tmp/test-project' });
const groupImpactSpy = vi
.spyOn(backend.getGroupService(), 'groupImpact')
.mockResolvedValue({ status: 'normalized' } as any);

await backend.callTool('impact', {
symbol: 'validate',
direction: 'upstream',
repo: '@grp',
});

expect(groupImpactSpy.mock.calls[0][0]).toMatchObject({ target: 'validate' });
});

it('dispatches query tool', async () => {
(executeParameterized as any).mockResolvedValue([]);
const result = await backend.callTool('query', { query: 'auth' });
Expand Down
15 changes: 13 additions & 2 deletions gitnexus/test/unit/tools.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,12 @@ describe('GITNEXUS_TOOLS', () => {
expect(contextTool.inputSchema.required).toEqual([]);
});

it('context tool advertises file as a compatibility alias for file_path', () => {
const contextTool = GITNEXUS_TOOLS.find((t) => t.name === 'context')!;
expect(contextTool.inputSchema.properties.file_path).toBeDefined();
expect(contextTool.inputSchema.properties.file).toMatchObject({ type: 'string' });
});

it('api_impact tool expresses the route-or-file requirement via anyOf (#2308)', () => {
const apiImpactTool = GITNEXUS_TOOLS.find((t) => t.name === 'api_impact')!;
expect(apiImpactTool.inputSchema.anyOf).toEqual([
Expand All @@ -138,10 +144,15 @@ describe('GITNEXUS_TOOLS', () => {
expect(apiImpactTool.inputSchema.required).toEqual([]);
});

it('impact tool requires target and direction', () => {
it('impact tool requires direction and advertises target, name, or symbol without combinators', () => {
const impactTool = GITNEXUS_TOOLS.find((t) => t.name === 'impact')!;
expect(impactTool.inputSchema.required).toContain('target');
expect(impactTool.inputSchema.required).toContain('direction');
expect(impactTool.inputSchema.required).not.toContain('target');
expect(impactTool.inputSchema.properties.name).toMatchObject({ type: 'string' });
expect(impactTool.inputSchema.properties.symbol).toMatchObject({ type: 'string' });
expect(impactTool.inputSchema).not.toHaveProperty('anyOf');
expect(impactTool.inputSchema).not.toHaveProperty('oneOf');
expect(impactTool.inputSchema).not.toHaveProperty('allOf');
});

it('impact tool advertises the PDG-only `line` statement anchor (integer, min 0, not required)', () => {
Expand Down
Loading