GitNexus/gitnexus/test/unit/tools.test.ts
Gergő Magyar 4682a477d8
feat(mcp): paginate list_repos to avoid client token truncation (#2119) (#2120)
* feat(mcp): paginate list_repos to avoid client token truncation (#2119)

list_repos returned every indexed repository in one unpaginated array,
which large/LLM MCP clients truncate by token limit — so agents with
hundreds of indexed repos could not enumerate them all (the data
transmits fully; the consuming client drops it).

Add bounded limit/offset pagination to the list_repos tool:
- result changes from a bare array to
  { repositories, pagination: { total, limit, offset, returned,
  hasMore, nextOffset } }; default page 50, max 200 (shared constants)
- reject malformed limit/offset; clamp limit above the max
- deterministic order (lower-cased name, then path) over one registry
  snapshot per call, so paging never skips or duplicates an entry
- covers both stdio and remote /api/mcp (shared createMCPServer/callTool)

The internal listRepos() method (5 callers), GET /api/repos, and the
`gitnexus list` CLI are unchanged. The array->object tool-result shape
is a deliberate contract change, documented in CHANGELOG.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(mcp): reject list_repos limit above the max instead of clamping (#2119)

parseListReposPagination silently clamped limit>max to the maximum while
throwing on every other out-of-bounds value (limit<1, offset<0, non-integer,
NaN). A client that advanced offset by its requested limit (rather than
pagination.nextOffset) then silently skipped repositories and saw
hasMore:false — defeating the "never skips" guarantee. Reject an over-max
limit too, so validation is symmetric and a caller never gets a smaller page
than it asked for without a clear error. Updates the schema/description, the
helper + ListReposPagination JSDoc, the guide note, and the two clamp tests.

Resolves the cross-engine-corroborated P2 (Codex + adversarial lane) and the
maintainability lane's clamp-vs-throw inconsistency from the PR #2120 review.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* refactor(mcp): name the list_repos return type and mark the parser @internal

Extract the inline listRepos() element shape into an exported RepoListing
interface and use it for both listRepos() and listReposPage().repositories,
replacing the opaque Awaited<ReturnType<LocalBackend['listRepos']>> expression
the maintainability review flagged. Tag parseListReposPagination @internal
(it is exported only for unit testing). Pure type/JSDoc change; no behavior.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* refactor(eval-server): type formatListReposResult to the paginated shape

Narrow formatListReposResult's parameter from `any` to
{ repositories: RepoListing[]; pagination?: ListReposPagination } and drop the
dead bare-array branch — after #2119 callTool('list_repos') always returns the
paginated object, so the Array.isArray shim was unreachable. Add a list_repos
continuation hint to the eval-server's getNextStepHint (parity with the MCP
server), and cover the previously-untested non-empty + hasMore:false formatter
branch. Migrates the two bare-array formatter tests to the object shape.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* test(mcp): harden list_repos pagination coverage

- Exercise the #2054 sibling-clone guarantee through the real callTool tool
  path (in the #2054 describe, which has temp-dir cleanup), proving siblings
  and remoteUrl survive listReposPage's sort+slice — not only listRepos().
- Assert total + limit on the middle-page test (a total miscalculation at a
  non-zero offset would otherwise slip past it).
- Cover the benign boundaries: negative-zero offset (accepted as page 0) and a
  MAX_SAFE_INTEGER offset (empty page).
- Replace the integration test's '\n\n---' split with a string-aware brace
  scan, so a repo path containing braces can never truncate the JSON parse.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* docs(skills): sync the list_repos pagination example to the guide mirrors

The .claude and gitnexus-claude-plugin guide mirrors only carried the one-line
table note; add the full "Paginating list_repos" section (shape + multi-page
traversal example + notes) so all three guide copies are byte-consistent with
the canonical gitnexus/skills/gitnexus-guide.md.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* chore: drop list_repos CHANGELOG entries from this PR

Restore gitnexus/CHANGELOG.md to match main so this PR contributes no
changelog change; the changelog is curated separately from feature PRs.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-06-09 19:59:54 +01:00

234 lines
9.6 KiB
TypeScript

/**
* Unit Tests: MCP Tool Definitions
*
* Tests: GITNEXUS_TOOLS from tools.ts
* - All 13 tools are defined (per-repo + group_list/group_sync)
* - Each tool has valid name, description, inputSchema
* - Required fields are correct
* - Optional repo parameter is present on tools that need it
*/
import { describe, it, expect } from 'vitest';
import {
GITNEXUS_TOOLS,
LIST_REPOS_DEFAULT_LIMIT,
LIST_REPOS_MAX_LIMIT,
} from '../../src/mcp/tools.js';
const GROUP_TOOLS = new Set(['group_list', 'group_sync']);
const MUTATING_TOOLS = new Set(['rename', 'group_sync']);
// Read-only tools that legitimately reach external systems. Add a tool name
// here when introducing a read-only tool that needs openWorldHint: true.
const OPEN_WORLD_READ_ONLY_TOOLS = new Set(['query']);
describe('GITNEXUS_TOOLS', () => {
it('exports all tools (7 base + 3 route/tool/shape + 1 api_impact + 2 group)', () => {
expect(GITNEXUS_TOOLS).toHaveLength(13);
});
it('contains all expected tool names', () => {
const names = GITNEXUS_TOOLS.map((t) => t.name);
expect(names).toEqual(
expect.arrayContaining([
'list_repos',
'query',
'cypher',
'context',
'detect_changes',
'rename',
'impact',
'api_impact',
]),
);
});
it('each tool has name, description, and inputSchema', () => {
for (const tool of GITNEXUS_TOOLS) {
expect(tool.name).toBeTruthy();
expect(typeof tool.name).toBe('string');
expect(tool.description).toBeTruthy();
expect(typeof tool.description).toBe('string');
expect(tool.annotations).toBeDefined();
expect(tool.inputSchema).toBeDefined();
expect(tool.inputSchema.type).toBe('object');
expect(tool.inputSchema.properties).toBeDefined();
expect(Array.isArray(tool.inputSchema.required)).toBe(true);
}
});
it('each tool exposes all MCP safety annotations', () => {
for (const tool of GITNEXUS_TOOLS) {
expect(typeof tool.annotations.readOnlyHint).toBe('boolean');
expect(typeof tool.annotations.destructiveHint).toBe('boolean');
expect(typeof tool.annotations.idempotentHint).toBe('boolean');
expect(typeof tool.annotations.openWorldHint).toBe('boolean');
}
});
it('read-only tools are marked non-destructive and idempotent', () => {
for (const tool of GITNEXUS_TOOLS) {
if (MUTATING_TOOLS.has(tool.name)) continue;
expect(tool.annotations.readOnlyHint).toBe(true);
expect(tool.annotations.destructiveHint).toBe(false);
expect(tool.annotations.idempotentHint).toBe(true);
expect(tool.annotations.openWorldHint).toBe(OPEN_WORLD_READ_ONLY_TOOLS.has(tool.name));
}
});
it('query is marked open-world because it may use external embeddings', () => {
const queryTool = GITNEXUS_TOOLS.find((t) => t.name === 'query')!;
expect(queryTool.annotations).toEqual({
readOnlyHint: true,
destructiveHint: false,
idempotentHint: true,
openWorldHint: true,
});
});
it('rename and group_sync are marked mutating and non-idempotent', () => {
for (const name of ['rename', 'group_sync'] as const) {
const tool = GITNEXUS_TOOLS.find((t) => t.name === name)!;
expect(tool.annotations).toEqual({
readOnlyHint: false,
destructiveHint: true,
idempotentHint: false,
openWorldHint: false,
});
}
});
it('query tool requires "query" parameter', () => {
const queryTool = GITNEXUS_TOOLS.find((t) => t.name === 'query')!;
expect(queryTool.inputSchema.required).toContain('query');
expect(queryTool.inputSchema.properties.query).toBeDefined();
expect(queryTool.inputSchema.properties.query.type).toBe('string');
});
it('cypher tool requires "query" parameter', () => {
const cypherTool = GITNEXUS_TOOLS.find((t) => t.name === 'cypher')!;
expect(cypherTool.inputSchema.required).toContain('query');
expect(cypherTool.inputSchema.properties.params).toBeDefined();
expect(cypherTool.inputSchema.properties.params.type).toBe('object');
expect(cypherTool.inputSchema.properties.params.description).toContain('prepared statement');
});
it('context tool has no required parameters', () => {
const contextTool = GITNEXUS_TOOLS.find((t) => t.name === 'context')!;
expect(contextTool.inputSchema.required).toEqual([]);
});
it('impact tool requires target and direction', () => {
const impactTool = GITNEXUS_TOOLS.find((t) => t.name === 'impact')!;
expect(impactTool.inputSchema.required).toContain('target');
expect(impactTool.inputSchema.required).toContain('direction');
});
it('rename tool requires new_name', () => {
const renameTool = GITNEXUS_TOOLS.find((t) => t.name === 'rename')!;
expect(renameTool.inputSchema.required).toContain('new_name');
});
it('detect_changes tool has no required parameters', () => {
const detectTool = GITNEXUS_TOOLS.find((t) => t.name === 'detect_changes')!;
expect(detectTool.inputSchema.required).toEqual([]);
});
it('list_repos tool exposes optional limit/offset pagination params', () => {
const listTool = GITNEXUS_TOOLS.find((t) => t.name === 'list_repos')!;
const props = listTool.inputSchema.properties;
expect(props.limit).toBeDefined();
expect(props.limit.type).toBe('integer');
expect(props.offset).toBeDefined();
expect(props.offset.type).toBe('integer');
// Pagination is opt-in: zero-arg callers must still be valid.
expect(listTool.inputSchema.required).toEqual([]);
// No `repo` param on list_repos (it lists all repos).
expect(props.repo).toBeUndefined();
// Description must teach an LLM to page through every repository.
expect(listTool.description.toLowerCase()).toContain('paginat');
expect(listTool.description).toContain('nextOffset');
expect(listTool.description).toContain('hasMore');
});
it('list_repos schema bounds match the exported pagination constants', () => {
const listTool = GITNEXUS_TOOLS.find((t) => t.name === 'list_repos')!;
const { limit, offset } = listTool.inputSchema.properties;
expect(limit.minimum).toBe(1);
expect(limit.maximum).toBe(LIST_REPOS_MAX_LIMIT);
expect(limit.default).toBe(LIST_REPOS_DEFAULT_LIMIT);
expect(offset.minimum).toBe(0);
expect(offset.default).toBe(0);
// Sane, documented bounds (guards against accidental constant drift).
expect(LIST_REPOS_DEFAULT_LIMIT).toBeLessThanOrEqual(LIST_REPOS_MAX_LIMIT);
expect(LIST_REPOS_DEFAULT_LIMIT).toBeGreaterThan(0);
});
it('per-repo tools have optional repo parameter for backend selection', () => {
for (const tool of GITNEXUS_TOOLS) {
if (tool.name === 'list_repos') continue;
if (GROUP_TOOLS.has(tool.name)) continue;
expect(tool.inputSchema.properties.repo).toBeDefined();
expect(tool.inputSchema.properties.repo.type).toBe('string');
expect(tool.inputSchema.required).not.toContain('repo');
}
});
it('group tools without backend repo param omit repo property', () => {
for (const name of ['group_list', 'group_sync'] as const) {
const tool = GITNEXUS_TOOLS.find((t) => t.name === name)!;
expect(tool.inputSchema.properties).not.toHaveProperty('repo');
}
});
it('impact, query, and context expose optional service with minLength', () => {
for (const n of ['impact', 'query', 'context'] as const) {
const tool = GITNEXUS_TOOLS.find((t) => t.name === n)!;
const svc = tool.inputSchema.properties.service;
expect(svc, n).toBeDefined();
expect(svc!.minLength).toBe(1);
}
});
it('impact schema bounds match cross-impact validation ranges', () => {
const impact = GITNEXUS_TOOLS.find((t) => t.name === 'impact')!;
expect(impact.inputSchema.properties.maxDepth.minimum).toBe(1);
expect(impact.inputSchema.properties.maxDepth.maximum).toBe(32);
expect(impact.inputSchema.properties.minConfidence.minimum).toBe(0);
expect(impact.inputSchema.properties.minConfidence.maximum).toBe(1);
expect(impact.inputSchema.properties.timeoutMs.maximum).toBe(3600000);
});
it('detect_changes scope has correct enum values', () => {
const detectTool = GITNEXUS_TOOLS.find((t) => t.name === 'detect_changes')!;
const scopeProp = detectTool.inputSchema.properties.scope;
expect(scopeProp.enum).toEqual(['unstaged', 'staged', 'all', 'compare']);
});
it('api_impact tool has no required parameters', () => {
const apiImpactTool = GITNEXUS_TOOLS.find((t) => t.name === 'api_impact')!;
expect(apiImpactTool).toBeDefined();
expect(apiImpactTool.inputSchema.required).toEqual([]);
expect(apiImpactTool.inputSchema.properties.route).toBeDefined();
expect(apiImpactTool.inputSchema.properties.file).toBeDefined();
expect(apiImpactTool.inputSchema.properties.repo).toBeDefined();
});
it('impact relationTypes is array of strings', () => {
const impactTool = GITNEXUS_TOOLS.find((t) => t.name === 'impact')!;
const relProp = impactTool.inputSchema.properties.relationTypes;
expect(relProp.type).toBe('array');
expect(relProp.items).toEqual({ type: 'string' });
});
it('route_map description defers to api_impact for pre-change analysis', () => {
const routeMapTool = GITNEXUS_TOOLS.find((t) => t.name === 'route_map')!;
expect(routeMapTool.description).toContain('api_impact');
expect(routeMapTool.description).toContain('pre-change analysis');
});
it('shape_check description defers to api_impact for pre-change analysis', () => {
const shapeCheckTool = GITNEXUS_TOOLS.find((t) => t.name === 'shape_check')!;
expect(shapeCheckTool.description).toContain('api_impact');
expect(shapeCheckTool.description).toContain('pre-change analysis');
});
});