mirror of
https://github.com/abhigyanpatwari/GitNexus.git
synced 2026-09-21 00:21:30 +00:00
* fix(mcp): advertise search_query/statement params for query/cypher tools (#2175) Claude Code drops a tool-call argument named exactly 'query', making the query and cypher tools unusable from it. Rename the advertised required parameters to search_query and statement so the client transmits them. Handler-side backward-compat for the legacy 'query' key follows in the next commit. * fix(mcp): accept search_query/statement with legacy query fallback (#2175) Resolve the new advertised param names in the backend while still accepting the legacy 'query' key, so curl/HTTP, other MCP clients, the CLI, the group path, and the internal executeCypher() all keep working. Alias is normalized once at the callTool chokepoint (covers group-forward + search alias); query() and cypher() dual-read defensively. New name wins when both are supplied. Updates the required-error message and adds dual-accept unit + integration coverage. * fix(cli): pass canonical search_query/statement params to query/cypher tools (#2175) Stop the CLI from depending on the deprecated 'query' alias. No user-facing change — the positional args are unchanged and the backend accepts both keys. * fix(mcp): generators advertise search_query in query() examples (#2175) Update the three doc/example generators (ai-context AGENTS/CLAUDE block, skill-gen community skills, resources repo hint) so future analyze runs emit query({search_query: ...}) — the param name Claude Code actually transmits. Tests assert the new form is present and the legacy query({query: form is absent (the #2059 generator-test pattern). * docs(mcp): advertise search_query/statement in skill & guidance examples (#2175) Sync the committed agent-facing docs to the renamed params so a Claude Code agent following them emits the transmittable key: AGENTS.md/CLAUDE.md gitnexus block, the canonical gitnexus/skills/* source and its installed/plugin/cursor mirrors, and the README examples. Scoped rewrite of the two call prefixes only (query({query: -> search_query, cypher({query: -> statement). * style(mcp): prettier line-wrap for #2175 alias-resolution edits * fix(review): uniform search_query precedence + cypher empty guard (#2175) Code-review findings (correctness/adversarial/api-contract/maintainability consensus): - Group-mode query inverted the 'new name wins' rule: the callTool chokepoint backfilled params.query only when empty and the @group-forward read params.query directly, so a both-keys (or whitespace-legacy) group call let the legacy value win — unlike the local path. Replace the hidden param mutation with a self-contained 'search_query ?? query' resolve at the group-forward; precedence is now uniformly new-wins at every consumer site. - cypher() now returns the same friendly required-param error as query() when neither statement nor query is supplied, instead of a raw DB prepare error. - Document the legacy alias as permanent (third-party clients may send query=). Adds group-forward alias tests (both-keys + legacy-only), empty/whitespace search_query, the search-alias path, and the cypher empty-statement guard. * fix(review): non-string alias safety + drop stale chokepoint comment (#2175) Tri-review findings (correctness/adversarial/security + maintainability): - Non-string statement/search_query/query (the MCP envelope is not schema-validated) hit .trim() and threw TypeError to the server boundary instead of a friendly required-param error. Introduce resolveAliasString() (new name wins; non-string -> undefined) used by query(), cypher(), and the group-forward, so all three return the structured error. Empirically verified (123 ?? '' -> 123, (123).trim() throws) — this overrides a critic refutation that mis-read ?? as a string coercion. - Remove the stale query() comment claiming alias resolution happens at a callTool chokepoint; that mutation was removed earlier in this PR — each site resolves the alias itself. - Document GroupToolPort.query's intentionally-narrower required type vs the wider LocalBackend impl. Adds non-string and empty-new-key precedence tests. * fix(mcp): alias falls back to legacy value when new key is blank (#2175) PR #2186 review finding: resolveAliasString used `canonical ?? legacy` (nullish), so an explicitly empty/whitespace new-name value (e.g. {search_query:'', query:'real'}) won and was rejected — discarding a valid legacy value, contradicting the 'new name wins when both supplied' intent. Resolve to the first NON-BLANK string instead (new preferred when it carries a real value, else legacy). Covers query(), cypher(), and the group-forward (all route through the helper); non-string still resolves to a friendly error. Flips the presence-based test and adds whitespace/cypher/group fallback cases. * fix(mcp): drop legacy "query" mention from query/cypher schema descriptions (#2175) PR #2186 review finding: the search_query/statement inputSchema descriptions named the legacy "query" key — the exact arg Claude Code drops — and description text is read by an LLM choosing arguments, weakly nudging it to send "query". Trim the descriptions to their clean form and move the legacy-alias note to a code comment next to the schema (preserved for maintainers / non-CC clients). properties/required unchanged (no `query`).
396 lines
14 KiB
TypeScript
396 lines
14 KiB
TypeScript
/**
|
|
* Unit Tests: MCP Resources
|
|
*
|
|
* Tests: getResourceDefinitions, getResourceTemplates, readResource
|
|
* - Static resource definitions
|
|
* - Dynamic resource templates
|
|
* - URI parsing and dispatch
|
|
* - Error handling for invalid URIs
|
|
* - Resource handlers with mocked backend
|
|
*/
|
|
import { describe, it, expect, vi } from 'vitest';
|
|
import {
|
|
getResourceDefinitions,
|
|
getResourceTemplates,
|
|
parseResourceUri,
|
|
readResource,
|
|
} from '../../src/mcp/resources.js';
|
|
|
|
// ─── Minimal mock backend ──────────────────────────────────────────
|
|
|
|
function createMockBackend(overrides: Partial<Record<string, any>> = {}): any {
|
|
return {
|
|
listRepos: vi.fn().mockResolvedValue(overrides.repos ?? []),
|
|
resolveRepo: vi.fn().mockResolvedValue(
|
|
overrides.resolvedRepo ?? {
|
|
name: 'test-repo',
|
|
repoPath: '/tmp/test-repo',
|
|
lastCommit: 'abc1234',
|
|
},
|
|
),
|
|
getContext: vi.fn().mockReturnValue(overrides.context ?? null),
|
|
queryClusters: vi.fn().mockResolvedValue(overrides.clusters ?? { clusters: [] }),
|
|
queryProcesses: vi.fn().mockResolvedValue(overrides.processes ?? { processes: [] }),
|
|
queryClusterDetail: vi
|
|
.fn()
|
|
.mockResolvedValue(overrides.clusterDetail ?? { error: 'Not found' }),
|
|
queryProcessDetail: vi
|
|
.fn()
|
|
.mockResolvedValue(overrides.processDetail ?? { error: 'Not found' }),
|
|
readGroupContractsResource: vi
|
|
.fn()
|
|
.mockResolvedValue(overrides.groupContractsBody ?? 'contracts: []\n'),
|
|
readGroupStatusResource: vi
|
|
.fn()
|
|
.mockResolvedValue(overrides.groupStatusBody ?? 'group: mock\n'),
|
|
...overrides,
|
|
};
|
|
}
|
|
|
|
// ─── Static definitions ─────────────────────────────────────────────
|
|
|
|
describe('getResourceDefinitions', () => {
|
|
it('returns 2 static resources', () => {
|
|
const defs = getResourceDefinitions();
|
|
expect(defs).toHaveLength(2);
|
|
});
|
|
|
|
it('includes repos resource', () => {
|
|
const defs = getResourceDefinitions();
|
|
const repos = defs.find((d) => d.uri === 'gitnexus://repos');
|
|
expect(repos).toBeDefined();
|
|
expect(repos!.mimeType).toBe('text/yaml');
|
|
});
|
|
|
|
it('includes setup resource', () => {
|
|
const defs = getResourceDefinitions();
|
|
const setup = defs.find((d) => d.uri === 'gitnexus://setup');
|
|
expect(setup).toBeDefined();
|
|
expect(setup!.mimeType).toBe('text/markdown');
|
|
});
|
|
|
|
it('each definition has uri, name, description, mimeType', () => {
|
|
for (const def of getResourceDefinitions()) {
|
|
expect(def.uri).toBeTruthy();
|
|
expect(def.name).toBeTruthy();
|
|
expect(def.description).toBeTruthy();
|
|
expect(def.mimeType).toBeTruthy();
|
|
}
|
|
});
|
|
});
|
|
|
|
describe('getResourceTemplates', () => {
|
|
it('returns 8 dynamic templates', () => {
|
|
const templates = getResourceTemplates();
|
|
expect(templates).toHaveLength(8);
|
|
});
|
|
|
|
it('includes context, clusters, processes, schema, cluster detail, process detail, group contracts/status', () => {
|
|
const templates = getResourceTemplates();
|
|
const uris = templates.map((t) => t.uriTemplate);
|
|
expect(uris).toContain('gitnexus://repo/{name}/context');
|
|
expect(uris).toContain('gitnexus://repo/{name}/clusters');
|
|
expect(uris).toContain('gitnexus://repo/{name}/processes');
|
|
expect(uris).toContain('gitnexus://repo/{name}/schema');
|
|
expect(uris).toContain('gitnexus://repo/{name}/cluster/{clusterName}');
|
|
expect(uris).toContain('gitnexus://repo/{name}/process/{processName}');
|
|
expect(uris).toContain('gitnexus://group/{name}/contracts');
|
|
expect(uris).toContain('gitnexus://group/{name}/status');
|
|
});
|
|
|
|
it('each template has uriTemplate, name, description, mimeType', () => {
|
|
for (const tmpl of getResourceTemplates()) {
|
|
expect(tmpl.uriTemplate).toBeTruthy();
|
|
expect(tmpl.name).toBeTruthy();
|
|
expect(tmpl.description).toBeTruthy();
|
|
expect(tmpl.mimeType).toBeTruthy();
|
|
}
|
|
});
|
|
});
|
|
|
|
describe('parseResourceUri', () => {
|
|
it('parses group contracts without query', () => {
|
|
const p = parseResourceUri('gitnexus://group/acme/contracts');
|
|
expect(p).toEqual({
|
|
kind: 'group',
|
|
groupName: 'acme',
|
|
resourceType: 'contracts',
|
|
contractsFilter: {},
|
|
});
|
|
});
|
|
|
|
it('parses nested group name and contracts query params', () => {
|
|
const p = parseResourceUri(
|
|
'gitnexus://group/acme/billing/contracts?type=http&repo=app%2Fapi&unmatchedOnly=true',
|
|
);
|
|
expect(p.kind).toBe('group');
|
|
if (p.kind !== 'group' || p.resourceType !== 'contracts') throw new Error('unexpected');
|
|
expect(p.groupName).toBe('acme/billing');
|
|
expect(p.contractsFilter).toEqual({
|
|
type: 'http',
|
|
repo: 'app/api',
|
|
unmatchedOnly: true,
|
|
});
|
|
});
|
|
|
|
it('coerces unmatchedOnly false from string', () => {
|
|
const p = parseResourceUri('gitnexus://group/g1/contracts?unmatchedOnly=false');
|
|
expect(p.kind).toBe('group');
|
|
if (p.kind !== 'group' || p.resourceType !== 'contracts') throw new Error('unexpected');
|
|
expect(p.contractsFilter.unmatchedOnly).toBe(false);
|
|
});
|
|
|
|
it('parses group status', () => {
|
|
const p = parseResourceUri('gitnexus://group/my/product/status');
|
|
expect(p).toEqual({
|
|
kind: 'group',
|
|
groupName: 'my/product',
|
|
resourceType: 'status',
|
|
});
|
|
});
|
|
|
|
it('round-trips repo URI like legacy regex', () => {
|
|
const p = parseResourceUri('gitnexus://repo/my%20project/schema');
|
|
expect(p).toEqual({
|
|
kind: 'repo',
|
|
repoName: 'my project',
|
|
resourceType: 'schema',
|
|
});
|
|
});
|
|
|
|
it('rejects unknown group resource tail', () => {
|
|
expect(() => parseResourceUri('gitnexus://group/foo/bar')).toThrow('Unknown group resource');
|
|
});
|
|
});
|
|
|
|
// ─── readResource URI parsing ────────────────────────────────────────
|
|
|
|
describe('readResource', () => {
|
|
it('routes gitnexus://repos to listRepos', async () => {
|
|
const backend = createMockBackend({
|
|
repos: [
|
|
{
|
|
name: 'my-project',
|
|
path: '/home/me/my-project',
|
|
indexedAt: '2024-01-01',
|
|
lastCommit: 'abc1234',
|
|
stats: { files: 10, nodes: 50, processes: 5 },
|
|
},
|
|
],
|
|
});
|
|
|
|
const result = await readResource('gitnexus://repos', backend);
|
|
expect(backend.listRepos).toHaveBeenCalled();
|
|
expect(result).toContain('my-project');
|
|
});
|
|
|
|
it('returns empty message when no repos', async () => {
|
|
const backend = createMockBackend({ repos: [] });
|
|
const result = await readResource('gitnexus://repos', backend);
|
|
expect(result).toContain('No repositories indexed');
|
|
});
|
|
|
|
it('routes gitnexus://setup to setup resource', async () => {
|
|
const backend = createMockBackend({
|
|
repos: [
|
|
{
|
|
name: 'proj',
|
|
path: '/tmp/proj',
|
|
indexedAt: '2024-01-01',
|
|
lastCommit: 'abc',
|
|
stats: { nodes: 10, edges: 20, processes: 3 },
|
|
},
|
|
],
|
|
});
|
|
const result = await readResource('gitnexus://setup', backend);
|
|
expect(result).toContain('GitNexus MCP');
|
|
expect(result).toContain('proj');
|
|
});
|
|
|
|
it('returns fallback when setup has no repos', async () => {
|
|
const backend = createMockBackend({ repos: [] });
|
|
const result = await readResource('gitnexus://setup', backend);
|
|
expect(result).toContain('No repositories indexed');
|
|
});
|
|
|
|
it('routes group contracts resource through backend', async () => {
|
|
const backend = createMockBackend();
|
|
const uri = 'gitnexus://group/g1/contracts?type=http&unmatchedOnly=true';
|
|
await readResource(uri, backend);
|
|
expect(backend.readGroupContractsResource).toHaveBeenCalledWith('g1', {
|
|
type: 'http',
|
|
unmatchedOnly: true,
|
|
});
|
|
});
|
|
|
|
it('routes group status resource through backend', async () => {
|
|
const backend = createMockBackend();
|
|
await readResource('gitnexus://group/acme/status', backend);
|
|
expect(backend.readGroupStatusResource).toHaveBeenCalledWith('acme');
|
|
});
|
|
|
|
it('routes gitnexus://repo/{name}/context correctly', async () => {
|
|
const backend = createMockBackend({
|
|
context: {
|
|
projectName: 'test-project',
|
|
stats: { fileCount: 10, functionCount: 50, communityCount: 3, processCount: 5 },
|
|
},
|
|
});
|
|
|
|
const result = await readResource('gitnexus://repo/test-project/context', backend);
|
|
expect(backend.resolveRepo).toHaveBeenCalledWith('test-project');
|
|
expect(result).toContain('test-project');
|
|
expect(result).toContain('files: 10');
|
|
});
|
|
|
|
it('returns error when context has no codebase loaded', async () => {
|
|
const backend = createMockBackend({ context: null });
|
|
const result = await readResource('gitnexus://repo/test-project/context', backend);
|
|
expect(result).toContain('error');
|
|
});
|
|
|
|
it('routes gitnexus://repo/{name}/schema to static schema', async () => {
|
|
const backend = createMockBackend();
|
|
const result = await readResource('gitnexus://repo/any/schema', backend);
|
|
expect(result).toContain('GitNexus Graph Schema');
|
|
expect(result).toContain('CALLS');
|
|
expect(result).toContain('IMPORTS');
|
|
});
|
|
|
|
it('routes gitnexus://repo/{name}/clusters correctly', async () => {
|
|
const backend = createMockBackend({
|
|
clusters: {
|
|
clusters: [{ heuristicLabel: 'Auth', symbolCount: 10, cohesion: 0.9 }],
|
|
},
|
|
});
|
|
const result = await readResource('gitnexus://repo/test/clusters', backend);
|
|
expect(backend.queryClusters).toHaveBeenCalledWith('test', 100);
|
|
expect(result).toContain('Auth');
|
|
});
|
|
|
|
it('returns empty modules when no clusters', async () => {
|
|
const backend = createMockBackend({ clusters: { clusters: [] } });
|
|
const result = await readResource('gitnexus://repo/test/clusters', backend);
|
|
expect(result).toContain('modules: []');
|
|
});
|
|
|
|
it('handles cluster query error gracefully', async () => {
|
|
const backend = createMockBackend();
|
|
backend.queryClusters = vi.fn().mockRejectedValue(new Error('DB locked'));
|
|
const result = await readResource('gitnexus://repo/test/clusters', backend);
|
|
expect(result).toContain('DB locked');
|
|
});
|
|
|
|
it('routes gitnexus://repo/{name}/processes correctly', async () => {
|
|
const backend = createMockBackend({
|
|
processes: {
|
|
processes: [{ heuristicLabel: 'LoginFlow', processType: 'intra_community', stepCount: 3 }],
|
|
},
|
|
});
|
|
const result = await readResource('gitnexus://repo/test/processes', backend);
|
|
expect(backend.queryProcesses).toHaveBeenCalledWith('test', 50);
|
|
expect(result).toContain('LoginFlow');
|
|
});
|
|
|
|
it('handles process query error gracefully', async () => {
|
|
const backend = createMockBackend();
|
|
backend.queryProcesses = vi.fn().mockRejectedValue(new Error('timeout'));
|
|
const result = await readResource('gitnexus://repo/test/processes', backend);
|
|
expect(result).toContain('timeout');
|
|
});
|
|
|
|
it('routes gitnexus://repo/{name}/cluster/{clusterName} correctly', async () => {
|
|
const backend = createMockBackend({
|
|
clusterDetail: {
|
|
cluster: { heuristicLabel: 'Auth', symbolCount: 5, cohesion: 0.85 },
|
|
members: [{ name: 'login', type: 'Function', filePath: 'src/auth.ts' }],
|
|
},
|
|
});
|
|
const result = await readResource('gitnexus://repo/test/cluster/Auth', backend);
|
|
expect(backend.queryClusterDetail).toHaveBeenCalledWith('Auth', 'test');
|
|
expect(result).toContain('Auth');
|
|
expect(result).toContain('login');
|
|
});
|
|
|
|
it('handles cluster detail error', async () => {
|
|
const backend = createMockBackend({
|
|
clusterDetail: { error: 'Cluster not found' },
|
|
});
|
|
const result = await readResource('gitnexus://repo/test/cluster/Missing', backend);
|
|
expect(result).toContain('Cluster not found');
|
|
});
|
|
|
|
it('routes gitnexus://repo/{name}/process/{processName} correctly', async () => {
|
|
const backend = createMockBackend({
|
|
processDetail: {
|
|
process: { heuristicLabel: 'LoginFlow', processType: 'intra_community', stepCount: 3 },
|
|
steps: [
|
|
{ step: 1, name: 'login', filePath: 'src/auth.ts' },
|
|
{ step: 2, name: 'validate', filePath: 'src/validate.ts' },
|
|
],
|
|
},
|
|
});
|
|
const result = await readResource('gitnexus://repo/test/process/LoginFlow', backend);
|
|
expect(backend.queryProcessDetail).toHaveBeenCalledWith('LoginFlow', 'test');
|
|
expect(result).toContain('LoginFlow');
|
|
expect(result).toContain('login');
|
|
expect(result).toContain('validate');
|
|
});
|
|
|
|
it('handles process detail error', async () => {
|
|
const backend = createMockBackend({
|
|
processDetail: { error: 'Process not found' },
|
|
});
|
|
const result = await readResource('gitnexus://repo/test/process/Missing', backend);
|
|
expect(result).toContain('Process not found');
|
|
});
|
|
|
|
it('throws for unknown resource URI', async () => {
|
|
const backend = createMockBackend();
|
|
await expect(readResource('gitnexus://unknown', backend)).rejects.toThrow(
|
|
'Unknown resource URI',
|
|
);
|
|
});
|
|
|
|
it('throws for unknown repo-scoped resource type', async () => {
|
|
const backend = createMockBackend();
|
|
await expect(readResource('gitnexus://repo/test/nonexistent', backend)).rejects.toThrow(
|
|
'Unknown resource',
|
|
);
|
|
});
|
|
|
|
it('decodes URI-encoded repo names', async () => {
|
|
const backend = createMockBackend();
|
|
await readResource('gitnexus://repo/my%20project/schema', backend);
|
|
// Should not throw — the schema resource is static
|
|
});
|
|
|
|
it('decodes URI-encoded cluster names', async () => {
|
|
const backend = createMockBackend({
|
|
clusterDetail: {
|
|
cluster: { heuristicLabel: 'Auth Module', symbolCount: 5 },
|
|
members: [],
|
|
},
|
|
});
|
|
await readResource('gitnexus://repo/test/cluster/Auth%20Module', backend);
|
|
expect(backend.queryClusterDetail).toHaveBeenCalledWith('Auth Module', 'test');
|
|
});
|
|
|
|
it('repos resource shows multi-repo hint for multiple repos', async () => {
|
|
const backend = createMockBackend({
|
|
repos: [
|
|
{ name: 'proj-a', path: '/a', indexedAt: '2024-01-01', lastCommit: 'abc' },
|
|
{ name: 'proj-b', path: '/b', indexedAt: '2024-01-02', lastCommit: 'def' },
|
|
],
|
|
});
|
|
const result = await readResource('gitnexus://repos', backend);
|
|
expect(result).toContain('Multiple repos indexed');
|
|
expect(result).toContain('repo parameter');
|
|
// The example must use a registered tool name, not the unregistered
|
|
// `gitnexus_search` / `gitnexus_*` prefix (#2059).
|
|
// #2175: advertise the renamed param, not the legacy "query" key.
|
|
expect(result).toContain('query({search_query: "auth"');
|
|
expect(result).not.toContain('query({query:');
|
|
expect(result).not.toMatch(/gitnexus_/);
|
|
});
|
|
});
|