From a75844b6921d7dcfb96c3888e3b1b98b64b6f3c8 Mon Sep 17 00:00:00 2001 From: Eva Date: Tue, 14 Jul 2026 01:54:49 +0700 Subject: [PATCH 1/3] feat(mcp): normalize impact and context aliases --- gitnexus/src/mcp/local/local-backend.ts | 87 ++++++++++++++----- gitnexus/src/mcp/tools.ts | 15 +++- .../local-backend-calltool.test.ts | 25 ++++++ gitnexus/test/unit/calltool-dispatch.test.ts | 79 +++++++++++++++++ gitnexus/test/unit/tools.test.ts | 17 +++- 5 files changed, 200 insertions(+), 23 deletions(-) diff --git a/gitnexus/src/mcp/local/local-backend.ts b/gitnexus/src/mcp/local/local-backend.ts index 0956b40719..2cf60e64be 100644 --- a/gitnexus/src/mcp/local/local-backend.ts +++ b/gitnexus/src/mcp/local/local-backend.ts @@ -142,6 +142,46 @@ function resolveAliasString(canonical: unknown, legacy: unknown): string | undef return undefined; } +interface StringAliasDefinition { + canonical: string; + aliases: readonly string[]; +} + +const TOOL_STRING_ALIASES: Readonly> = { + impact: [{ canonical: 'target', aliases: ['name', 'symbol'] }], + context: [{ canonical: 'file_path', aliases: ['file'] }], +}; + +function normalizeToolParams( + method: string, + params: unknown, +): { params: Record } | { error: string } { + const input = params && typeof params === 'object' ? (params as Record) : {}; + 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 = keys.flatMap((key) => { + const value = input[key]; + return typeof value === 'string' && value.trim() ? [{ key, value: value.trim() }] : []; + }); + 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; + } + return { params: normalized }; +} + // AI context generation is CLI-only (gitnexus analyze) // import { generateAIContextFiles } from '../../cli/ai-context.js'; @@ -1661,7 +1701,9 @@ export class LocalBackend { return this.handleGroupTool(method, params || {}); } - const p = params && typeof params === 'object' ? (params as Record) : {}; + 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 @@ -1682,47 +1724,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[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}`); } diff --git a/gitnexus/src/mcp/tools.ts b/gitnexus/src/mcp/tools.ts index 704b553c88..57f71f6752 100644 --- a/gitnexus/src/mcp/tools.ts +++ b/gitnexus/src/mcp/tools.ts @@ -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: @@ -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: @@ -578,7 +590,8 @@ SERVICE: optional monorepo path prefix (case-sensitive path segments). When "rep maximum: 3600000, }, }, - required: ['target', 'direction'], + required: ['direction'], + anyOf: [{ required: ['target'] }, { required: ['name'] }, { required: ['symbol'] }], }, }, { diff --git a/gitnexus/test/integration/local-backend-calltool.test.ts b/gitnexus/test/integration/local-backend-calltool.test.ts index 0da59583a9..d352df04a5 100644 --- a/gitnexus/test/integration/local-backend-calltool.test.ts +++ b/gitnexus/test/integration/local-backend-calltool.test.ts @@ -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'); diff --git a/gitnexus/test/unit/calltool-dispatch.test.ts b/gitnexus/test/unit/calltool-dispatch.test.ts index afee977fd8..cbd2f3cef1 100644 --- a/gitnexus/test/unit/calltool-dispatch.test.ts +++ b/gitnexus/test/unit/calltool-dispatch.test.ts @@ -327,6 +327,85 @@ 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; + 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; + expect(dispatched.file_path).toBe('src/auth.ts'); + expect(dispatched).not.toHaveProperty('file'); + }); + + 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('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' }); diff --git a/gitnexus/test/unit/tools.test.ts b/gitnexus/test/unit/tools.test.ts index 19aa57bf7c..4757fd9f7c 100644 --- a/gitnexus/test/unit/tools.test.ts +++ b/gitnexus/test/unit/tools.test.ts @@ -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([ @@ -138,10 +144,17 @@ describe('GITNEXUS_TOOLS', () => { expect(apiImpactTool.inputSchema.required).toEqual([]); }); - it('impact tool requires target and direction', () => { + it('impact tool requires direction and accepts target, name, or symbol', () => { 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.anyOf).toEqual([ + { required: ['target'] }, + { required: ['name'] }, + { required: ['symbol'] }, + ]); }); it('impact tool advertises the PDG-only `line` statement anchor (integer, min 0, not required)', () => { From 3f3494fd323f36ad616ed36713a8efc9c9674904 Mon Sep 17 00:00:00 2001 From: Eva Date: Thu, 16 Jul 2026 09:29:47 +0700 Subject: [PATCH 2/3] fix(mcp): validate aliases without schema combinators --- gitnexus/src/mcp/local/local-backend.ts | 19 ++++++++-- gitnexus/src/mcp/tools.ts | 1 - gitnexus/test/unit/calltool-dispatch.test.ts | 37 ++++++++++++++++++++ gitnexus/test/unit/tools.test.ts | 10 +++--- 4 files changed, 57 insertions(+), 10 deletions(-) diff --git a/gitnexus/src/mcp/local/local-backend.ts b/gitnexus/src/mcp/local/local-backend.ts index 2cf60e64be..72741bd2de 100644 --- a/gitnexus/src/mcp/local/local-backend.ts +++ b/gitnexus/src/mcp/local/local-backend.ts @@ -163,10 +163,15 @@ function normalizeToolParams( const normalized = { ...input }; for (const { canonical, aliases } of definitions) { const keys = [canonical, ...aliases]; - const supplied = keys.flatMap((key) => { + const supplied: Array<{ key: string; value: string }> = []; + for (const key of keys) { + if (!Object.prototype.hasOwnProperty.call(input, key)) continue; const value = input[key]; - return typeof value === 'string' && value.trim() ? [{ key, value: value.trim() }] : []; - }); + if (typeof value !== 'string' || !value.trim()) { + return { error: `MCP parameter ${method}.${key} must be a non-empty string.` }; + } + supplied.push({ key, value: value.trim() }); + } const distinctValues = new Set(supplied.map(({ value }) => value)); if (distinctValues.size > 1) { return { @@ -179,6 +184,14 @@ function normalizeToolParams( 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 }; } diff --git a/gitnexus/src/mcp/tools.ts b/gitnexus/src/mcp/tools.ts index 57f71f6752..792f62b3ba 100644 --- a/gitnexus/src/mcp/tools.ts +++ b/gitnexus/src/mcp/tools.ts @@ -591,7 +591,6 @@ SERVICE: optional monorepo path prefix (case-sensitive path segments). When "rep }, }, required: ['direction'], - anyOf: [{ required: ['target'] }, { required: ['name'] }, { required: ['symbol'] }], }, }, { diff --git a/gitnexus/test/unit/calltool-dispatch.test.ts b/gitnexus/test/unit/calltool-dispatch.test.ts index cbd2f3cef1..7439f02397 100644 --- a/gitnexus/test/unit/calltool-dispatch.test.ts +++ b/gitnexus/test/unit/calltool-dispatch.test.ts @@ -391,6 +391,43 @@ describe('LocalBackend.callTool', () => { expect(resolveSpy).not.toHaveBeenCalled(); }); + it.each([ + ['impact', { target: '', direction: 'upstream' }], + ['impact', { name: 42, direction: 'upstream' }], + ['context', { name: 'validate', file: ' ' }], + ])('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 diff --git a/gitnexus/test/unit/tools.test.ts b/gitnexus/test/unit/tools.test.ts index 4757fd9f7c..82cd718fcc 100644 --- a/gitnexus/test/unit/tools.test.ts +++ b/gitnexus/test/unit/tools.test.ts @@ -144,17 +144,15 @@ describe('GITNEXUS_TOOLS', () => { expect(apiImpactTool.inputSchema.required).toEqual([]); }); - it('impact tool requires direction and accepts target, name, or symbol', () => { + 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('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.anyOf).toEqual([ - { required: ['target'] }, - { required: ['name'] }, - { required: ['symbol'] }, - ]); + 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)', () => { From d4576630eb9734579b6732e144906132f5430474 Mon Sep 17 00:00:00 2001 From: Eva Date: Thu, 16 Jul 2026 10:07:06 +0700 Subject: [PATCH 3/3] fix(mcp): ignore undefined alias inputs --- gitnexus/src/mcp/local/local-backend.ts | 2 ++ gitnexus/test/unit/calltool-dispatch.test.ts | 16 ++++++++++++++++ 2 files changed, 18 insertions(+) diff --git a/gitnexus/src/mcp/local/local-backend.ts b/gitnexus/src/mcp/local/local-backend.ts index 72741bd2de..b10cb42694 100644 --- a/gitnexus/src/mcp/local/local-backend.ts +++ b/gitnexus/src/mcp/local/local-backend.ts @@ -167,6 +167,8 @@ function normalizeToolParams( 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.` }; } diff --git a/gitnexus/test/unit/calltool-dispatch.test.ts b/gitnexus/test/unit/calltool-dispatch.test.ts index 7439f02397..eb0b94e264 100644 --- a/gitnexus/test/unit/calltool-dispatch.test.ts +++ b/gitnexus/test/unit/calltool-dispatch.test.ts @@ -363,6 +363,21 @@ describe('LocalBackend.callTool', () => { 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') @@ -395,6 +410,7 @@ describe('LocalBackend.callTool', () => { ['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');