From 9e782a22ba1de8a6eacbeb2a0c7e121c6d480f29 Mon Sep 17 00:00:00 2001 From: ivkond Date: Thu, 16 Apr 2026 01:20:27 +0300 Subject: [PATCH] =?UTF-8?q?feat(mcp):=20Issue=20#794=20phases=203=E2=80=93?= =?UTF-8?q?5=20=E2=80=94=20group=20impact=20CLI,=20@repo=20routing,=20tool?= =?UTF-8?q?=20cleanup?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Phase 3: Add integration tests for groupImpact and CLI group impact; add unit tests for GroupService group-mode API and LocalBackend @group routing. Phase 4: Remove MCP tools group_contracts, group_query, and group_status; extend impact, query, and context schemas and descriptions for @group repo syntax, optional service, and numeric bounds aligned with validation. Phase 5: Route impact/query/context in LocalBackend.callTool when repo starts with @ before resolveRepo; narrow handleGroupTool to group_list and group_sync; add resolveAtGroupMemberRepoPath helper and groupQuery service path filtering. Made-with: Cursor --- gitnexus/src/cli/group.ts | 74 +++++++++ gitnexus/src/core/group/cross-impact.ts | 3 + gitnexus/src/core/group/resolve-at-member.ts | 34 +++++ gitnexus/src/core/group/service.ts | 30 +++- gitnexus/src/mcp/local/local-backend.ts | 89 ++++++++--- gitnexus/src/mcp/tools.ts | 142 +++++++++++------- .../test/integration/group/group-cli.test.ts | 47 ++++++ .../integration/group/group-impact.test.ts | 74 +++++++++ .../group/group-service-group-mode.test.ts | 129 ++++++++++++++++ gitnexus/test/unit/group/group-tools.test.ts | 10 +- .../test/unit/mcp/group-repo-routing.test.ts | 138 +++++++++++++++++ gitnexus/test/unit/tools.test.ts | 42 +++--- 12 files changed, 704 insertions(+), 108 deletions(-) create mode 100644 gitnexus/src/core/group/resolve-at-member.ts create mode 100644 gitnexus/test/integration/group/group-impact.test.ts create mode 100644 gitnexus/test/unit/group/group-service-group-mode.test.ts create mode 100644 gitnexus/test/unit/mcp/group-repo-routing.test.ts diff --git a/gitnexus/src/cli/group.ts b/gitnexus/src/cli/group.ts index 70ca9537a..d9929cc1f 100644 --- a/gitnexus/src/cli/group.ts +++ b/gitnexus/src/cli/group.ts @@ -184,6 +184,80 @@ export function registerGroupCommands(program: Command): void { } }); + group + .command('impact ') + .description('Cross-repo impact for a symbol in one member repo of a group') + .requiredOption('--target ', 'Symbol or file name to analyze') + .requiredOption( + '--repo ', + 'Member path from group.yaml (e.g. app/backend), not the indexed repo name', + ) + .option('--direction ', 'upstream or downstream', 'upstream') + .option('--service ', 'Optional monorepo service directory prefix (path filter)') + .option('--subgroup ', 'Optional prefix limiting which group repos participate in cross fan-out') + .option('--max-depth ', 'Max graph traversal depth') + .option('--cross-depth ', 'Cross-repository hop depth') + .option('--min-confidence ', 'Minimum relation confidence (0–1)') + .option('--include-tests', 'Include test files in traversal', false) + .option('--timeout-ms ', 'Phase-1 local impact wall time in milliseconds') + .option('--json', 'JSON output') + .action(async (name: string, opts: Record) => { + const { LocalBackend } = await import('../mcp/local/local-backend.js'); + + const backend = new LocalBackend(); + try { + await backend.init(); + + const payload: Record = { + name, + repo: opts.repo, + target: opts.target, + direction: (opts.direction as string) || 'upstream', + }; + if (opts.service) payload.service = opts.service; + if (opts.subgroup) payload.subgroup = opts.subgroup; + if (opts.maxDepth !== undefined && opts.maxDepth !== '') { + const n = parseInt(String(opts.maxDepth), 10); + if (!Number.isNaN(n)) payload.maxDepth = n; + } + if (opts.crossDepth !== undefined && opts.crossDepth !== '') { + const n = parseInt(String(opts.crossDepth), 10); + if (!Number.isNaN(n)) payload.crossDepth = n; + } + if (opts.minConfidence !== undefined && opts.minConfidence !== '') { + const n = parseFloat(String(opts.minConfidence)); + if (!Number.isNaN(n)) payload.minConfidence = n; + } + if (opts.timeoutMs !== undefined && opts.timeoutMs !== '') { + const n = parseInt(String(opts.timeoutMs), 10); + if (!Number.isNaN(n)) payload.timeoutMs = n; + } + if (opts.includeTests) payload.includeTests = true; + + const raw = await backend.getGroupService().groupImpact(payload); + if (raw && typeof raw === 'object' && 'error' in raw) { + console.error(String((raw as { error: string }).error)); + process.exitCode = 1; + return; + } + + if (opts.json) { + console.log(JSON.stringify(raw, null, 2)); + } else { + const summary = (raw as { summary?: Record })?.summary; + const risk = (raw as { risk?: string })?.risk; + console.log(`Group impact for "${name}" (${String(opts.repo)}): risk=${risk ?? '?'}`); + if (summary) { + console.log( + ` direct=${summary.direct ?? 0} processes=${summary.processes_affected ?? 0} cross=${summary.cross_repo_hits ?? 0}`, + ); + } + } + } finally { + await backend.dispose().catch(() => {}); + } + }); + group .command('query ') .description('Search execution flows across all repos in a group') diff --git a/gitnexus/src/core/group/cross-impact.ts b/gitnexus/src/core/group/cross-impact.ts index 53c87add6..6b549422d 100644 --- a/gitnexus/src/core/group/cross-impact.ts +++ b/gitnexus/src/core/group/cross-impact.ts @@ -120,6 +120,9 @@ export function validateGroupImpactParams(params: Record): { if (!name) return { ok: false, error: 'name is required' }; if (!repoPath) return { ok: false, error: 'repo is required (group repo path, e.g. app/backend)' }; if (!target) return { ok: false, error: 'target is required' }; + if (params.service !== undefined && params.service !== null && String(params.service).trim() === '') { + return { ok: false, error: 'service must not be an empty string' }; + } const direction = parseDirection(params.direction); if (!direction) return { ok: false, error: 'direction must be upstream or downstream' }; diff --git a/gitnexus/src/core/group/resolve-at-member.ts b/gitnexus/src/core/group/resolve-at-member.ts new file mode 100644 index 000000000..e36506c38 --- /dev/null +++ b/gitnexus/src/core/group/resolve-at-member.ts @@ -0,0 +1,34 @@ +/** + * Map MCP/CLI `@groupName` or `@groupName/memberPath` to a concrete member path in group.yaml. + */ + +import { loadGroupConfig } from './config-parser.js'; +import { getDefaultGitnexusDir, getGroupDir } from './storage.js'; + +export async function resolveAtGroupMemberRepoPath( + groupName: string, + explicitMemberPath: string | undefined, +): Promise<{ ok: true; repoPath: string } | { ok: false; error: string }> { + const trimmed = groupName.trim(); + if (!trimmed) return { ok: false, error: 'Group name is empty.' }; + try { + const groupDir = getGroupDir(getDefaultGitnexusDir(), trimmed); + const config = await loadGroupConfig(groupDir); + const keys = Object.keys(config.repos).sort((a, b) => a.localeCompare(b)); + if (keys.length === 0) { + return { ok: false, error: `Group "${trimmed}" has no repos in group.yaml.` }; + } + if (explicitMemberPath !== undefined && explicitMemberPath !== '') { + if (!(explicitMemberPath in config.repos)) { + return { + ok: false, + error: `Unknown member path "${explicitMemberPath}" in group "${trimmed}". Known paths: ${keys.join(', ')}`, + }; + } + return { ok: true, repoPath: explicitMemberPath }; + } + return { ok: true, repoPath: keys[0]! }; + } catch (e) { + return { ok: false, error: e instanceof Error ? e.message : String(e) }; + } +} diff --git a/gitnexus/src/core/group/service.ts b/gitnexus/src/core/group/service.ts index c4454ba14..fca8f693b 100644 --- a/gitnexus/src/core/group/service.ts +++ b/gitnexus/src/core/group/service.ts @@ -107,6 +107,20 @@ function isStoredContract(raw: unknown): raw is StoredContract { ); } +function filterQueryByServicePrefix( + queryResult: { processes?: Array>; process_symbols?: Array> }, + servicePrefix: string, +): { processes: Array>; process_symbols: Array> } { + const symbols = (queryResult.process_symbols || []).filter((s) => + fileMatchesServicePrefix(typeof s.filePath === 'string' ? s.filePath : undefined, servicePrefix), + ); + const allowed = new Set( + symbols.map((s) => String((s as { process_id?: string }).process_id ?? '')).filter(Boolean), + ); + const processes = (queryResult.processes || []).filter((p) => allowed.has(String(p.id))); + return { processes, process_symbols: symbols }; +} + function isCrossLink(raw: unknown): raw is CrossLink { if (!raw || typeof raw !== 'object') return false; const o = raw as Record; @@ -280,6 +294,9 @@ export class GroupService { const uid = typeof params.uid === 'string' ? params.uid.trim() : undefined; const file_path = typeof params.file_path === 'string' ? params.file_path : undefined; const include_content = Boolean(params.include_content); + if (params.service !== undefined && params.service !== null && String(params.service).trim() === '') { + return { group: name || '', error: 'service must not be an empty string', results: [] }; + } const servicePrefix = normalizeServicePrefix(params.service); const subgroup = typeof params.subgroup === 'string' ? params.subgroup : undefined; @@ -348,6 +365,10 @@ export class GroupService { const name = String(params.name ?? '').trim(); const queryText = String(params.query ?? '').trim(); if (!name || !queryText) return { error: 'name and query are required' }; + if (params.service !== undefined && params.service !== null && String(params.service).trim() === '') { + return { error: 'service must not be an empty string' }; + } + const servicePrefix = normalizeServicePrefix(params.service); const limit = typeof params.limit === 'number' && params.limit > 0 ? params.limit : 5; const subgroup = typeof params.subgroup === 'string' ? params.subgroup : undefined; @@ -364,8 +385,13 @@ export class GroupService { limit, max_symbols: 10, include_content: false, - })) as { processes?: Array> }; - const processes = queryResult.processes || []; + })) as { + processes?: Array>; + process_symbols?: Array>; + }; + const processes = servicePrefix + ? filterQueryByServicePrefix(queryResult, servicePrefix).processes + : queryResult.processes || []; const scored = processes.map((p, idx) => ({ ...p, _rrf_score: 1 / (idx + 1 + 60), diff --git a/gitnexus/src/mcp/local/local-backend.ts b/gitnexus/src/mcp/local/local-backend.ts index 2a86a88a9..f0efb0889 100644 --- a/gitnexus/src/mcp/local/local-backend.ts +++ b/gitnexus/src/mcp/local/local-backend.ts @@ -28,6 +28,7 @@ import { type RegistryEntry, } from '../../storage/repo-manager.js'; import { GroupService, type GroupToolPort } from '../../core/group/service.js'; +import { resolveAtGroupMemberRepoPath } from '../../core/group/resolve-at-member.js'; // AI context generation is CLI-only (gitnexus analyze) // import { generateAIContextFiles } from '../../cli/ai-context.js'; @@ -463,8 +464,17 @@ export class LocalBackend { return this.handleGroupTool(method, params || {}); } + const p = params && typeof params === 'object' ? (params as Record) : {}; + if ( + (method === 'impact' || method === 'query' || method === 'context') && + typeof p.repo === 'string' && + p.repo.startsWith('@') + ) { + return this.callToolAtGroupRepo(method, p); + } + // Resolve repo from optional param (re-reads registry on miss) - const repo = await this.resolveRepo(params?.repo); + const repo = await this.resolveRepo((params as { repo?: string } | undefined)?.repo); switch (method) { case 'query': @@ -2547,17 +2557,66 @@ export class LocalBackend { return this.groupList(params); case 'group_sync': return this.groupSync(params); - case 'group_contracts': - return this.groupContracts(params); - case 'group_query': - return this.groupQuery(params); - case 'group_status': - return this.groupStatus(params); default: - throw new Error(`Unknown group tool: ${method}`); + throw new Error( + `Unknown group tool: ${method}. Removed tools: use repo "@" on impact, query, or context (optional "/"), or MCP resources.`, + ); } } + /** + * Dispatch impact/query/context when `repo` is `@groupName` or `@groupName/memberPath` + * (group mode — not the global indexed-repo `repo` parameter). + */ + private async callToolAtGroupRepo( + method: string, + params: Record, + ): Promise { + await this.refreshRepos(); + + if ( + params.service !== undefined && + params.service !== null && + String(params.service).trim() === '' + ) { + return { error: 'service must not be an empty string' }; + } + + const raw = String(params.repo).slice(1); + const slash = raw.indexOf('/'); + const groupName = (slash === -1 ? raw : raw.slice(0, slash)).trim(); + const memberRest = slash === -1 ? undefined : raw.slice(slash + 1).trim() || undefined; + + const resolved = await resolveAtGroupMemberRepoPath(groupName, memberRest); + if (resolved.ok === false) return { error: resolved.error }; + + const svc = this.getGroupService(); + if (method === 'impact') { + return svc.groupImpact({ + ...params, + name: groupName, + repo: resolved.repoPath, + }); + } + if (method === 'query') { + const { repo: _r, ...rest } = params; + return svc.groupQuery({ + ...rest, + name: groupName, + ...(memberRest ? { subgroup: memberRest } : {}), + }); + } + if (method === 'context') { + const { repo: _r, ...rest } = params; + return svc.groupContext({ + ...rest, + name: groupName, + ...(memberRest ? { subgroup: memberRest } : {}), + }); + } + throw new Error(`Internal: unsupported group-repo tool ${method}`); + } + private async groupList(params: Record): Promise { return this.getGroupService().groupList(params); } @@ -2566,20 +2625,6 @@ export class LocalBackend { return this.getGroupService().groupSync(params); } - private async groupContracts(params: Record): Promise { - return this.getGroupService().groupContracts(params); - } - - private async groupQuery(params: Record): Promise { - await this.refreshRepos(); - return this.getGroupService().groupQuery(params); - } - - private async groupStatus(params: Record): Promise { - await this.refreshRepos(); - return this.getGroupService().groupStatus(params); - } - /** * Fetch Route nodes with their consumers in a single query. * Shared by routeMap and shapeCheck to avoid N+1 query patterns. diff --git a/gitnexus/src/mcp/tools.ts b/gitnexus/src/mcp/tools.ts index 2c884d10b..4adf2ba3c 100644 --- a/gitnexus/src/mcp/tools.ts +++ b/gitnexus/src/mcp/tools.ts @@ -15,9 +15,12 @@ export interface ToolDefinition { { type: string; description?: string; - default?: any; + default?: unknown; items?: { type: string }; enum?: string[]; + minimum?: number; + maximum?: number; + minLength?: number; } >; required: string[]; @@ -55,7 +58,11 @@ Returns results grouped by process (execution flow): - process_symbols: all symbols in those flows with file locations and module (functional area) - definitions: standalone types/interfaces not in any process -Hybrid ranking: BM25 keyword + semantic vector search, ranked by Reciprocal Rank Fusion.`, +Hybrid ranking: BM25 keyword + semantic vector search, ranked by Reciprocal Rank Fusion. + +GROUP MODE: set "repo" to "@" to search all member repos in that group (merged via RRF), or "@/" to run against a single member (same path keys as in group.yaml). If you use "@" 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.`, inputSchema: { type: 'object', properties: { @@ -69,11 +76,19 @@ Hybrid ranking: BM25 keyword + semantic vector search, ranked by Reciprocal Rank description: 'What you want to find (e.g., "existing auth validation logic"). Helps ranking.', }, - limit: { type: 'number', description: 'Max processes to return (default: 5)', default: 5 }, + limit: { + type: 'number', + description: 'Max processes to return (default: 5)', + default: 5, + minimum: 1, + maximum: 100, + }, max_symbols: { type: 'number', description: 'Max symbols per process (default: 10)', default: 10, + minimum: 1, + maximum: 200, }, include_content: { type: 'boolean', @@ -82,7 +97,14 @@ Hybrid ranking: BM25 keyword + semantic vector search, ranked by Reciprocal Rank }, repo: { type: 'string', - description: 'Repository name or path. Omit if only one repo is indexed.', + description: + 'Indexed repository name or path, or group mode "@" / "@/" (member path keys from group.yaml). Omit when only one indexed repo exists.', + }, + service: { + 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.', }, }, required: ['query'], @@ -156,7 +178,11 @@ AFTER THIS: Use impact() if planning changes, or READ gitnexus://repo/{name}/pro Handles disambiguation: if multiple symbols share the same name, returns candidates for you to pick from. Use uid param for zero-ambiguity lookup from prior results. -NOTE: ACCESSES edges (field read/write tracking) are included in context results with reason 'read' or 'write'. CALLS edges resolve through field access chains and method-call chains (e.g., user.address.getCity().save() produces CALLS edges at each step).`, +NOTE: ACCESSES edges (field read/write tracking) are included in context results with reason 'read' or 'write'. CALLS edges resolve through field access chains and method-call chains (e.g., user.address.getCity().save() produces CALLS edges at each step). + +GROUP MODE: set "repo" to "@" to run context in each member repo (aggregated list), or "@/" for one member. If you use "@" 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).`, inputSchema: { type: 'object', properties: { @@ -173,7 +199,14 @@ NOTE: ACCESSES edges (field read/write tracking) are included in context results }, repo: { type: 'string', - description: 'Repository name or path. Omit if only one repo is indexed.', + description: + 'Indexed repository name or path, or group mode "@" / "@/". Omit if only one repo is indexed.', + }, + service: { + 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.', }, }, required: [], @@ -266,7 +299,11 @@ Depth groups: TIP: Default traversal uses CALLS/IMPORTS/EXTENDS/IMPLEMENTS. For class members, include HAS_METHOD and HAS_PROPERTY in relationTypes. For field access analysis, include ACCESSES in relationTypes. EdgeType: CALLS, IMPORTS, EXTENDS, IMPLEMENTS, HAS_METHOD, HAS_PROPERTY, METHOD_OVERRIDES, METHOD_IMPLEMENTS, ACCESSES -Confidence: 1.0 = certain, <0.8 = fuzzy match`, +Confidence: 1.0 = certain, <0.8 = fuzzy match + +GROUP MODE: set "repo" to "@" for cross-repo impact anchored at the default member (lexicographically first key in group.yaml "repos"), or "@/" 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.`, inputSchema: { type: 'object', properties: { @@ -277,8 +314,18 @@ Confidence: 1.0 = certain, <0.8 = fuzzy match`, }, maxDepth: { type: 'number', - description: 'Max relationship depth (default: 3)', + description: 'Max relationship depth (default: 3, server clamps to 1–32)', default: 3, + minimum: 1, + maximum: 32, + }, + crossDepth: { + type: 'number', + description: + 'Cross-repository hop depth via contract bridge (default: 1; values above server maximum are clamped)', + default: 1, + minimum: 1, + maximum: 32, }, relationTypes: { type: 'array', @@ -287,10 +334,40 @@ Confidence: 1.0 = certain, <0.8 = fuzzy match`, 'Filter: CALLS, IMPORTS, EXTENDS, IMPLEMENTS, HAS_METHOD, HAS_PROPERTY, METHOD_OVERRIDES, METHOD_IMPLEMENTS, ACCESSES (default: usage-based, ACCESSES excluded by default)', }, includeTests: { type: 'boolean', description: 'Include test files (default: false)' }, - minConfidence: { type: 'number', description: 'Minimum confidence 0-1 (default: 0.7)' }, + minConfidence: { + type: 'number', + description: 'Minimum edge confidence 0–1 (default: 0 when omitted; server clamps to 0–1)', + default: 0, + minimum: 0, + maximum: 1, + }, repo: { type: 'string', - description: 'Repository name or path. Omit if only one repo is indexed.', + description: + 'Indexed repository name or path, or group mode "@" / "@/". Omit if only one repo is indexed.', + }, + service: { + 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.', + }, + subgroup: { + type: 'string', + description: + 'Optional group subgroup prefix (member repo paths) limiting which repos participate in cross fan-out.', + }, + timeoutMs: { + type: 'number', + description: 'Wall-clock budget in milliseconds for the Phase-1 local impact leg (default 30000)', + minimum: 1, + maximum: 3600000, + }, + timeout: { + type: 'number', + description: 'Alias of timeoutMs (milliseconds) when timeoutMs is omitted', + minimum: 1, + maximum: 3600000, }, }, required: ['target', 'direction'], @@ -408,49 +485,4 @@ WHEN TO USE: After changing group.yaml or re-indexing member repos.`, required: ['name'], }, }, - { - name: 'group_contracts', - description: `Inspect contracts and cross-links from the group's contracts.json. - -WHEN TO USE: Debug cross-repo links after group_sync.`, - inputSchema: { - type: 'object', - properties: { - name: { type: 'string', description: 'Group name' }, - type: { type: 'string', description: 'Filter by contract type (http, topic, …)' }, - repo: { type: 'string', description: 'Filter by group repo path (e.g. app/backend)' }, - unmatchedOnly: { type: 'boolean', description: 'Only contracts with no cross-link' }, - }, - required: ['name'], - }, - }, - { - name: 'group_query', - description: `Run the query tool across all repos in a group and merge process results via reciprocal rank fusion. - -WHEN TO USE: Semantic / hybrid search across a whole product group.`, - inputSchema: { - type: 'object', - properties: { - name: { type: 'string', description: 'Group name' }, - query: { type: 'string', description: 'Search query' }, - subgroup: { type: 'string', description: 'Limit to repo paths under this prefix' }, - limit: { type: 'number', description: 'Max merged results (default 5)' }, - }, - required: ['name', 'query'], - }, - }, - { - name: 'group_status', - description: `Report index staleness (commit vs HEAD) and Contract Registry staleness (indexedAt) for each repo in a group. - -WHEN TO USE: Before group_sync or when agents should refresh indexes.`, - inputSchema: { - type: 'object', - properties: { - name: { type: 'string', description: 'Group name' }, - }, - required: ['name'], - }, - }, ]; diff --git a/gitnexus/test/integration/group/group-cli.test.ts b/gitnexus/test/integration/group/group-cli.test.ts index 02da904dc..7a76f86fd 100644 --- a/gitnexus/test/integration/group/group-cli.test.ts +++ b/gitnexus/test/integration/group/group-cli.test.ts @@ -65,4 +65,51 @@ describe('group CLI', () => { const blanketClosePattern = /closeLbug\s*\(\s*\)/; expect(source).not.toMatch(blanketClosePattern); }); + + it('group impact requires --target and --repo', () => { + const c = runGroup(['create', 'impcli']); + expect(c.status).toBe(0); + const r = runGroup(['impact', 'impcli']); + expect(r.status).not.toBe(0); + }); + + it('group impact runs with Issue #794 style flags (fixture-backed home)', () => { + const home = fs.mkdtempSync(path.join(os.tmpdir(), 'gitnexus-cli-impact-')); + try { + const gd = path.join(home, 'groups', 'test-group'); + fs.mkdirSync(gd, { recursive: true }); + fs.copyFileSync( + path.join(repoRoot, 'test', 'fixtures', 'group', 'group.yaml'), + path.join(gd, 'group.yaml'), + ); + const r = spawnSync( + process.execPath, + [ + '--import', + tsxImportUrl, + cliEntry, + 'group', + 'impact', + 'test-group', + '--target', + 'health', + '--repo', + 'app/backend', + '--json', + ], + { + cwd: repoRoot, + encoding: 'utf8', + timeout: 20000, + stdio: ['pipe', 'pipe', 'pipe'], + env: { ...process.env, GITNEXUS_HOME: home }, + }, + ); + expect(r.status).not.toBe(0); + const msg = `${r.stderr}\n${r.stdout}`; + expect(msg).toMatch(/error|indexed|not found|repository/i); + } finally { + fs.rmSync(home, { recursive: true, force: true }); + } + }); }); diff --git a/gitnexus/test/integration/group/group-impact.test.ts b/gitnexus/test/integration/group/group-impact.test.ts new file mode 100644 index 000000000..50331cc90 --- /dev/null +++ b/gitnexus/test/integration/group/group-impact.test.ts @@ -0,0 +1,74 @@ +/** + * Group impact: exercise GroupService.groupImpact with fixture-backed group config + * and a stubbed port (no LadybugDB / bridge required when local impact yields no UIDs). + */ +import { describe, it, expect, vi, beforeAll, afterAll } from 'vitest'; +import * as fs from 'node:fs'; +import * as path from 'node:path'; +import { fileURLToPath } from 'node:url'; +import os from 'node:os'; +import { GroupService, type GroupToolPort } from '../../../src/core/group/service.js'; + +const __dirname = path.dirname(fileURLToPath(import.meta.url)); +const fixturesDir = path.resolve(__dirname, '../../fixtures/group'); + +let tmpHome: string; + +beforeAll(() => { + tmpHome = fs.mkdtempSync(path.join(os.tmpdir(), 'gitnexus-grp-impact-int-')); + const groupDir = path.join(tmpHome, 'groups', 'test-group'); + fs.mkdirSync(groupDir, { recursive: true }); + fs.copyFileSync(path.join(fixturesDir, 'group.yaml'), path.join(groupDir, 'group.yaml')); +}); + +afterAll(() => { + if (tmpHome) fs.rmSync(tmpHome, { recursive: true, force: true }); +}); + +function stubPort(): GroupToolPort { + return { + resolveRepo: vi.fn(async () => ({ + id: 'stub', + name: 'stub', + repoPath: '/tmp/repo', + storagePath: '/tmp/.gitnexus', + })), + impact: vi.fn(async () => ({ + target: {}, + byDepth: {}, + summary: { direct: 0, processes_affected: 0, modules_affected: 0 }, + risk: 'LOW', + })), + query: vi.fn(), + impactByUid: vi.fn(), + context: vi.fn(), + }; +} + +describe('group impact integration', () => { + it('returns validation error when parameters are incomplete', async () => { + const svc = new GroupService(stubPort()); + const r = (await svc.groupImpact({ name: 'x', direction: 'upstream' })) as { error: string }; + expect(r.error).toMatch(/repo is required|target is required/); + }); + + it('runs happy-path stub against fixture group (stops before bridge when no symbol UIDs)', async () => { + const prev = process.env.GITNEXUS_HOME; + process.env.GITNEXUS_HOME = tmpHome; + try { + const svc = new GroupService(stubPort()); + const r = (await svc.groupImpact({ + name: 'test-group', + repo: 'app/backend', + target: 'health', + direction: 'upstream', + })) as { group?: string; error?: string; cross?: unknown[] }; + expect(r.error).toBeUndefined(); + expect(r.group).toBe('test-group'); + expect(Array.isArray(r.cross)).toBe(true); + } finally { + if (prev === undefined) delete process.env.GITNEXUS_HOME; + else process.env.GITNEXUS_HOME = prev; + } + }); +}); diff --git a/gitnexus/test/unit/group/group-service-group-mode.test.ts b/gitnexus/test/unit/group/group-service-group-mode.test.ts new file mode 100644 index 000000000..ebd0ee0ba --- /dev/null +++ b/gitnexus/test/unit/group/group-service-group-mode.test.ts @@ -0,0 +1,129 @@ +/** + * Documents MCP → GroupService mapping: callers use `name` + concrete params; + * the "@group" string is interpreted only in LocalBackend.callTool (Issue #794). + */ +import { describe, it, expect, vi } from 'vitest'; +import * as fs from 'node:fs'; +import * as path from 'node:path'; +import * as os from 'node:os'; +import { + GroupService, + type GroupToolPort, + type GroupRepoHandle, +} from '../../../src/core/group/service.js'; + +function makeTmpGroup(): { tmpDir: string; cleanup: () => void } { + const tmpDir = path.join(os.tmpdir(), `gitnexus-gmode-${Date.now()}`); + const groupDir = path.join(tmpDir, 'groups', 'test-group'); + fs.mkdirSync(groupDir, { recursive: true }); + fs.writeFileSync( + path.join(groupDir, 'group.yaml'), + `version: 1 +name: test-group +repos: + app/backend: test-backend + app/frontend: test-frontend +`, + ); + return { tmpDir, cleanup: () => fs.rmSync(tmpDir, { recursive: true, force: true }) }; +} + +function makePort(overrides: Partial = {}): GroupToolPort { + return { + resolveRepo: vi.fn( + async (name?: string): Promise => ({ + id: name || 'test', + name: name || 'test', + repoPath: '/tmp/repo', + storagePath: '/tmp/repo/.gitnexus', + }), + ), + impact: vi.fn(async () => ({ target: {}, byDepth: {} })), + query: vi.fn(async () => ({ + processes: [{ id: 'p1', heuristicLabel: 'Proc' }], + process_symbols: [ + { id: 's1', process_id: 'p1', filePath: 'services/auth/a.ts' }, + { id: 's2', process_id: 'p1', filePath: 'other/b.ts' }, + ], + })), + impactByUid: vi.fn(async () => null), + context: vi.fn(async () => ({ + status: 'found', + symbol: { filePath: 'services/auth/x.ts', uid: 'u1', name: 'X' }, + })), + ...overrides, + }; +} + +describe('GroupService group-mode API surface', () => { + it('groupQuery uses name (never @-repo) and optional service filters processes', async () => { + const { tmpDir, cleanup } = makeTmpGroup(); + vi.stubEnv('GITNEXUS_HOME', tmpDir); + try { + const query = vi.fn(async () => ({ + processes: [{ id: 'p1' }], + process_symbols: [ + { id: 's1', process_id: 'p1', filePath: 'services/auth/a.ts' }, + { id: 's2', process_id: 'p1', filePath: 'other/b.ts' }, + ], + })); + const svc = new GroupService(makePort({ query })); + const r = (await svc.groupQuery({ + name: 'test-group', + query: 'oauth', + service: 'services/auth', + })) as { results: Array<{ id?: string }> }; + expect(query).toHaveBeenCalled(); + expect(r.results.every((row) => row.id === 'p1')).toBe(true); + } finally { + vi.unstubAllEnvs(); + cleanup(); + } + }); + + it('groupQuery rejects empty service string', async () => { + const { tmpDir, cleanup } = makeTmpGroup(); + vi.stubEnv('GITNEXUS_HOME', tmpDir); + try { + const svc = new GroupService(makePort()); + const r = await svc.groupQuery({ name: 'test-group', query: 'x', service: ' ' }); + expect(r).toEqual({ error: 'service must not be an empty string' }); + } finally { + vi.unstubAllEnvs(); + cleanup(); + } + }); + + it('groupContext uses name + target (MCP maps @group to name)', async () => { + const { tmpDir, cleanup } = makeTmpGroup(); + vi.stubEnv('GITNEXUS_HOME', tmpDir); + try { + const svc = new GroupService(makePort()); + const r = await svc.groupContext({ name: 'test-group', target: 'MySym' }); + expect(r.group).toBe('test-group'); + expect(r.results).toHaveLength(2); + } finally { + vi.unstubAllEnvs(); + cleanup(); + } + }); + + it('groupImpact with mock port returns structured result without @ in params', async () => { + const { tmpDir, cleanup } = makeTmpGroup(); + vi.stubEnv('GITNEXUS_HOME', tmpDir); + try { + const svc = new GroupService(makePort()); + const r = (await svc.groupImpact({ + name: 'test-group', + repo: 'app/backend', + target: 't', + direction: 'upstream', + })) as { group?: string; error?: string }; + expect(r.error).toBeUndefined(); + expect(r.group).toBe('test-group'); + } finally { + vi.unstubAllEnvs(); + cleanup(); + } + }); +}); diff --git a/gitnexus/test/unit/group/group-tools.test.ts b/gitnexus/test/unit/group/group-tools.test.ts index e58077442..85ba45f0a 100644 --- a/gitnexus/test/unit/group/group-tools.test.ts +++ b/gitnexus/test/unit/group/group-tools.test.ts @@ -2,16 +2,10 @@ import { describe, it, expect } from 'vitest'; import { GITNEXUS_TOOLS } from '../../../src/mcp/tools.js'; -const GROUP_TOOL_NAMES = [ - 'group_list', - 'group_sync', - 'group_contracts', - 'group_query', - 'group_status', -]; +const GROUP_TOOL_NAMES = ['group_list', 'group_sync']; describe('Group MCP tools', () => { - it('all 5 group tools are registered', () => { + it('group_list and group_sync are registered', () => { for (const name of GROUP_TOOL_NAMES) { const tool = GITNEXUS_TOOLS.find((t) => t.name === name); expect(tool, `tool ${name} should be registered`).toBeDefined(); diff --git a/gitnexus/test/unit/mcp/group-repo-routing.test.ts b/gitnexus/test/unit/mcp/group-repo-routing.test.ts new file mode 100644 index 000000000..4b3a3ee63 --- /dev/null +++ b/gitnexus/test/unit/mcp/group-repo-routing.test.ts @@ -0,0 +1,138 @@ +/** + * LocalBackend.callTool routes impact/query/context to GroupService when repo starts with "@". + */ +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import * as fs from 'node:fs'; +import * as path from 'node:path'; +import * as os from 'node:os'; + +const { lbugMocks } = vi.hoisted(() => ({ + lbugMocks: { + initLbug: vi.fn().mockResolvedValue(undefined), + executeQuery: vi.fn().mockResolvedValue([]), + executeParameterized: vi.fn().mockResolvedValue([]), + closeLbug: vi.fn().mockResolvedValue(undefined), + isLbugReady: vi.fn().mockReturnValue(true), + }, +})); + +vi.mock('../../../src/core/lbug/pool-adapter.js', async (importOriginal) => { + const actual = await importOriginal(); + return { ...actual, ...lbugMocks }; +}); + +vi.mock('../../../src/mcp/core/lbug-adapter.js', async (importOriginal) => { + const actual = await importOriginal(); + return { ...actual, ...lbugMocks }; +}); + +vi.mock('../../../src/storage/repo-manager.js', () => ({ + listRegisteredRepos: vi.fn().mockResolvedValue([]), + cleanupOldKuzuFiles: vi.fn().mockResolvedValue({ found: false, needsReindex: false }), +})); + +vi.mock('../../../src/core/search/bm25-index.js', () => ({ + searchFTSFromLbug: vi.fn().mockResolvedValue([]), +})); + +vi.mock('../../../src/mcp/core/embedder.js', () => ({ + embedQuery: vi.fn().mockResolvedValue([]), + getEmbeddingDims: vi.fn().mockReturnValue(384), +})); + +import { LocalBackend } from '../../../src/mcp/local/local-backend.js'; +import { GroupService } from '../../../src/core/group/service.js'; + +describe('LocalBackend @group repo routing', () => { + let tmpDir: string; + let groupSpyQuery: ReturnType; + let groupSpyImpact: ReturnType; + let groupSpyContext: ReturnType; + + beforeEach(() => { + tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gitnexus-atgrp-')); + const groupDir = path.join(tmpDir, 'groups', 'g1'); + fs.mkdirSync(groupDir, { recursive: true }); + fs.writeFileSync( + path.join(groupDir, 'group.yaml'), + `version: 1 +name: g1 +repos: + app/backend: test-backend + app/frontend: test-frontend +`, + ); + vi.stubEnv('GITNEXUS_HOME', tmpDir); + groupSpyQuery = vi.spyOn(GroupService.prototype, 'groupQuery').mockResolvedValue({ via: 'query' }); + groupSpyImpact = vi.spyOn(GroupService.prototype, 'groupImpact').mockResolvedValue({ via: 'impact' }); + groupSpyContext = vi.spyOn(GroupService.prototype, 'groupContext').mockResolvedValue({ + group: 'g1', + results: [], + }); + }); + + afterEach(() => { + vi.unstubAllEnvs(); + vi.restoreAllMocks(); + fs.rmSync(tmpDir, { recursive: true, force: true }); + }); + + it('routes query to groupQuery with default member path (first sorted repos key)', async () => { + const backend = new LocalBackend(); + const out = await backend.callTool('query', { repo: '@g1', query: 'login' }); + expect(out).toEqual({ via: 'query' }); + expect(groupSpyQuery).toHaveBeenCalledWith( + expect.objectContaining({ name: 'g1', query: 'login' }), + ); + const arg = groupSpyQuery.mock.calls[0][0] as Record; + expect(arg).not.toHaveProperty('repo'); + }); + + it('routes query with explicit member path after slash as subgroup', 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' }), + ); + }); + + it('routes impact to groupImpact with resolved repo member path', async () => { + const backend = new LocalBackend(); + const out = await backend.callTool('impact', { + repo: '@g1', + target: 'Sym', + direction: 'upstream', + }); + expect(out).toEqual({ via: 'impact' }); + expect(groupSpyImpact).toHaveBeenCalledWith( + expect.objectContaining({ + name: 'g1', + repo: 'app/backend', + target: 'Sym', + direction: 'upstream', + }), + ); + }); + + it('routes context to groupContext', async () => { + const backend = new LocalBackend(); + await backend.callTool('context', { repo: '@g1', target: 'Sym' }); + expect(groupSpyContext).toHaveBeenCalledWith( + expect.objectContaining({ name: 'g1', target: 'Sym' }), + ); + }); + + it('rejects empty service without calling group tools', async () => { + const backend = new LocalBackend(); + const out = await backend.callTool('query', { repo: '@g1', query: 'x', service: '' }); + expect(out).toEqual({ error: 'service must not be an empty string' }); + expect(groupSpyQuery).not.toHaveBeenCalled(); + }); + + it('unknown group_* tools mention removal', async () => { + const backend = new LocalBackend(); + await expect(backend.callTool('group_query', { name: 'g1', query: 'x' })).rejects.toThrow( + /Removed tools/, + ); + }); +}); diff --git a/gitnexus/test/unit/tools.test.ts b/gitnexus/test/unit/tools.test.ts index 4274716a7..231f55e3c 100644 --- a/gitnexus/test/unit/tools.test.ts +++ b/gitnexus/test/unit/tools.test.ts @@ -2,7 +2,7 @@ * Unit Tests: MCP Tool Definitions * * Tests: GITNEXUS_TOOLS from tools.ts - * - All 16 tools are defined (per-repo + group_*) + * - 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 @@ -10,17 +10,11 @@ import { describe, it, expect } from 'vitest'; import { GITNEXUS_TOOLS } from '../../src/mcp/tools.js'; -const GROUP_TOOLS = new Set([ - 'group_list', - 'group_sync', - 'group_contracts', - 'group_query', - 'group_status', -]); +const GROUP_TOOLS = new Set(['group_list', 'group_sync']); describe('GITNEXUS_TOOLS', () => { - it('exports all tools (7 base + 3 route/tool/shape + 1 api_impact + 5 group)', () => { - expect(GITNEXUS_TOOLS).toHaveLength(16); + 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', () => { @@ -101,23 +95,29 @@ describe('GITNEXUS_TOOLS', () => { } }); - it('group_contracts has optional repo filter', () => { - const groupContracts = GITNEXUS_TOOLS.find((t) => t.name === 'group_contracts')!; - expect(groupContracts.inputSchema.properties).toHaveProperty('repo'); - expect(groupContracts.inputSchema.required).not.toContain('repo'); - }); - it('group tools without backend repo param omit repo property', () => { - for (const name of ['group_list', 'group_status', 'group_sync', 'group_query'] as const) { + 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('group_query requires name and query', () => { - const groupQuery = GITNEXUS_TOOLS.find((t) => t.name === 'group_query')!; - expect(groupQuery.inputSchema.required).toContain('name'); - expect(groupQuery.inputSchema.required).toContain('query'); + 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', () => {