From 1f64becb301a02016d27aa9130350bcc38993adc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gerg=C5=91=20Magyar?= Date: Sat, 12 Sep 2026 07:55:14 +0100 Subject: [PATCH] fix(mcp): reject unknown tool arguments and honor depth (#3267) --- gitnexus/src/mcp/local/local-backend.ts | 14 +- gitnexus/src/mcp/server.ts | 7 + gitnexus/src/mcp/tool-arguments.ts | 174 ++++++++++++++++++ gitnexus/src/mcp/tools.ts | 25 +++ gitnexus/test/unit/calltool-dispatch.test.ts | 32 ++++ gitnexus/test/unit/mcp-tool-arguments.test.ts | 147 +++++++++++++++ .../test/unit/mcp/group-repo-routing.test.ts | 44 +++++ gitnexus/test/unit/server.test.ts | 82 +++++++++ gitnexus/test/unit/tools.test.ts | 13 ++ 9 files changed, 532 insertions(+), 6 deletions(-) create mode 100644 gitnexus/src/mcp/tool-arguments.ts create mode 100644 gitnexus/test/unit/mcp-tool-arguments.test.ts diff --git a/gitnexus/src/mcp/local/local-backend.ts b/gitnexus/src/mcp/local/local-backend.ts index 5761baf64..1e6f561b0 100644 --- a/gitnexus/src/mcp/local/local-backend.ts +++ b/gitnexus/src/mcp/local/local-backend.ts @@ -117,6 +117,7 @@ import { PDG_QUERY_DEFAULT_LIMIT, PDG_QUERY_MAX_LIMIT, } from '../tools.js'; +import { foldNumericToolArgumentAliases } from '../tool-arguments.js'; import { findImportCycles, IMPORT_CYCLE_LIMIT } from '../../core/graph/import-cycles.js'; import { decodeTaintPath } from '../../core/ingestion/taint/path-codec.js'; import { decodeReachingDefReason } from '../../core/ingestion/cfg/reaching-def-reason-codec.js'; @@ -274,10 +275,8 @@ function normalizeToolParams( ): { 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) { + for (const { canonical, aliases } of definitions ?? []) { const keys = [canonical, ...aliases]; const supplied: Array<{ key: string; value: string }> = []; for (const key of keys) { @@ -308,14 +307,17 @@ function normalizeToolParams( if (supplied.length > 0) normalized[canonical] = supplied[0].value; } + const folded = foldNumericToolArgumentAliases(method, normalized); + if ('error' in folded) return folded; + if ( method === 'impact' && - typeof normalized.target !== 'string' && - (typeof normalized.target_uid !== 'string' || !normalized.target_uid.trim()) + typeof folded.params.target !== 'string' && + (typeof folded.params.target_uid !== 'string' || !folded.params.target_uid.trim()) ) { return { error: 'MCP impact requires target, name, symbol, or target_uid.' }; } - return { params: normalized }; + return { params: folded.params }; } // AI context generation is CLI-only (gitnexus analyze) diff --git a/gitnexus/src/mcp/server.ts b/gitnexus/src/mcp/server.ts index 118f28fa1..f95edcec4 100644 --- a/gitnexus/src/mcp/server.ts +++ b/gitnexus/src/mcp/server.ts @@ -42,6 +42,7 @@ import { mcpRepositoryPolicyConfigured, } from './repository-policy.js'; import { applyMcpMaxTokens, resolveMcpMaxTokens, withoutMcpBudgetArg } from './output-budget.js'; +import { assertKnownMcpToolArguments, schemaSourceToolName } from './tool-arguments.js'; /** * Next-step hints appended to tool responses. @@ -220,6 +221,12 @@ export function createMCPServer( try { const typedArgs = args as Record | undefined; assertMcpReadOnlyToolCall(name, typedArgs, readOnly); + const schemaSource = schemaSourceToolName(name); + const advertisedTool = GITNEXUS_TOOLS.find((tool) => tool.name === schemaSource); + if (advertisedTool) { + const listed = toolForReadOnlyMcp(repositoryPolicy.toolForMcp(advertisedTool), readOnly); + assertKnownMcpToolArguments(name, typedArgs, listed.inputSchema.properties); + } maxTokens = resolveMcpMaxTokens(name, typedArgs); const result = await scopedBackend.callTool(name, withoutMcpBudgetArg(typedArgs)); const resultText = typeof result === 'string' ? result : JSON.stringify(result, null, 2); diff --git a/gitnexus/src/mcp/tool-arguments.ts b/gitnexus/src/mcp/tool-arguments.ts new file mode 100644 index 000000000..04d6cec1f --- /dev/null +++ b/gitnexus/src/mcp/tool-arguments.ts @@ -0,0 +1,174 @@ +/** + * MCP tool-argument contract (#3261). + * + * `tools/list` advertises `inputSchema`; `tools/call` used to forward any JSON + * object. A misspelled or CLI-taught key (`depth` instead of `maxDepth`) then + * produced a well-formed answer computed from the server default — no error, + * no warning. This module is the single dispatch-time check that the keys a + * caller sent are ones the advertised schema (or an unpublished handler alias) + * actually reads. + */ + +import { GITNEXUS_TOOLS } from './tools.js'; + +/** Legacy MCP names that reuse another tool's advertised schema. */ +export const LEGACY_TOOL_SCHEMA_SOURCE: Readonly> = { + search: 'query', + explore: 'context', +}; + +/** + * Keys the handler still reads but that must NOT appear in `inputSchema` + * (#2175: advertising `query` makes Claude Code drop the argument). + */ +export const UNPUBLISHED_TOOL_ARGUMENT_ALIASES: Readonly> = { + query: ['query'], + cypher: ['query'], + // Group-mode context still reads `target` as the symbol name; local + // `name` is the advertised key. Advertising `target` would collide with + // impact's target vocabulary and is not in tools/list. Legacy `search` + // and `explore` inherit via schemaSourceToolName. + context: ['target'], +}; + +export interface NumericArgumentAlias { + canonical: string; + aliases: readonly string[]; +} + +/** + * Numeric aliases that the backend folds onto the advertised canonical key. + * `depth` is the CLI flag name for `maxDepth` on impact and trace. + */ +export const TOOL_NUMERIC_ARGUMENT_ALIASES: Readonly< + Record +> = { + impact: [{ canonical: 'maxDepth', aliases: ['depth'] }], + trace: [{ canonical: 'maxDepth', aliases: ['depth'] }], +}; + +export function schemaSourceToolName(toolName: string): string { + return LEGACY_TOOL_SCHEMA_SOURCE[toolName] ?? toolName; +} + +export function advertisedToolPropertyNames(toolName: string): string[] | undefined { + const source = schemaSourceToolName(toolName); + const tool = GITNEXUS_TOOLS.find((entry) => entry.name === source); + if (!tool) return undefined; + return Object.keys(tool.inputSchema.properties); +} + +function normalizeArgumentKey(key: string): string { + return key.toLowerCase().replace(/_/gu, ''); +} + +export function suggestKnownToolArgument( + unknownKey: string, + knownKeys: readonly string[], +): string | undefined { + const needle = normalizeArgumentKey(unknownKey); + if (!needle) return undefined; + const exact = knownKeys.find((key) => normalizeArgumentKey(key) === needle); + if (exact) return exact; + const contained = knownKeys.filter((key) => { + const normalized = normalizeArgumentKey(key); + return normalized.includes(needle) || needle.includes(normalized); + }); + return contained.length === 1 ? contained[0] : undefined; +} + +function formatUnknownArgumentError( + toolName: string, + unknownKeys: readonly string[], + advertisedKeys: readonly string[], +): string { + const quoted = unknownKeys.map((key) => `"${key}"`).join(', '); + const noun = unknownKeys.length === 1 ? 'argument' : 'arguments'; + const verb = unknownKeys.length === 1 ? 'does' : 'do'; + const suggestion = + unknownKeys.length === 1 ? suggestKnownToolArgument(unknownKeys[0], advertisedKeys) : undefined; + if (suggestion) { + return `Unknown ${noun} ${quoted} for tool "${toolName}". Did you mean "${suggestion}"?`; + } + return ( + `Unknown ${noun} ${quoted} for tool "${toolName}". ` + + `The advertised inputSchema ${verb} not include ${unknownKeys.length === 1 ? 'this key' : 'these keys'}.` + ); +} + +/** + * Reject top-level tool arguments that are neither advertised nor an + * unpublished handler alias. `advertisedProperties` should be the schema the + * caller actually saw (`tools/list` after read-only / repository-policy + * scrubbing). When it is omitted, the canonical `GITNEXUS_TOOLS` schema is + * used. Tools with no schema (legacy `overview`) are left unchecked. + */ +export function assertKnownMcpToolArguments( + toolName: string, + args: Record | undefined, + advertisedProperties?: Record, +): void { + if (!args) return; + const propertyNames = + advertisedProperties !== undefined + ? Object.keys(advertisedProperties) + : advertisedToolPropertyNames(toolName); + if (!propertyNames) return; + + const unpublished = + UNPUBLISHED_TOOL_ARGUMENT_ALIASES[toolName] ?? + UNPUBLISHED_TOOL_ARGUMENT_ALIASES[schemaSourceToolName(toolName)] ?? + []; + const allowed = new Set([...propertyNames, ...unpublished]); + const unknownKeys = Object.keys(args).filter((key) => !allowed.has(key)); + if (unknownKeys.length === 0) return; + throw new Error(formatUnknownArgumentError(toolName, unknownKeys, propertyNames)); +} + +/** + * Fold numeric aliases onto their canonical key (e.g. `depth` → `maxDepth`). + * Conflicting values error; a single agreed value is written to the canonical + * key and the alias keys are removed so every downstream reader sees one name. + */ +export function foldNumericToolArgumentAliases( + toolName: string, + params: Record, +): { params: Record } | { error: string } { + const definitions = TOOL_NUMERIC_ARGUMENT_ALIASES[toolName]; + if (!definitions) return { params }; + + const normalized = { ...params }; + for (const { canonical, aliases } of definitions) { + const keys = [canonical, ...aliases]; + const supplied: Array<{ key: string; value: number }> = []; + for (const key of keys) { + if (!Object.prototype.hasOwnProperty.call(normalized, key)) continue; + const value = normalized[key]; + if (value === undefined) continue; + if (typeof value !== 'number') { + return { error: `MCP parameter ${toolName}.${key} must be a number.` }; + } + // #2279: some MCP adapters materialize an omitted optional number as 0, + // and a coerced missing value arrives as NaN. Treat both sentinels as + // absent so they cannot conflict with a real maxDepth or fold onto + // `params.maxDepth || 3`. The handlers already map a non-positive or + // non-integer maxDepth to their default; erroring here turned that + // contract into an error payload instead. + if (value === 0 || Number.isNaN(value)) continue; + supplied.push({ key, value }); + } + const distinctValues = new Set(supplied.map(({ value }) => value)); + if (distinctValues.size > 1) { + return { + error: `Conflicting MCP parameters for ${toolName}.${canonical}: ${supplied + .map(({ key }) => key) + .join(', ')} must agree.`, + }; + } + // Drop every source key, then write back the single agreed value (if any), + // so a sentinel 0/NaN never survives on the canonical key. + for (const key of keys) delete normalized[key]; + if (supplied.length > 0) normalized[canonical] = supplied[0].value; + } + return { params: normalized }; +} diff --git a/gitnexus/src/mcp/tools.ts b/gitnexus/src/mcp/tools.ts index 01f2a6a31..dfb9423ff 100644 --- a/gitnexus/src/mcp/tools.ts +++ b/gitnexus/src/mcp/tools.ts @@ -28,6 +28,7 @@ export interface ToolDefinition { } >; required: string[]; + additionalProperties?: false; }; } @@ -575,6 +576,13 @@ SERVICE: optional monorepo path prefix (case-sensitive path segments). When "rep minimum: 1, maximum: IMPACT_MAX_DEPTH, }, + depth: { + type: 'number', + description: + 'Compatibility alias for maxDepth (CLI --depth). Values must agree when both are present. Literal 0 is an omitted-value compatibility sentinel.', + minimum: 0, + maximum: IMPACT_MAX_DEPTH, + }, crossDepth: { type: 'number', description: @@ -931,6 +939,13 @@ DESTINATION TRACE (cross-repo): for an "@groupName" trace, OMIT to/to_uid/to_fil minimum: 1, maximum: 30, }, + depth: { + type: 'number', + description: + 'Compatibility alias for maxDepth (CLI --depth). Values must agree when both are present. Literal 0 is an omitted-value compatibility sentinel.', + minimum: 0, + maximum: 30, + }, includeTests: { type: 'boolean', description: 'Include test-file symbols in traversal (default: false)', @@ -993,6 +1008,16 @@ export const REPO_SCOPED_TOOLS = new Set([ ]); for (const tool of GITNEXUS_TOOLS) { + // Advertises a closed schema; tools/call still fail-closes on the scrubbed key list. + // The unpublished handler aliases in tool-arguments.ts stay off this schema on + // purpose (#2175), and closing it strands no caller: every alias has an + // advertised counterpart reaching the same handler — `query` → `search_query` + // on query, `query` → `statement` on cypher, and `target` → `name` on group + // context, which local-backend maps to the group target (the group name comes + // from `repo: "@group"`, not from `name`; see test/unit/mcp/group-repo-routing). + // A schema-validating client therefore has a valid call for every tool, and + // advertising the aliases instead would re-break Claude Code on `query`. + tool.inputSchema.additionalProperties = false; 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 diff --git a/gitnexus/test/unit/calltool-dispatch.test.ts b/gitnexus/test/unit/calltool-dispatch.test.ts index 7b38e3982..3daa804a1 100644 --- a/gitnexus/test/unit/calltool-dispatch.test.ts +++ b/gitnexus/test/unit/calltool-dispatch.test.ts @@ -515,9 +515,41 @@ describe('LocalBackend.callTool', () => { expect(impactSpy.mock.calls[0][1]).toMatchObject({ target: 'validate' }); }); + it('folds CLI-style depth onto maxDepth before impact (#3261)', async () => { + const impactSpy = vi + .spyOn(backend as any, 'impact') + .mockResolvedValue({ status: 'normalized' }); + + await backend.callTool('impact', { + target: 'validate', + direction: 'upstream', + depth: 2, + }); + + expect(impactSpy.mock.calls[0][1]).toMatchObject({ target: 'validate', maxDepth: 2 }); + expect(impactSpy.mock.calls[0][1]).not.toHaveProperty('depth'); + }); + + it('treats depth 0 as omitted when maxDepth is present (#2279)', async () => { + const impactSpy = vi + .spyOn(backend as any, 'impact') + .mockResolvedValue({ status: 'normalized' }); + + await backend.callTool('impact', { + target: 'validate', + direction: 'upstream', + maxDepth: 2, + depth: 0, + }); + + expect(impactSpy.mock.calls[0][1]).toMatchObject({ target: 'validate', maxDepth: 2 }); + expect(impactSpy.mock.calls[0][1]).not.toHaveProperty('depth'); + }); + it.each([ ['impact', { target: 'validate', name: 'login', direction: 'upstream' }], ['impact', { name: 'validate', symbol: 'login', direction: 'upstream' }], + ['impact', { target: 'validate', direction: 'upstream', maxDepth: 3, depth: 1 }], ['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, 'selectToolRepository'); diff --git a/gitnexus/test/unit/mcp-tool-arguments.test.ts b/gitnexus/test/unit/mcp-tool-arguments.test.ts new file mode 100644 index 000000000..1555fedf1 --- /dev/null +++ b/gitnexus/test/unit/mcp-tool-arguments.test.ts @@ -0,0 +1,147 @@ +import { describe, expect, it } from 'vitest'; +import { + assertKnownMcpToolArguments, + foldNumericToolArgumentAliases, + suggestKnownToolArgument, +} from '../../src/mcp/tool-arguments.js'; +import { GITNEXUS_TOOLS } from '../../src/mcp/tools.js'; + +const impactProperties = GITNEXUS_TOOLS.find((tool) => tool.name === 'impact')!.inputSchema + .properties; + +describe('assertKnownMcpToolArguments (#3261)', () => { + it('allows advertised impact keys including the depth alias', () => { + expect(() => + assertKnownMcpToolArguments( + 'impact', + { target: 'auth', direction: 'downstream', depth: 2 }, + impactProperties, + ), + ).not.toThrow(); + }); + + it('rejects an unknown key and names it', () => { + expect(() => + assertKnownMcpToolArguments( + 'impact', + { target: 'auth', direction: 'downstream', notARealArg: 2 }, + impactProperties, + ), + ).toThrow(/Unknown argument "notARealArg" for tool "impact"/); + }); + + it('suggests maxDepth for the snake_case misspelling', () => { + expect(() => + assertKnownMcpToolArguments( + 'impact', + { target: 'auth', direction: 'downstream', max_depth: 2 }, + impactProperties, + ), + ).toThrow(/Did you mean "maxDepth"/); + }); + + it('still accepts the unpublished query alias (#2175)', () => { + const queryProperties = GITNEXUS_TOOLS.find((tool) => tool.name === 'query')!.inputSchema + .properties; + expect(() => + assertKnownMcpToolArguments('query', { query: 'auth' }, queryProperties), + ).not.toThrow(); + const cypherProperties = GITNEXUS_TOOLS.find((tool) => tool.name === 'cypher')!.inputSchema + .properties; + expect(() => + assertKnownMcpToolArguments('cypher', { query: 'MATCH (n) RETURN n' }, cypherProperties), + ).not.toThrow(); + }); + + it('still accepts unpublished query on the legacy search name', () => { + const queryProperties = GITNEXUS_TOOLS.find((tool) => tool.name === 'query')!.inputSchema + .properties; + expect(() => + assertKnownMcpToolArguments('search', { query: 'auth' }, queryProperties), + ).not.toThrow(); + }); + + it('still accepts unpublished context target (group-mode alias)', () => { + const contextProperties = GITNEXUS_TOOLS.find((tool) => tool.name === 'context')!.inputSchema + .properties; + expect(() => + assertKnownMcpToolArguments('context', { repo: '@g1', target: 'Sym' }, contextProperties), + ).not.toThrow(); + expect(() => + assertKnownMcpToolArguments('explore', { repo: '@g1', target: 'Sym' }, contextProperties), + ).not.toThrow(); + }); + + it('does not suggest the unpublished query alias (#2175)', () => { + const queryProperties = GITNEXUS_TOOLS.find((tool) => tool.name === 'query')!.inputSchema + .properties; + expect(() => assertKnownMcpToolArguments('query', { Query: 'auth' }, queryProperties)).toThrow( + /Unknown argument "Query" for tool "query"/, + ); + expect(() => + assertKnownMcpToolArguments('query', { Query: 'auth' }, queryProperties), + ).not.toThrow(/Did you mean "query"/); + }); + + it('skips tools that have no advertised schema', () => { + expect(() => + assertKnownMcpToolArguments('overview', { showClusters: true, extra: 1 }), + ).not.toThrow(); + }); +}); + +describe('suggestKnownToolArgument', () => { + it('matches underscore and case folding', () => { + expect(suggestKnownToolArgument('max_depth', ['maxDepth', 'target'])).toBe('maxDepth'); + }); +}); + +describe('foldNumericToolArgumentAliases', () => { + it('folds depth onto maxDepth', () => { + expect(foldNumericToolArgumentAliases('impact', { target: 'auth', depth: 2 })).toEqual({ + params: { target: 'auth', maxDepth: 2 }, + }); + }); + + it('keeps an agreed depth and maxDepth', () => { + expect(foldNumericToolArgumentAliases('impact', { maxDepth: 2, depth: 2 })).toEqual({ + params: { maxDepth: 2 }, + }); + }); + + it('rejects conflicting depth and maxDepth', () => { + expect(foldNumericToolArgumentAliases('impact', { maxDepth: 3, depth: 1 })).toEqual({ + error: 'Conflicting MCP parameters for impact.maxDepth: maxDepth, depth must agree.', + }); + }); + + it('rejects a non-numeric depth', () => { + expect(foldNumericToolArgumentAliases('trace', { depth: '2' })).toEqual({ + error: 'MCP parameter trace.depth must be a number.', + }); + }); + + it('treats literal 0 as an omitted adapter sentinel (#2279)', () => { + expect(foldNumericToolArgumentAliases('impact', { maxDepth: 2, depth: 0 })).toEqual({ + params: { maxDepth: 2 }, + }); + expect(foldNumericToolArgumentAliases('impact', { target: 'auth', depth: 0 })).toEqual({ + params: { target: 'auth' }, + }); + }); + + it('treats NaN maxDepth as omitted so trace keeps its default-depth contract', () => { + expect( + foldNumericToolArgumentAliases('trace', { from: 'A', to: 'B', maxDepth: Number.NaN }), + ).toEqual({ params: { from: 'A', to: 'B' } }); + expect(foldNumericToolArgumentAliases('trace', { maxDepth: Number.NaN, depth: 2 })).toEqual({ + params: { maxDepth: 2 }, + }); + }); + + it('keeps a negative depth so the handler applies its own default', () => { + expect(foldNumericToolArgumentAliases('trace', { depth: -5 })).toEqual({ + params: { maxDepth: -5 }, + }); + }); +}); diff --git a/gitnexus/test/unit/mcp/group-repo-routing.test.ts b/gitnexus/test/unit/mcp/group-repo-routing.test.ts index 474abeaa8..b75e31ab8 100644 --- a/gitnexus/test/unit/mcp/group-repo-routing.test.ts +++ b/gitnexus/test/unit/mcp/group-repo-routing.test.ts @@ -48,6 +48,7 @@ describe('LocalBackend @group repo routing', () => { let groupSpyQuery: ReturnType; let groupSpyImpact: ReturnType; let groupSpyContext: ReturnType; + let groupSpyTrace: ReturnType; beforeEach(() => { tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gitnexus-atgrp-')); @@ -73,6 +74,9 @@ repos: group: 'g1', results: [], }); + groupSpyTrace = vi + .spyOn(GroupService.prototype, 'groupTrace') + .mockResolvedValue({ via: 'trace' }); }); afterEach(() => { @@ -123,6 +127,46 @@ repos: ); }); + it('forwards folded depth as maxDepth on group impact (#3261)', async () => { + const backend = new LocalBackend(); + await backend.callTool('impact', { + repo: '@g1', + target: 'Sym', + direction: 'upstream', + depth: 2, + }); + expect(groupSpyImpact).toHaveBeenCalledWith( + expect.objectContaining({ + name: 'g1', + target: 'Sym', + direction: 'upstream', + maxDepth: 2, + }), + ); + const arg = groupSpyImpact.mock.calls[0][0] as Record; + expect(arg).not.toHaveProperty('depth'); + }); + + it('forwards folded depth as maxDepth on group trace (#3261)', async () => { + const backend = new LocalBackend(); + await backend.callTool('trace', { + repo: '@g1', + from: 'A', + to: 'B', + depth: 2, + }); + expect(groupSpyTrace).toHaveBeenCalledWith( + expect.objectContaining({ + name: 'g1', + from: 'A', + to: 'B', + maxDepth: 2, + }), + ); + const arg = groupSpyTrace.mock.calls[0][0] as Record; + expect(arg).not.toHaveProperty('depth'); + }); + it('routes context to groupContext', async () => { const backend = new LocalBackend(); await backend.callTool('context', { repo: '@g1', target: 'Sym' }); diff --git a/gitnexus/test/unit/server.test.ts b/gitnexus/test/unit/server.test.ts index c5ed1f1a0..945414da1 100644 --- a/gitnexus/test/unit/server.test.ts +++ b/gitnexus/test/unit/server.test.ts @@ -329,6 +329,88 @@ describe('MCP output budgets', () => { } }); + it('rejects unknown tool arguments before backend execution (#3261)', async () => { + const backend = createMockBackend(); + const { text, isError } = await callToolThroughServer(backend, 'impact', { + target: 'auth', + direction: 'downstream', + notARealArg: 2, + }); + expect(isError).toBe(true); + expect(text).toMatch(/Unknown argument "notARealArg" for tool "impact"/); + expect(backend.callTool).not.toHaveBeenCalled(); + }); + + it('accepts CLI-style depth as a known impact alias (#3261)', async () => { + // The CLI flag is --depth; since #3261 MCP advertises it alongside maxDepth + // as a compatibility alias. Before #3261, `depth` was silently dropped. + // After the fix it is a known alias and is forwarded. + const backend = createMockBackend(); + const { isError } = await callToolThroughServer(backend, 'impact', { + target: 'auth', + direction: 'downstream', + depth: 2, + }); + expect(isError).toBe(false); + expect(backend.callTool).toHaveBeenCalledWith('impact', { + target: 'auth', + direction: 'downstream', + depth: 2, + }); + }); + + it('still accepts unpublished context target through tools/call', async () => { + const backend = createMockBackend(); + const { isError } = await callToolThroughServer(backend, 'context', { + repo: '@g1', + target: 'Sym', + }); + expect(isError).toBe(false); + expect(backend.callTool).toHaveBeenCalledWith('context', { repo: '@g1', target: 'Sym' }); + }); + + it('still accepts the unpublished query alias for query (#2175)', async () => { + const backend = createMockBackend(); + const { isError } = await callToolThroughServer(backend, 'query', { + query: 'auth', + }); + expect(isError).toBe(false); + expect(backend.callTool).toHaveBeenCalledWith('query', { query: 'auth' }); + }); + + it('still accepts the unpublished query alias for cypher (#2175)', async () => { + const backend = createMockBackend(); + const { isError } = await callToolThroughServer(backend, 'cypher', { + query: 'MATCH (n) RETURN n', + }); + expect(isError).toBe(false); + expect(backend.callTool).toHaveBeenCalledWith('cypher', { query: 'MATCH (n) RETURN n' }); + }); + + it('maps legacy search/explore names through the advertised schema (#3261)', async () => { + const backend = createMockBackend(); + const searchUnknown = await callToolThroughServer(backend, 'search', { notARealArg: 1 }); + expect(searchUnknown.isError).toBe(true); + expect(searchUnknown.text).toMatch(/Unknown argument "notARealArg"/); + expect(backend.callTool).not.toHaveBeenCalled(); + + const searchOk = await callToolThroughServer(backend, 'search', { query: 'auth' }); + expect(searchOk.isError).toBe(false); + expect(backend.callTool).toHaveBeenCalledWith('search', { query: 'auth' }); + + backend.callTool.mockClear(); + const exploreUnknown = await callToolThroughServer(backend, 'explore', { notARealArg: 1 }); + expect(exploreUnknown.isError).toBe(true); + expect(backend.callTool).not.toHaveBeenCalled(); + + const exploreOk = await callToolThroughServer(backend, 'explore', { + repo: '@g1', + target: 'Sym', + }); + expect(exploreOk.isError).toBe(false); + expect(backend.callTool).toHaveBeenCalledWith('explore', { repo: '@g1', target: 'Sym' }); + }); + it('rejects a non-positive explicit maxTokens before backend execution', async () => { const backend = createMockBackend(); const { text, isError } = await callToolThroughServer(backend, 'query', { diff --git a/gitnexus/test/unit/tools.test.ts b/gitnexus/test/unit/tools.test.ts index ff6ee9cc8..9ba40dfe9 100644 --- a/gitnexus/test/unit/tools.test.ts +++ b/gitnexus/test/unit/tools.test.ts @@ -58,6 +58,19 @@ describe('GITNEXUS_TOOLS', () => { expect(tool.inputSchema.type).toBe('object'); expect(tool.inputSchema.properties).toBeDefined(); expect(Array.isArray(tool.inputSchema.required)).toBe(true); + expect(tool.inputSchema.additionalProperties).toBe(false); + } + }); + + it('impact and trace advertise depth as a maxDepth alias (#3261)', () => { + for (const name of ['impact', 'trace'] as const) { + const tool = GITNEXUS_TOOLS.find((t) => t.name === name)!; + expect(tool.inputSchema.properties.depth).toMatchObject({ + type: 'number', + description: expect.stringMatching(/maxDepth/), + minimum: 0, + }); + expect(tool.inputSchema.properties.maxDepth).toBeDefined(); } });