mirror of
https://github.com/abhigyanpatwari/GitNexus.git
synced 2026-09-14 23:22:54 +00:00
fix(mcp): reject unknown tool arguments and honor depth (#3267)
This commit is contained in:
parent
68eca0ced8
commit
1f64becb30
9 changed files with 532 additions and 6 deletions
|
|
@ -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<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) {
|
||||
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)
|
||||
|
|
|
|||
|
|
@ -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<string, unknown> | 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);
|
||||
|
|
|
|||
174
gitnexus/src/mcp/tool-arguments.ts
Normal file
174
gitnexus/src/mcp/tool-arguments.ts
Normal file
|
|
@ -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<Record<string, string>> = {
|
||||
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<Record<string, readonly string[]>> = {
|
||||
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<string, readonly NumericArgumentAlias[]>
|
||||
> = {
|
||||
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<string, unknown> | undefined,
|
||||
advertisedProperties?: Record<string, unknown>,
|
||||
): 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<string, unknown>,
|
||||
): { params: Record<string, unknown> } | { 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 };
|
||||
}
|
||||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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');
|
||||
|
|
|
|||
147
gitnexus/test/unit/mcp-tool-arguments.test.ts
Normal file
147
gitnexus/test/unit/mcp-tool-arguments.test.ts
Normal file
|
|
@ -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 },
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
@ -48,6 +48,7 @@ describe('LocalBackend @group repo routing', () => {
|
|||
let groupSpyQuery: ReturnType<typeof vi.spyOn>;
|
||||
let groupSpyImpact: ReturnType<typeof vi.spyOn>;
|
||||
let groupSpyContext: ReturnType<typeof vi.spyOn>;
|
||||
let groupSpyTrace: ReturnType<typeof vi.spyOn>;
|
||||
|
||||
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<string, unknown>;
|
||||
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<string, unknown>;
|
||||
expect(arg).not.toHaveProperty('depth');
|
||||
});
|
||||
|
||||
it('routes context to groupContext', async () => {
|
||||
const backend = new LocalBackend();
|
||||
await backend.callTool('context', { repo: '@g1', target: 'Sym' });
|
||||
|
|
|
|||
|
|
@ -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', {
|
||||
|
|
|
|||
|
|
@ -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();
|
||||
}
|
||||
});
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue