mirror of
https://github.com/abhigyanpatwari/GitNexus.git
synced 2026-10-06 02:49:56 +00:00
fix(group-mcp): address cross-AI review for @repo routing
- Add subgroupExact to groupQuery/groupContext so @group/member does not prefix-match nested member paths (Codex/OpenCode HIGH). - Build explicit param payloads in callToolAtGroupRepo; map context tool symbol name to target so group name is not overwritten (correctness). - Narrow MCP tool copy: service applies in @ group mode only; align text. - Refresh ai-context cross-repo section (remove retired group_* tools). - Extend unit tests for exact filter, routing errors, removed tools, MCP name. Made-with: Cursor
This commit is contained in:
parent
9e782a22ba
commit
b18d2ecae6
6 changed files with 169 additions and 25 deletions
|
|
@ -179,7 +179,7 @@ ${
|
|||
groupNames && groupNames.length > 0
|
||||
? `## Cross-Repo Groups
|
||||
|
||||
This repository is listed under GitNexus **group(s): ${groupNames.join(', ')}** (see \`~/.gitnexus/groups/\`). For blast radius across repository boundaries, use MCP tools \`group_impact\`, \`group_sync\`, \`group_query\`, \`group_contracts\`, \`group_status\`, and \`group_list\`. From the terminal: \`npx gitnexus group list\`, \`npx gitnexus group sync <name>\`, \`npx gitnexus group impact <name> --target <symbol> --repo <group-path>\`.
|
||||
This repository is listed under GitNexus **group(s): ${groupNames.join(', ')}** (see \`~/.gitnexus/groups/\`). For cross-repo analysis, use MCP tools \`impact\`, \`query\`, and \`context\` with \`repo\` set to \`@<groupName>\` or \`@<groupName>/<memberPath>\` (paths match keys in that group’s \`group.yaml\`). Use \`group_list\` / \`group_sync\` for membership and sync. From the terminal: \`npx gitnexus group list\`, \`npx gitnexus group sync <name>\`, \`npx gitnexus group impact <name> --target <symbol> --repo <group-path>\`.
|
||||
|
||||
`
|
||||
: ''
|
||||
|
|
|
|||
|
|
@ -78,9 +78,10 @@ export interface GroupToolPort {
|
|||
): Promise<unknown>;
|
||||
}
|
||||
|
||||
function repoInSubgroup(repoPath: string, subgroup?: string): boolean {
|
||||
function repoInSubgroup(repoPath: string, subgroup?: string, exact?: boolean): boolean {
|
||||
if (!subgroup?.trim()) return true;
|
||||
const s = subgroup.replace(/\/+$/, '');
|
||||
if (exact) return repoPath === s;
|
||||
return repoPath === s || repoPath.startsWith(`${s}/`);
|
||||
}
|
||||
|
||||
|
|
@ -299,6 +300,7 @@ export class GroupService {
|
|||
}
|
||||
const servicePrefix = normalizeServicePrefix(params.service);
|
||||
const subgroup = typeof params.subgroup === 'string' ? params.subgroup : undefined;
|
||||
const subgroupExact = params.subgroupExact === true;
|
||||
|
||||
if (!name) {
|
||||
return { group: '', error: 'name is required', results: [] };
|
||||
|
|
@ -324,7 +326,7 @@ export class GroupService {
|
|||
const results: GroupContextResult['results'] = [];
|
||||
|
||||
for (const [repoPath, registryName] of Object.entries(config.repos)) {
|
||||
if (!repoInSubgroup(repoPath, subgroup)) continue;
|
||||
if (!repoInSubgroup(repoPath, subgroup, subgroupExact)) continue;
|
||||
try {
|
||||
const repoObj = await this.port.resolveRepo(registryName);
|
||||
const payload = await this.port.context(repoObj, {
|
||||
|
|
@ -372,12 +374,13 @@ export class GroupService {
|
|||
|
||||
const limit = typeof params.limit === 'number' && params.limit > 0 ? params.limit : 5;
|
||||
const subgroup = typeof params.subgroup === 'string' ? params.subgroup : undefined;
|
||||
const subgroupExact = params.subgroupExact === true;
|
||||
const groupDir = getGroupDir(getDefaultGitnexusDir(), name);
|
||||
const config = await loadGroupConfig(groupDir);
|
||||
|
||||
const perRepo: Array<{ repo: string; score: number; processes: unknown[] }> = [];
|
||||
for (const [repoPath, registryName] of Object.entries(config.repos)) {
|
||||
if (!repoInSubgroup(repoPath, subgroup)) continue;
|
||||
if (!repoInSubgroup(repoPath, subgroup, subgroupExact)) continue;
|
||||
try {
|
||||
const repoObj = await this.port.resolveRepo(registryName);
|
||||
const queryResult = (await this.port.query(repoObj, {
|
||||
|
|
|
|||
|
|
@ -2592,27 +2592,60 @@ export class LocalBackend {
|
|||
|
||||
const svc = this.getGroupService();
|
||||
if (method === 'impact') {
|
||||
return svc.groupImpact({
|
||||
...params,
|
||||
const impactArgs: Record<string, unknown> = {
|
||||
name: groupName,
|
||||
repo: resolved.repoPath,
|
||||
});
|
||||
target: params.target,
|
||||
direction: params.direction,
|
||||
};
|
||||
if (params.maxDepth !== undefined) impactArgs.maxDepth = params.maxDepth;
|
||||
if (params.crossDepth !== undefined) impactArgs.crossDepth = params.crossDepth;
|
||||
if (params.relationTypes !== undefined) impactArgs.relationTypes = params.relationTypes;
|
||||
if (params.includeTests !== undefined) impactArgs.includeTests = params.includeTests;
|
||||
if (params.minConfidence !== undefined) impactArgs.minConfidence = params.minConfidence;
|
||||
if (params.service !== undefined && params.service !== null) impactArgs.service = params.service;
|
||||
if (typeof params.subgroup === 'string') impactArgs.subgroup = params.subgroup;
|
||||
if (params.timeoutMs !== undefined) impactArgs.timeoutMs = params.timeoutMs;
|
||||
if (params.timeout !== undefined) impactArgs.timeout = params.timeout;
|
||||
return svc.groupImpact(impactArgs);
|
||||
}
|
||||
if (method === 'query') {
|
||||
const { repo: _r, ...rest } = params;
|
||||
return svc.groupQuery({
|
||||
...rest,
|
||||
const queryArgs: Record<string, unknown> = {
|
||||
name: groupName,
|
||||
...(memberRest ? { subgroup: memberRest } : {}),
|
||||
});
|
||||
query: params.query,
|
||||
};
|
||||
if (typeof params.task_context === 'string') queryArgs.task_context = params.task_context;
|
||||
if (typeof params.goal === 'string') queryArgs.goal = params.goal;
|
||||
if (typeof params.limit === 'number') queryArgs.limit = params.limit;
|
||||
if (typeof params.max_symbols === 'number') queryArgs.max_symbols = params.max_symbols;
|
||||
if (params.include_content !== undefined) queryArgs.include_content = params.include_content;
|
||||
if (params.service !== undefined && params.service !== null) queryArgs.service = params.service;
|
||||
if (memberRest !== undefined) {
|
||||
queryArgs.subgroup = memberRest;
|
||||
queryArgs.subgroupExact = true;
|
||||
}
|
||||
return svc.groupQuery(queryArgs);
|
||||
}
|
||||
if (method === 'context') {
|
||||
const { repo: _r, ...rest } = params;
|
||||
return svc.groupContext({
|
||||
...rest,
|
||||
const targetSym =
|
||||
typeof params.target === 'string' && params.target.trim() !== ''
|
||||
? params.target.trim()
|
||||
: typeof params.name === 'string' && params.name.trim() !== ''
|
||||
? params.name.trim()
|
||||
: undefined;
|
||||
const contextArgs: Record<string, unknown> = {
|
||||
name: groupName,
|
||||
...(memberRest ? { subgroup: memberRest } : {}),
|
||||
});
|
||||
target: targetSym,
|
||||
};
|
||||
if (typeof params.uid === 'string') contextArgs.uid = params.uid;
|
||||
if (typeof params.file_path === 'string') contextArgs.file_path = params.file_path;
|
||||
if (params.include_content !== undefined) contextArgs.include_content = params.include_content;
|
||||
if (params.service !== undefined && params.service !== null) contextArgs.service = params.service;
|
||||
if (memberRest !== undefined) {
|
||||
contextArgs.subgroup = memberRest;
|
||||
contextArgs.subgroupExact = true;
|
||||
}
|
||||
return svc.groupContext(contextArgs);
|
||||
}
|
||||
throw new Error(`Internal: unsupported group-repo tool ${method}`);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -62,7 +62,7 @@ Hybrid ranking: BM25 keyword + semantic vector search, ranked by Reciprocal Rank
|
|||
|
||||
GROUP MODE: set "repo" to "@<groupName>" to search all member repos in that group (merged via RRF), or "@<groupName>/<groupRepoPath>" to run against a single member (same path keys as in group.yaml). If you use "@<groupName>" only, the member repo defaults to the lexicographically first key in group.yaml "repos". Prefer resources for contracts/status (see migration from legacy group_* tools).
|
||||
|
||||
SERVICE (group or single repo): optional monorepo path prefix (POSIX-style, case-sensitive segments). When set, only processes whose symbols have file paths under that prefix are included.`,
|
||||
SERVICE: optional monorepo path prefix (POSIX-style, case-sensitive segments). When "repo" starts with "@", only processes whose symbols fall under that prefix are included. For a normal indexed repo name (no leading @), this field is currently ignored by the server.`,
|
||||
inputSchema: {
|
||||
type: 'object',
|
||||
properties: {
|
||||
|
|
@ -104,7 +104,7 @@ SERVICE (group or single repo): optional monorepo path prefix (POSIX-style, case
|
|||
type: 'string',
|
||||
minLength: 1,
|
||||
description:
|
||||
'Optional monorepo service root (relative path, "/" separators). Prefix-matches symbol file paths; omit for full repo/group scope. Empty string is rejected server-side.',
|
||||
'Optional monorepo service root (relative path, "/" separators). In group mode (@repo), prefix-matches symbol file paths; ignored for a normal repo name. Empty string is rejected server-side.',
|
||||
},
|
||||
},
|
||||
required: ['query'],
|
||||
|
|
@ -182,7 +182,7 @@ NOTE: ACCESSES edges (field read/write tracking) are included in context results
|
|||
|
||||
GROUP MODE: set "repo" to "@<groupName>" to run context in each member repo (aggregated list), or "@<groupName>/<groupRepoPath>" for one member. If you use "@<groupName>" only, the member defaults to the lexicographically first key in group.yaml "repos".
|
||||
|
||||
SERVICE: optional monorepo path prefix (case-sensitive path segments). Prefix-matches resolved symbol file paths; when a hit is outside the prefix, that member returns an empty payload for the symbol (no extra filter on unrelated repos).`,
|
||||
SERVICE: optional monorepo path prefix (case-sensitive path segments). When "repo" starts with "@", prefix-matches resolved symbol file paths; when a hit is outside the prefix, that member returns an empty payload for the symbol. Ignored for a normal indexed repo name.`,
|
||||
inputSchema: {
|
||||
type: 'object',
|
||||
properties: {
|
||||
|
|
@ -206,7 +206,7 @@ SERVICE: optional monorepo path prefix (case-sensitive path segments). Prefix-ma
|
|||
type: 'string',
|
||||
minLength: 1,
|
||||
description:
|
||||
'Optional monorepo service root (relative path). Prefix-matches symbol file paths in group mode. Empty string is rejected server-side.',
|
||||
'Optional monorepo service root (relative path). Applies in group mode (@repo) only; ignored for a normal repo name. Empty string is rejected server-side.',
|
||||
},
|
||||
},
|
||||
required: [],
|
||||
|
|
@ -303,7 +303,7 @@ Confidence: 1.0 = certain, <0.8 = fuzzy match
|
|||
|
||||
GROUP MODE: set "repo" to "@<groupName>" for cross-repo impact anchored at the default member (lexicographically first key in group.yaml "repos"), or "@<groupName>/<groupRepoPath>" to choose the member (same path keys as in group.yaml). Phase-1 walk runs in that member; cross-boundary fan-out uses the group bridge.
|
||||
|
||||
SERVICE: optional monorepo path prefix (case-sensitive path segments). Scopes the local impact walk and cross-repo symbol paths to files under that prefix; omit for full member repo.`,
|
||||
SERVICE: optional monorepo path prefix (case-sensitive path segments). When "repo" starts with "@", scopes the local impact walk and cross-repo symbol paths to files under that prefix; ignored for a normal indexed repo name.`,
|
||||
inputSchema: {
|
||||
type: 'object',
|
||||
properties: {
|
||||
|
|
@ -350,7 +350,7 @@ SERVICE: optional monorepo path prefix (case-sensitive path segments). Scopes th
|
|||
type: 'string',
|
||||
minLength: 1,
|
||||
description:
|
||||
'Optional monorepo service root (relative path). Prefix-matches file paths for impact traversal and cross hits. Empty string is rejected server-side.',
|
||||
'Optional monorepo service root (relative path). Applies when "repo" is group mode (@…); ignored for a normal repo name. Empty string is rejected server-side.',
|
||||
},
|
||||
subgroup: {
|
||||
type: 'string',
|
||||
|
|
|
|||
|
|
@ -372,6 +372,47 @@ describe('GroupService', () => {
|
|||
cleanup();
|
||||
}
|
||||
});
|
||||
|
||||
it('test_groupQuery_subgroupExact_skips_descendant_member_paths', async () => {
|
||||
const tmpDir = path.join(os.tmpdir(), `gitnexus-svc-nest-${Date.now()}`);
|
||||
const groupDir = path.join(tmpDir, 'groups', 'nest-group');
|
||||
fs.mkdirSync(groupDir, { recursive: true });
|
||||
fs.writeFileSync(
|
||||
path.join(groupDir, 'group.yaml'),
|
||||
`version: 1
|
||||
name: nest-group
|
||||
repos:
|
||||
app/frontend: fe-root
|
||||
app/frontend/mobile: fe-nested
|
||||
app/backend: be1
|
||||
`,
|
||||
);
|
||||
try {
|
||||
vi.stubEnv('GITNEXUS_HOME', tmpDir);
|
||||
const query = vi.fn(async () => ({ processes: [{ name: 'p1' }] }));
|
||||
const port = makePort({ query });
|
||||
const svc = new GroupService(port);
|
||||
|
||||
const prefixOnly = (await svc.groupQuery({
|
||||
name: 'nest-group',
|
||||
query: 'x',
|
||||
subgroup: 'app/frontend',
|
||||
})) as { per_repo: Array<{ repo: string }> };
|
||||
expect(prefixOnly.per_repo.map((r) => r.repo).sort()).toEqual(['app/frontend', 'app/frontend/mobile']);
|
||||
|
||||
const exact = (await svc.groupQuery({
|
||||
name: 'nest-group',
|
||||
query: 'x',
|
||||
subgroup: 'app/frontend',
|
||||
subgroupExact: true,
|
||||
})) as { per_repo: Array<{ repo: string }> };
|
||||
expect(exact.per_repo).toHaveLength(1);
|
||||
expect(exact.per_repo[0].repo).toBe('app/frontend');
|
||||
} finally {
|
||||
vi.unstubAllEnvs();
|
||||
fs.rmSync(tmpDir, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
describe('groupImpact', () => {
|
||||
|
|
@ -405,6 +446,36 @@ describe('GroupService', () => {
|
|||
}
|
||||
});
|
||||
|
||||
it('test_groupContext_subgroupExact_skips_descendant_member_paths', async () => {
|
||||
const tmpDir = path.join(os.tmpdir(), `gitnexus-ctx-nest-${Date.now()}`);
|
||||
const groupDir = path.join(tmpDir, 'groups', 'nest-group');
|
||||
fs.mkdirSync(groupDir, { recursive: true });
|
||||
fs.writeFileSync(
|
||||
path.join(groupDir, 'group.yaml'),
|
||||
`version: 1
|
||||
name: nest-group
|
||||
repos:
|
||||
app/frontend: fe-root
|
||||
app/frontend/mobile: fe-nested
|
||||
`,
|
||||
);
|
||||
try {
|
||||
vi.stubEnv('GITNEXUS_HOME', tmpDir);
|
||||
const port = makePort();
|
||||
const svc = new GroupService(port);
|
||||
await svc.groupContext({
|
||||
name: 'nest-group',
|
||||
target: 'X',
|
||||
subgroup: 'app/frontend',
|
||||
subgroupExact: true,
|
||||
});
|
||||
expect(port.context).toHaveBeenCalledTimes(1);
|
||||
} finally {
|
||||
vi.unstubAllEnvs();
|
||||
fs.rmSync(tmpDir, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
it('test_groupContext_service_prefix_filters_payload', async () => {
|
||||
const { cleanup, tmpDir } = makeTmpGroup();
|
||||
try {
|
||||
|
|
|
|||
|
|
@ -88,11 +88,16 @@ repos:
|
|||
expect(arg).not.toHaveProperty('repo');
|
||||
});
|
||||
|
||||
it('routes query with explicit member path after slash as subgroup', async () => {
|
||||
it('routes query with explicit member path as exact subgroup (no descendant repo bleed)', async () => {
|
||||
const backend = new LocalBackend();
|
||||
await backend.callTool('query', { repo: '@g1/app/frontend', query: 'x' });
|
||||
expect(groupSpyQuery).toHaveBeenCalledWith(
|
||||
expect.objectContaining({ name: 'g1', query: 'x', subgroup: 'app/frontend' }),
|
||||
expect.objectContaining({
|
||||
name: 'g1',
|
||||
query: 'x',
|
||||
subgroup: 'app/frontend',
|
||||
subgroupExact: true,
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
|
|
@ -122,6 +127,28 @@ repos:
|
|||
);
|
||||
});
|
||||
|
||||
it('maps MCP symbol name to groupContext target (does not overwrite group name)', async () => {
|
||||
const backend = new LocalBackend();
|
||||
await backend.callTool('context', { repo: '@g1', name: 'MyClass' });
|
||||
expect(groupSpyContext).toHaveBeenCalledWith(
|
||||
expect.objectContaining({ name: 'g1', target: 'MyClass' }),
|
||||
);
|
||||
});
|
||||
|
||||
it('returns error for unknown group name', async () => {
|
||||
const backend = new LocalBackend();
|
||||
const out = await backend.callTool('query', { repo: '@no-such-group', query: 'x' });
|
||||
expect(out).toHaveProperty('error');
|
||||
expect(String((out as { error: string }).error)).toMatch(/not found|no such|unknown|exist|ENOENT/i);
|
||||
});
|
||||
|
||||
it('returns error for unknown member path', async () => {
|
||||
const backend = new LocalBackend();
|
||||
const out = await backend.callTool('query', { repo: '@g1/not-a-member', query: 'x' });
|
||||
expect(out).toHaveProperty('error');
|
||||
expect(String((out as { error: string }).error)).toMatch(/Unknown member path/i);
|
||||
});
|
||||
|
||||
it('rejects empty service without calling group tools', async () => {
|
||||
const backend = new LocalBackend();
|
||||
const out = await backend.callTool('query', { repo: '@g1', query: 'x', service: '' });
|
||||
|
|
@ -135,4 +162,14 @@ repos:
|
|||
/Removed tools/,
|
||||
);
|
||||
});
|
||||
|
||||
it('removed group_contracts mentions migration', async () => {
|
||||
const backend = new LocalBackend();
|
||||
await expect(backend.callTool('group_contracts', { name: 'g1' })).rejects.toThrow(/Removed tools/);
|
||||
});
|
||||
|
||||
it('removed group_status mentions migration', async () => {
|
||||
const backend = new LocalBackend();
|
||||
await expect(backend.callTool('group_status', { name: 'g1' })).rejects.toThrow(/Removed tools/);
|
||||
});
|
||||
});
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue