From 363245eb6334c72ea55151eecb51ce5c0d183f24 Mon Sep 17 00:00:00 2001 From: Ryanba <92616678+Gujiassh@users.noreply.github.com> Date: Sun, 19 Apr 2026 14:10:36 +0800 Subject: [PATCH 01/13] fix: detect React component paths before lowercasing (#260) --- gitnexus/src/core/ingestion/framework-detection.ts | 8 ++++++-- gitnexus/test/unit/framework-detection.test.ts | 9 ++++----- 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/gitnexus/src/core/ingestion/framework-detection.ts b/gitnexus/src/core/ingestion/framework-detection.ts index 9ea43800c..739f22967 100644 --- a/gitnexus/src/core/ingestion/framework-detection.ts +++ b/gitnexus/src/core/ingestion/framework-detection.ts @@ -34,10 +34,14 @@ export interface FrameworkHint { */ export function detectFrameworkFromPath(filePath: string): FrameworkHint | null { // Normalize path separators and ensure leading slash for consistent matching - let p = filePath.toLowerCase().replace(/\\/g, '/'); + const originalPath = filePath.replace(/\\/g, '/'); + let p = originalPath.toLowerCase(); if (!p.startsWith('/')) { p = '/' + p; // Add leading slash so patterns like '/app/' match 'app/...' } + const originalPathWithLeadingSlash = originalPath.startsWith('/') + ? originalPath + : `/${originalPath}`; // ========== JAVASCRIPT / TYPESCRIPT FRAMEWORKS ========== @@ -128,7 +132,7 @@ export function detectFrameworkFromPath(filePath: string): FrameworkHint | null (p.endsWith('.tsx') || p.endsWith('.jsx')) ) { // Only boost if PascalCase filename (likely a component, not util) - const fileName = p.split('/').pop() || ''; + const fileName = originalPathWithLeadingSlash.split('/').pop() || ''; if (/^[A-Z]/.test(fileName)) { return { framework: 'react', entryPointMultiplier: 1.5, reason: 'react-component' }; } diff --git a/gitnexus/test/unit/framework-detection.test.ts b/gitnexus/test/unit/framework-detection.test.ts index b07580044..ce9d76f10 100644 --- a/gitnexus/test/unit/framework-detection.test.ts +++ b/gitnexus/test/unit/framework-detection.test.ts @@ -90,12 +90,11 @@ describe('detectFrameworkFromPath', () => { describe('React', () => { it('has React component detection rule for views/components folders', () => { - // Note: The current implementation lowercases the path before checking - // PascalCase, so PascalCase detection currently can't match. - // This test documents the current behavior. const result = detectFrameworkFromPath('views/Button.tsx'); - // Returns null because path is lowercased before PascalCase regex check - expect(result).toBeNull(); + expect(result).not.toBeNull(); + expect(result!.framework).toBe('react'); + expect(result!.entryPointMultiplier).toBe(1.5); + expect(result!.reason).toBe('react-component'); }); }); From dae7bd3b3fd387b0c1370a0ce65907710c1d314e Mon Sep 17 00:00:00 2001 From: azizur100389 Date: Sun, 19 Apr 2026 07:23:48 +0100 Subject: [PATCH 02/13] feat(cli): analyze --name + duplicate-name guard for the repo registry (#955) --- gitnexus/src/cli/analyze.ts | 46 ++++- gitnexus/src/cli/index.ts | 10 ++ gitnexus/src/cli/list.ts | 13 +- gitnexus/src/core/run-analyze.ts | 31 +++- gitnexus/src/mcp/local/local-backend.ts | 22 ++- gitnexus/src/storage/repo-manager.ts | 116 ++++++++++++- gitnexus/test/integration/cli-e2e.test.ts | 201 ++++++++++++++++++++++ gitnexus/test/unit/repo-manager.test.ts | 138 +++++++++++++++ 8 files changed, 564 insertions(+), 13 deletions(-) diff --git a/gitnexus/src/cli/analyze.ts b/gitnexus/src/cli/analyze.ts index 1e75ea675..46cedc434 100644 --- a/gitnexus/src/cli/analyze.ts +++ b/gitnexus/src/cli/analyze.ts @@ -13,7 +13,11 @@ import { execFileSync } from 'child_process'; import v8 from 'v8'; import cliProgress from 'cli-progress'; import { closeLbug } from '../core/lbug/lbug-adapter.js'; -import { getStoragePaths, getGlobalRegistryPath } from '../storage/repo-manager.js'; +import { + getStoragePaths, + getGlobalRegistryPath, + RegistryNameCollisionError, +} from '../storage/repo-manager.js'; import { getGitRoot, hasGitDir } from '../storage/git.js'; import { runFullAnalysis } from '../core/run-analyze.js'; import fs from 'fs/promises'; @@ -59,6 +63,21 @@ export interface AnalyzeOptions { noStats?: boolean; /** Index the folder even when no .git directory is present. */ skipGit?: boolean; + /** + * Override the default basename-derived registry `name` with a + * user-supplied alias (#829). Disambiguates repos whose paths share a + * basename. Persisted — subsequent re-analyses of the same path without + * `--name` preserve the alias. + */ + name?: string; + /** + * Allow registration even when another path already uses the same + * `--name` alias (#829). Intentionally a distinct flag from `--force` + * because the user may want to coexist under the same name WITHOUT + * paying the cost of a pipeline re-index. Maps to registerRepo's + * `allowDuplicateName` option end-to-end. + */ + allowDuplicateName?: boolean; } export const analyzeCommand = async (inputPath?: string, options?: AnalyzeOptions) => { @@ -186,11 +205,20 @@ export const analyzeCommand = async (inputPath?: string, options?: AnalyzeOption const result = await runFullAnalysis( repoPath, { + // Pipeline re-index — OR'd with --skills because skill generation + // needs a fresh pipelineResult. Has no bearing on the registry + // collision guard (see allowDuplicateName below). force: options?.force || options?.skills, embeddings: options?.embeddings, skipGit: options?.skipGit, skipAgentsMd: options?.skipAgentsMd, noStats: options?.noStats, + registryName: options?.name, + // Registry-collision bypass — its own CLI flag, intentionally NOT + // overloading --force. A user who hits the collision guard should + // be able to accept the duplicate name without also paying the + // cost of a full pipeline re-index. See #829 review round 2. + allowDuplicateName: options?.allowDuplicateName, }, { onProgress: (_phase, percent, message) => { @@ -298,6 +326,22 @@ export const analyzeCommand = async (inputPath?: string, options?: AnalyzeOption bar.stop(); const msg = err.message || String(err); + + // Registry name-collision from --name (#829) — surface as an + // actionable error rather than a generic stack-trace. + if (err instanceof RegistryNameCollisionError) { + console.error(`\n Registry name collision:\n`); + console.error(` "${err.registryName}" is already used by "${err.existingPath}".\n`); + console.error(` Options:`); + console.error(` • Pick a different alias: gitnexus analyze --name `); + console.error( + ` • Allow the duplicate: gitnexus analyze --allow-duplicate-name (leaves "-r ${err.registryName}" ambiguous)`, + ); + console.error(''); + process.exitCode = 1; + return; + } + console.error(`\n Analysis failed: ${msg}\n`); // Provide helpful guidance for known failure modes diff --git a/gitnexus/src/cli/index.ts b/gitnexus/src/cli/index.ts index 02581ae47..dca5983e0 100644 --- a/gitnexus/src/cli/index.ts +++ b/gitnexus/src/cli/index.ts @@ -28,6 +28,16 @@ program .option('--skip-agents-md', 'Skip updating the gitnexus section in AGENTS.md and CLAUDE.md') .option('--no-stats', 'Omit volatile file/symbol counts from AGENTS.md and CLAUDE.md') .option('--skip-git', 'Index a folder without requiring a .git directory') + .option( + '--name ', + 'Register this repo under a custom name in ~/.gitnexus/registry.json ' + + '(disambiguates repos whose paths share a basename, e.g. two different .../app folders)', + ) + .option( + '--allow-duplicate-name', + 'Register this repo even if another path already uses the same --name alias. ' + + 'Leaves `-r ` ambiguous for the two paths; use -r to disambiguate.', + ) .option('-v, --verbose', 'Enable verbose ingestion warnings (default: false)') .addHelpText( 'after', diff --git a/gitnexus/src/cli/list.ts b/gitnexus/src/cli/list.ts index 722c8ad59..5da9a86f0 100644 --- a/gitnexus/src/cli/list.ts +++ b/gitnexus/src/cli/list.ts @@ -17,12 +17,23 @@ export const listCommand = async () => { console.log(`\n Indexed Repositories (${entries.length})\n`); + // Count occurrences of each name so colliding entries can be + // disambiguated in the header (#829). Unique-name entries render + // identically to pre-#829 output; only collisions gain a suffix. + const nameCounts = new Map(); + for (const e of entries) { + const key = e.name.toLowerCase(); + nameCounts.set(key, (nameCounts.get(key) ?? 0) + 1); + } + for (const entry of entries) { const indexedDate = new Date(entry.indexedAt).toLocaleString(); const stats = entry.stats || {}; const commitShort = entry.lastCommit?.slice(0, 7) || 'unknown'; + const hasCollision = (nameCounts.get(entry.name.toLowerCase()) ?? 0) > 1; + const header = hasCollision ? `${entry.name} (${entry.path})` : entry.name; - console.log(` ${entry.name}`); + console.log(` ${header}`); console.log(` Path: ${entry.path}`); console.log(` Indexed: ${indexedDate}`); console.log(` Commit: ${commitShort}`); diff --git a/gitnexus/src/core/run-analyze.ts b/gitnexus/src/core/run-analyze.ts index 910624f5f..b17fb8e57 100644 --- a/gitnexus/src/core/run-analyze.ts +++ b/gitnexus/src/core/run-analyze.ts @@ -46,6 +46,12 @@ export interface AnalyzeCallbacks { } export interface AnalyzeOptions { + /** + * Force a full re-index of the pipeline. Callers may OR this with + * other flags that imply re-analysis (e.g. `--skills`), so the value + * here is the PIPELINE-force signal, NOT the registry-collision + * bypass. See `allowDuplicateName` below. + */ force?: boolean; embeddings?: boolean; skipGit?: boolean; @@ -53,6 +59,21 @@ export interface AnalyzeOptions { skipAgentsMd?: boolean; /** Omit volatile symbol/relationship counts from AGENTS.md and CLAUDE.md. */ noStats?: boolean; + /** + * User-provided alias for the registry `name` (#829). When set, + * forwarded to `registerRepo` so the indexed repo is stored under + * this alias instead of the path-derived basename. + */ + registryName?: string; + /** + * Bypass the `RegistryNameCollisionError` guard and allow two paths + * to register under the same `name` (#829). Controlled by the + * dedicated `--allow-duplicate-name` CLI flag, intentionally + * independent from `--force` — users who hit the collision guard + * should be able to accept the duplicate without paying the cost + * of a pipeline re-index. + */ + allowDuplicateName?: boolean; } export interface AnalyzeResult { @@ -313,7 +334,15 @@ export async function runFullAnalysis( }, }; await saveMeta(storagePath, meta); - await registerRepo(repoPath, meta); + // Forward the --name alias and the registry-collision bypass bit. + // `allowDuplicateName` is its own concern — independent from the + // pipeline `force` above. The CLI maps it from + // `--allow-duplicate-name` only; `--force` and `--skills` both + // trigger pipeline re-run but never bypass the registry guard. + await registerRepo(repoPath, meta, { + name: options.registryName, + allowDuplicateName: options.allowDuplicateName, + }); // Only attempt to update .gitignore when a .git directory is present. if (hasGitDir(repoPath)) { diff --git a/gitnexus/src/mcp/local/local-backend.ts b/gitnexus/src/mcp/local/local-backend.ts index 5b73885cd..55157cf05 100644 --- a/gitnexus/src/mcp/local/local-backend.ts +++ b/gitnexus/src/mcp/local/local-backend.ts @@ -342,13 +342,25 @@ export class LocalBackend { if (this.repos.size === 0) { throw new Error('No indexed repositories. Run: gitnexus analyze'); } - if (repoParam) { - const names = [...this.repos.values()].map((h) => h.name); - throw new Error(`Repository "${repoParam}" not found. Available: ${names.join(', ')}`); + + // Build a disambiguated "Available: …" list (#829). When two handles + // share a name, annotate each colliding label with its path so the + // caller can actually pick the right one. Single-name entries render + // identically to pre-#829 output. + const nameCounts = new Map(); + for (const h of this.repos.values()) { + const key = h.name.toLowerCase(); + nameCounts.set(key, (nameCounts.get(key) ?? 0) + 1); + } + const labels = [...this.repos.values()].map((h) => + (nameCounts.get(h.name.toLowerCase()) ?? 0) > 1 ? `${h.name} (${h.repoPath})` : h.name, + ); + + if (repoParam) { + throw new Error(`Repository "${repoParam}" not found. Available: ${labels.join(', ')}`); } - const names = [...this.repos.values()].map((h) => h.name); throw new Error( - `Multiple repositories indexed. Specify which one with the "repo" parameter. Available: ${names.join(', ')}`, + `Multiple repositories indexed. Specify which one with the "repo" parameter. Available: ${labels.join(', ')}`, ); } diff --git a/gitnexus/src/storage/repo-manager.ts b/gitnexus/src/storage/repo-manager.ts index b233d44fb..4ba17b21b 100644 --- a/gitnexus/src/storage/repo-manager.ts +++ b/gitnexus/src/storage/repo-manager.ts @@ -244,21 +244,127 @@ const writeRegistry = async (entries: RegistryEntry[]): Promise => { await fs.writeFile(getGlobalRegistryPath(), JSON.stringify(entries, null, 2), 'utf-8'); }; +/** + * Options for {@link registerRepo}. All optional — callers without any + * disambiguation requirement can keep calling `registerRepo(path, meta)` + * unchanged. + */ +export interface RegisterRepoOptions { + /** + * User-provided alias from `analyze --name ` (#829). Overrides + * the default basename-derived registry `name`. Persisted — subsequent + * re-analyses of the same path without `--name` preserve the alias. + */ + name?: string; + /** + * Allow two DIFFERENT repo paths to register under the same alias + * (#829). Mapped from the `--allow-duplicate-name` CLI flag. + * + * Scope: this flag governs cross-path alias sharing only — one repo + * path always has exactly one registry entry (and therefore exactly + * one alias). Re-analyzing the same path with `--name Y` overwrites + * a previous `--name X`; it does NOT create a second entry or a + * second alias for the same path (see the upsert-by-resolved-path + * logic in {@link registerRepo} and the + * `re-registerRepo with a different name overrides the previous + * alias` test in `test/unit/repo-manager.test.ts`). + * + * Distinct from `--force` (which only triggers pipeline re-index); + * a user accepting a duplicate alias should not be forced to also + * re-run the full pipeline. + */ + allowDuplicateName?: boolean; +} + +/** + * Thrown by {@link registerRepo} when a requested name is already in + * use by a DIFFERENT path. The CLI layer surfaces this as an actionable + * error instead of relying on `.message` string-matching. + * + * The colliding alias is exposed as `err.registryName` (not `err.name`). + * `err.name` keeps its inherited `Error.prototype.name` semantics (the + * class name) so downstream code can do the usual `err.name === + * 'RegistryNameCollisionError'` checks; use the `kind` discriminant or + * `instanceof RegistryNameCollisionError` for type-safe narrowing. + */ +export class RegistryNameCollisionError extends Error { + readonly kind = 'RegistryNameCollisionError' as const; + constructor( + public readonly registryName: string, + public readonly existingPath: string, + public readonly requestedPath: string, + ) { + super( + `Registry name "${registryName}" is already used by "${existingPath}".\n` + + `Pass --name to register "${requestedPath}" under a different name, ` + + `or --allow-duplicate-name to allow both paths under the same name (leaves -r ambiguous for these two).`, + ); + this.name = 'RegistryNameCollisionError'; + } +} + +/** Returns true when a previously-registered entry's `name` differs from + * `path.basename(entry.path)` — i.e. a user explicitly aliased it via + * `analyze --name ` on a prior run. Used to preserve the alias + * across re-analyses that omit `--name`. */ +const hasCustomAlias = (entry: RegistryEntry): boolean => { + return entry.name !== path.basename(path.resolve(entry.path)); +}; + /** * Register (add or update) a repo in the global registry. * Called after `gitnexus analyze` completes. + * + * Name resolution precedence (#829): + * 1. explicit `opts.name` (from `analyze --name `) + * 2. preserved alias on an existing entry for this path + * 3. `path.basename(repoPath)` (the original default) + * + * Duplicate-name guard: if another path already uses the resolved + * `name`, throw {@link RegistryNameCollisionError} unless + * `opts.allowDuplicateName` is set. The guard ONLY fires when the user explicitly passed a + * `name`; un-aliased basename collisions continue to register silently + * so existing users who don't know about `--name` see no behaviour + * change. */ -export const registerRepo = async (repoPath: string, meta: RepoMeta): Promise => { +export const registerRepo = async ( + repoPath: string, + meta: RepoMeta, + opts?: RegisterRepoOptions, +): Promise => { const resolved = path.resolve(repoPath); - const name = path.basename(resolved); const { storagePath } = getStoragePaths(resolved); const entries = await readRegistry(); - const existing = entries.findIndex((e) => { + const existingIdx = entries.findIndex((e) => { const a = path.resolve(e.path); const b = resolved; return process.platform === 'win32' ? a.toLowerCase() === b.toLowerCase() : a === b; }); + const existing = existingIdx >= 0 ? entries[existingIdx] : null; + + // Precedence: explicit --name > preserved alias > basename. + const name = + opts?.name ?? (existing && hasCustomAlias(existing) ? existing.name : path.basename(resolved)); + + // Duplicate-name guard: only fire when the user EXPLICITLY asked for + // this name (via opts.name or a preserved alias). Unqualified basename + // collisions are preserved for backward-compat — they still register, + // and the user sees the ambiguity at `-r` / `list` resolution time + // (which is already improved by the disambiguated error messages and + // list output this PR also ships). + const explicitName = opts?.name !== undefined || (existing && hasCustomAlias(existing)); + if (explicitName && !opts?.allowDuplicateName) { + const collidingEntry = entries.find( + (e, i) => + i !== existingIdx && + e.name.toLowerCase() === name.toLowerCase() && + path.resolve(e.path) !== resolved, + ); + if (collidingEntry) { + throw new RegistryNameCollisionError(name, collidingEntry.path, resolved); + } + } const entry: RegistryEntry = { name, @@ -269,8 +375,8 @@ export const registerRepo = async (repoPath: string, meta: RepoMeta): Promise= 0) { - entries[existing] = entry; + if (existingIdx >= 0) { + entries[existingIdx] = entry; } else { entries.push(entry); } diff --git a/gitnexus/test/integration/cli-e2e.test.ts b/gitnexus/test/integration/cli-e2e.test.ts index abdd436a4..844e3978d 100644 --- a/gitnexus/test/integration/cli-e2e.test.ts +++ b/gitnexus/test/integration/cli-e2e.test.ts @@ -110,6 +110,55 @@ function runCliRaw(extraArgs: string[], cwd: string, timeoutMs = 15000) { }); } +/** + * Like runCliRaw but accepts extra env vars. Used by tests that need to + * isolate the global registry via GITNEXUS_HOME so they don't touch the + * developer / CI agent's real ~/.gitnexus/registry.json (#829). + */ +function runCliWithEnv( + extraArgs: string[], + cwd: string, + extraEnv: Record, + timeoutMs = 15000, +) { + return spawnSync(process.execPath, ['--import', tsxImportUrl, cliEntry, ...extraArgs], { + cwd, + encoding: 'utf8', + timeout: timeoutMs, + stdio: ['pipe', 'pipe', 'pipe'], + env: { + ...process.env, + NODE_OPTIONS: `${process.env.NODE_OPTIONS || ''} --max-old-space-size=8192`.trim(), + ...extraEnv, + }, + }); +} + +/** + * Create a fresh git-initialised throwaway repo at `/` + * and return its path. Used for tests that need multiple repos whose + * basenames intentionally collide (#829 reproduction). + */ +function makeMiniRepoCopy(basename: string, prefix: string): string { + const parent = fs.mkdtempSync(path.join(os.tmpdir(), prefix)); + const repo = path.join(parent, basename); + fs.cpSync(FIXTURE_SRC, repo, { recursive: true }); + spawnSync('git', ['init'], { cwd: repo, stdio: 'pipe' }); + spawnSync('git', ['add', '-A'], { cwd: repo, stdio: 'pipe' }); + spawnSync('git', ['commit', '-m', 'initial commit'], { + cwd: repo, + stdio: 'pipe', + env: { + ...process.env, + GIT_AUTHOR_NAME: 'test', + GIT_AUTHOR_EMAIL: 'test@test', + GIT_COMMITTER_NAME: 'test', + GIT_COMMITTER_EMAIL: 'test@test', + }, + }); + return repo; +} + describe('CLI end-to-end', () => { it('status command exits cleanly', () => { const result = runCli('status', MINI_REPO); @@ -144,6 +193,158 @@ describe('CLI end-to-end', () => { expect(fs.statSync(gitnexusDir).isDirectory()).toBe(true); }); + // ─── analyze --name + --allow-duplicate-name (#829) ────── + // + // End-to-end regression guard for the name-collision feature: + // 1. `analyze --name X` persists the alias to ~/.gitnexus/registry.json + // 2. A second `analyze --name X` on a DIFFERENT path is rejected with + // a collision error (exit code 1, "already used" in output) + // 3. `analyze --name X --allow-duplicate-name` bypasses the guard; + // both entries coexist in registry.json + // 4. Pipeline-re-index flags (e.g. --skills) WITHOUT + // --allow-duplicate-name must STILL hit the collision guard — + // the bypass must stay gated on its dedicated flag so it isn't + // silently triggered by unrelated pipeline signals + // (review round 2/3 design decision). + // + // This test invokes the real CLI → runFullAnalysis → registerRepo + // chain, so any wiring regression fails here. + describe('analyze --name and --allow-duplicate-name (#829)', () => { + // Path-equality assertions across CLI spawn boundaries are fragile + // cross-platform: + // - macOS: os.tmpdir() returns /var/folders/...; child processes + // resolve the symlink to /private/var/folders/... + // - Windows: os.tmpdir() on GitHub runners returns 8.3 short-name + // form (C:\Users\RUNNER~1\...); the child sees the long form + // (C:\Users\runneradmin\...). fs.realpathSync does NOT reliably + // expand 8.3 to long form. + // Rather than fight the platform-path quagmire, we assert STRUCTURAL + // properties: entry count, alias value, path basename, path + // distinctness. That covers the behavior this test is here to + // protect without depending on exact-string path equality. + + it('--name alias stores; collision rejects; --allow-duplicate-name bypasses', () => { + // Isolate the global registry so this test never touches the + // developer's real ~/.gitnexus. + const gnHome = fs.mkdtempSync(path.join(os.tmpdir(), 'gn-home-')); + + // Two mini-repo copies whose basenames intentionally collide. + const repoA = makeMiniRepoCopy('collide-app', 'gn-collide-a-'); + const repoB = makeMiniRepoCopy('collide-app', 'gn-collide-b-'); + const parentA = path.dirname(repoA); + const parentB = path.dirname(repoB); + + try { + // Step 1: analyze repoA with --name shared → registry entry created. + const r1 = runCliWithEnv( + ['analyze', '--name', 'shared'], + repoA, + { GITNEXUS_HOME: gnHome }, + 60000, + ); + if (r1.status === null) return; // CI timeout tolerance + expect( + r1.status, + [`step 1 exited with ${r1.status}`, `stdout: ${r1.stdout}`, `stderr: ${r1.stderr}`].join( + '\n', + ), + ).toBe(0); + + const registryPath = path.join(gnHome, 'registry.json'); + const afterStep1 = JSON.parse(fs.readFileSync(registryPath, 'utf-8')); + expect(Array.isArray(afterStep1)).toBe(true); + expect(afterStep1).toHaveLength(1); + expect(afterStep1[0].name).toBe('shared'); + expect(path.basename(afterStep1[0].path)).toBe('collide-app'); + + // Step 2: analyze repoB with the SAME --name → collision error. + const r2 = runCliWithEnv( + ['analyze', '--name', 'shared'], + repoB, + { GITNEXUS_HOME: gnHome }, + 60000, + ); + if (r2.status === null) return; + expect(r2.status).toBe(1); + const r2Output = `${r2.stdout}${r2.stderr}`; + expect(r2Output).toMatch(/Registry name collision|already used/i); + + // Registry still has just the first entry — step 2 must not have + // silently added, overwritten, or corrupted anything. + const afterStep2 = JSON.parse(fs.readFileSync(registryPath, 'utf-8')); + expect(afterStep2).toHaveLength(1); + // Registry still has only the step-1 entry — the failed call + // must not have silently added, overwritten, or corrupted state. + expect(afterStep2[0].path).toBe(afterStep1[0].path); + + // Step 3: REGRESSION GUARD for the missing collision-bypass wire + // (originally a --force passthrough bug; per review round 3 the + // bypass moved to its own --allow-duplicate-name flag to avoid + // conflating it with pipeline re-index). + const r3 = runCliWithEnv( + ['analyze', '--name', 'shared', '--allow-duplicate-name'], + repoB, + { GITNEXUS_HOME: gnHome }, + 60000, + ); + if (r3.status === null) return; + expect( + r3.status, + [ + `step 3 (--allow-duplicate-name bypass) exited with ${r3.status}`, + `stdout: ${r3.stdout}`, + `stderr: ${r3.stderr}`, + ].join('\n'), + ).toBe(0); + + const afterStep3 = JSON.parse(fs.readFileSync(registryPath, 'utf-8')); + expect(afterStep3).toHaveLength(2); + expect(afterStep3.every((e: { name: string }) => e.name === 'shared')).toBe(true); + // Both entries point to distinct paths (we registered two different + // repos under the same alias) and both have the right basename. + const step3Basenames = afterStep3.map((e: { path: string }) => path.basename(e.path)); + expect(step3Basenames).toEqual(['collide-app', 'collide-app']); + const step3Paths = new Set(afterStep3.map((e: { path: string }) => e.path)); + expect(step3Paths.size).toBe(2); + // One of the two entries is the original from step 1 — unchanged. + expect(afterStep3.map((e: { path: string }) => e.path)).toContain(afterStep1[0].path); + + // Step 4: REGRESSION GUARD for the design decision in review + // round 2/3 — pipeline-re-index flags must NOT bypass the + // registry collision guard. `--skills` triggers pipeline + // re-run (skills generation needs a fresh pipelineResult) but + // must leave the registry guard in force. Bypass requires the + // explicit --allow-duplicate-name flag. + const repoC = makeMiniRepoCopy('collide-app', 'gn-collide-c-'); + const parentC = path.dirname(repoC); + try { + const r4 = runCliWithEnv( + ['analyze', '--name', 'shared', '--skills'], + repoC, + { GITNEXUS_HOME: gnHome }, + 60000, + ); + if (r4.status === null) return; + expect(r4.status).toBe(1); + const r4Output = `${r4.stdout}${r4.stderr}`; + expect(r4Output).toMatch(/Registry name collision|already used/i); + // The error hint should point at the new flag. + expect(r4Output).toMatch(/--allow-duplicate-name/); + + // Registry unchanged — still only A + B under "shared". + const afterStep4 = JSON.parse(fs.readFileSync(registryPath, 'utf-8')); + expect(afterStep4).toHaveLength(2); + } finally { + fs.rmSync(parentC, { recursive: true, force: true }); + } + } finally { + fs.rmSync(gnHome, { recursive: true, force: true }); + fs.rmSync(parentA, { recursive: true, force: true }); + fs.rmSync(parentB, { recursive: true, force: true }); + } + }, 360000); // 6-min outer budget (4 × ~60s analyze calls + fixture setup) + }); + describe('unhappy path', () => { it('exits with error when no command is given', () => { const result = runCliRaw([], MINI_REPO); diff --git a/gitnexus/test/unit/repo-manager.test.ts b/gitnexus/test/unit/repo-manager.test.ts index 08afceaff..b9eabccb4 100644 --- a/gitnexus/test/unit/repo-manager.test.ts +++ b/gitnexus/test/unit/repo-manager.test.ts @@ -13,6 +13,10 @@ import { getStoragePaths, readRegistry, loadCLIConfig, + registerRepo, + listRegisteredRepos, + RegistryNameCollisionError, + type RepoMeta, } from '../../src/storage/repo-manager.js'; import { createTempDir } from '../helpers/test-db.js'; @@ -133,3 +137,137 @@ describe('API key file permissions', () => { expect(source).toContain("process.platform !== 'win32'"); }); }); + +// ─── analyze --name + duplicate-name guard (#829) ──────────── +// +// Each test isolates the global registry by pointing GITNEXUS_HOME at a +// per-test tmpdir. `getGlobalDir()` honors that env var, so registerRepo +// writes/reads a sandboxed registry.json without touching the user's +// real ~/.gitnexus. + +describe('registerRepo name override + collision guard (#829)', () => { + let tmpHome: Awaited>; + let tmpRepoA: Awaited>; + let tmpRepoB: Awaited>; + let savedGitnexusHome: string | undefined; + + const meta: RepoMeta = { + repoPath: '', + lastCommit: 'abc1234', + indexedAt: '2026-04-18T12:00:00.000Z', + stats: { files: 1, nodes: 1 }, + }; + + beforeEach(async () => { + tmpHome = await createTempDir('gitnexus-registry-home-'); + tmpRepoA = await createTempDir('gitnexus-repo-a-'); + tmpRepoB = await createTempDir('gitnexus-repo-b-'); + savedGitnexusHome = process.env.GITNEXUS_HOME; + process.env.GITNEXUS_HOME = tmpHome.dbPath; + }); + + afterEach(async () => { + if (savedGitnexusHome === undefined) delete process.env.GITNEXUS_HOME; + else process.env.GITNEXUS_HOME = savedGitnexusHome; + await tmpHome.cleanup(); + await tmpRepoA.cleanup(); + await tmpRepoB.cleanup(); + }); + + it('registerRepo({ name: "alias" }) stores the alias instead of basename', async () => { + await registerRepo(tmpRepoA.dbPath, meta, { name: 'custom-alias' }); + + const entries = await listRegisteredRepos(); + expect(entries).toHaveLength(1); + expect(entries[0].name).toBe('custom-alias'); + expect(entries[0].name).not.toBe(path.basename(tmpRepoA.dbPath)); + }); + + it('re-registerRepo on same path without name preserves an existing alias', async () => { + await registerRepo(tmpRepoA.dbPath, meta, { name: 'custom-alias' }); + // Second call with no opts should keep the alias, not revert to basename. + await registerRepo(tmpRepoA.dbPath, meta); + + const entries = await listRegisteredRepos(); + expect(entries).toHaveLength(1); + expect(entries[0].name).toBe('custom-alias'); + }); + + it('re-registerRepo with a different name overrides the previous alias', async () => { + await registerRepo(tmpRepoA.dbPath, meta, { name: 'old-alias' }); + await registerRepo(tmpRepoA.dbPath, meta, { name: 'new-alias' }); + + const entries = await listRegisteredRepos(); + expect(entries).toHaveLength(1); + expect(entries[0].name).toBe('new-alias'); + }); + + it('registerRepo throws RegistryNameCollisionError when another path uses the name', async () => { + await registerRepo(tmpRepoA.dbPath, meta, { name: 'shared' }); + + await expect(registerRepo(tmpRepoB.dbPath, meta, { name: 'shared' })).rejects.toBeInstanceOf( + RegistryNameCollisionError, + ); + + // And the colliding entry in the error carries enough info for the + // CLI layer to surface an actionable message without string-matching. + try { + await registerRepo(tmpRepoB.dbPath, meta, { name: 'shared' }); + } catch (e) { + expect(e).toBeInstanceOf(RegistryNameCollisionError); + const err = e as RegistryNameCollisionError; + // err.registryName carries the colliding alias (exposed as its own + // field so err.name retains the inherited Error.prototype.name + // semantics for downstream `err.name === '…Error'` checks). + expect(err.registryName).toBe('shared'); + expect(err.name).toBe('RegistryNameCollisionError'); + expect(path.resolve(err.existingPath)).toBe(path.resolve(tmpRepoA.dbPath)); + expect(path.resolve(err.requestedPath)).toBe(path.resolve(tmpRepoB.dbPath)); + } + + // Registry still only has the first entry — the failed call didn't + // corrupt state. + const entries = await listRegisteredRepos(); + expect(entries).toHaveLength(1); + expect(entries[0].name).toBe('shared'); + }); + + it('registerRepo({ name, allowDuplicateName: true }) allows the duplicate to coexist', async () => { + await registerRepo(tmpRepoA.dbPath, meta, { name: 'shared' }); + await registerRepo(tmpRepoB.dbPath, meta, { name: 'shared', allowDuplicateName: true }); + + const entries = await listRegisteredRepos(); + expect(entries).toHaveLength(2); + expect(entries.every((e) => e.name === 'shared')).toBe(true); + // Both paths are stored distinctly — the collision is surfaced to the + // user via resolveRepo / list output, not hidden at the storage layer. + const paths = entries.map((e) => path.resolve(e.path)).sort(); + expect(paths).toEqual([path.resolve(tmpRepoA.dbPath), path.resolve(tmpRepoB.dbPath)].sort()); + }); + + it('basename collisions without an explicit --name still register silently (backward-compat)', async () => { + // Create two sibling dirs whose basenames collide. Neither caller + // passes { name }, so the guard must NOT fire — this preserves the + // pre-#829 behaviour for users who don't know about --name yet. + const parentA = await createTempDir('gitnexus-collide-parent-a-'); + const parentB = await createTempDir('gitnexus-collide-parent-b-'); + const sharedBasename = 'app'; + const pathA = path.join(parentA.dbPath, sharedBasename); + const pathB = path.join(parentB.dbPath, sharedBasename); + await fs.mkdir(pathA, { recursive: true }); + await fs.mkdir(pathB, { recursive: true }); + + try { + await registerRepo(pathA, meta); + await registerRepo(pathB, meta); // must NOT throw + + const entries = await listRegisteredRepos(); + expect(entries).toHaveLength(2); + expect(entries[0].name).toBe(sharedBasename); + expect(entries[1].name).toBe(sharedBasename); + } finally { + await parentA.cleanup(); + await parentB.cleanup(); + } + }); +}); From fa39a4b4a5e175ef4a125bbc13e01fca899355bf Mon Sep 17 00:00:00 2001 From: Copilot <198982749+Copilot@users.noreply.github.com> Date: Sun, 19 Apr 2026 07:46:21 +0100 Subject: [PATCH 03/13] fix(docker): build and push Docker images for Release Candidates (#978) --- .github/scripts/check-workflow-concurrency.py | 20 ++++++---- .github/workflows/docker.yml | 37 ++++++++++++++++++- .github/workflows/release-candidate.yml | 22 +++++++++++ CONTRIBUTING.md | 28 +++++++++++++- README.md | 24 +++++++++--- 5 files changed, 116 insertions(+), 15 deletions(-) diff --git a/.github/scripts/check-workflow-concurrency.py b/.github/scripts/check-workflow-concurrency.py index 300cc7f4b..0d7a49291 100644 --- a/.github/scripts/check-workflow-concurrency.py +++ b/.github/scripts/check-workflow-concurrency.py @@ -11,10 +11,14 @@ Rules: `concurrency:` block. 2. Reusable workflows (on: workflow_call ONLY) do NOT declare one. 3. The `concurrency.group` expression MUST reference either - `${{ github.workflow }}` or a literal `CI-` prefix (the documented - ci.yml reusable-workflow-safe exception). This is checked by substring - containment rather than prefix match because ci.yml's group is a - conditional expression that resolves to a `CI-…` literal at runtime. + `${{ github.workflow }}` or one of the approved hardcoded literal prefixes + for workflows that are simultaneously entry-points AND reusable (on: push/ + workflow_call). Two such exceptions are currently approved: + - `CI-` for ci.yml (the original canonical form) + - `docker-build-push-` for docker.yml + This is checked by substring containment rather than prefix match because + the group value is a conditional expression that resolves to a `CI-…` or + `docker-build-push-…` literal at runtime. We deliberately do not use a YAML library — keeps the script dependency-free on any vanilla runner. `on:` block parsing is line-based and handles both the @@ -28,7 +32,7 @@ import re import sys -REQUIRED_TOKENS = ("${{ github.workflow }}", "CI-") +REQUIRED_TOKENS = ("${{ github.workflow }}", "CI-", "docker-build-push-") def is_reusable(lines: list[str]) -> bool: @@ -150,8 +154,10 @@ def check(workflows_dir: pathlib.Path) -> int: if not any(token in group for token in REQUIRED_TOKENS): print( f"::error file={path}::concurrency.group `{group}` must " - f"reference one of {REQUIRED_TOKENS}. See CONTRIBUTING.md -> " - "GitHub Actions — Concurrency Convention." + f"reference one of {REQUIRED_TOKENS} (use ${{{{ github.workflow }}}} " + "for normal entry-point workflows; use an approved literal prefix " + "only for workflows that are both entry-points AND reusable — " + "see CONTRIBUTING.md -> GitHub Actions — Concurrency Convention)." ) fail = 1 diff --git a/.github/workflows/docker.yml b/.github/workflows/docker.yml index d27358bf6..ce583e537 100644 --- a/.github/workflows/docker.yml +++ b/.github/workflows/docker.yml @@ -7,12 +7,26 @@ on: # No workflow_dispatch: publishing is exclusively tag-driven so that every # signed image corresponds 1:1 to a published `gitnexus@X.Y.Z` on npm. A # manual run from a branch ref would fail the version check below anyway. + workflow_call: + inputs: + tag: + description: >- + The full v-prefixed tag to build (e.g. v1.2.3-rc.1). + The tag must already exist in the repo and its tree must contain + a gitnexus/package.json whose version matches the tag. + required: true + type: string # Concurrency convention: see CONTRIBUTING.md → "GitHub Actions — Concurrency Convention". # Tag refs are unique per release, so distinct tags run in parallel. # Re-pushes of the same tag serialize. cancel-in-progress: false — never cancel a publish mid-flight. +# Hardcoded `docker-build-push-` prefix (not `${{ github.workflow }}`) when invoked as a reusable +# workflow: in called-workflow context `github.workflow` is ambiguous and could resolve to the +# caller's name, sharing a concurrency group with the caller → deadlock. +# Direct tag-push invocations use `docker-build-push-`; workflow_call invocations get a +# per-run-unique group (they are already serialized by the caller's own concurrency group). concurrency: - group: ${{ github.workflow }}-${{ github.ref }} + group: ${{ (github.event_name == 'push') && format('docker-build-push-{0}', github.ref) || format('docker-build-push-nested-{0}', github.run_id) }} cancel-in-progress: false jobs: @@ -46,7 +60,12 @@ jobs: slug: gitnexus steps: + # When triggered by workflow_call the caller passes the RC tag as an input; + # we check out that tag so the Dockerfile and package.json match the built image. + # For tag-push events github.ref is already the tag ref — no override needed. - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 + with: + ref: ${{ inputs.tag || github.ref }} # ── Lock the docker image version to the npm package version ────────── # Mirrors the check in publish.yml: refuse to build unless the git tag @@ -56,8 +75,16 @@ jobs: - name: Verify tag matches gitnexus/package.json version id: version shell: bash + env: + # For workflow_call the tag comes from the caller input; for push events + # it is derived from GITHUB_REF (set to empty so the else-branch fires). + INPUT_TAG: ${{ inputs.tag }} run: | - TAG_VERSION="${GITHUB_REF#refs/tags/v}" + if [ -n "$INPUT_TAG" ]; then + TAG_VERSION="${INPUT_TAG#v}" + else + TAG_VERSION="${GITHUB_REF#refs/tags/v}" + fi if ! [[ "$TAG_VERSION" =~ ^[0-9]+\.[0-9]+\.[0-9]+(-[a-zA-Z0-9.]+)?$ ]]; then echo "::error::Tag does not follow semver: v$TAG_VERSION" exit 1 @@ -92,6 +119,11 @@ jobs: # v1.2.3-rc.1 → :1.2.3-rc.1 only (prereleases never become :latest) # `:latest` is only emitted for tag pushes thanks to `flavor: latest=auto`, # ensuring it always points at a real npm-published version. + # + # For workflow_call invocations github.ref is the caller's branch ref, so + # the type=semver patterns would not match. In that case we add an explicit + # type=raw tag using the version already verified above, so the same + # image-naming rules apply regardless of how the workflow was triggered. - name: Extract Docker metadata id: meta uses: docker/metadata-action@030e881283bb7a6894de51c315a6bfe6a94e05cf # v6.0.0 @@ -102,6 +134,7 @@ jobs: type=semver,pattern={{version}} type=semver,pattern={{major}}.{{minor}} type=semver,pattern={{major}} + type=raw,value=${{ steps.version.outputs.version }},enable=${{ github.event_name == 'workflow_call' }} - name: Build and push id: build diff --git a/.github/workflows/release-candidate.yml b/.github/workflows/release-candidate.yml index d4db75db0..3e35a58ee 100644 --- a/.github/workflows/release-candidate.yml +++ b/.github/workflows/release-candidate.yml @@ -125,6 +125,8 @@ jobs: permissions: contents: write # push rc tag + marker id-token: write # npm provenance + outputs: + vtag: ${{ steps.reltag.outputs.vtag }} steps: - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 with: @@ -364,3 +366,23 @@ jobs: Release candidates are pre-stable builds intended for early testing. Stable releases remain on the `latest` dist-tag. + + # ── Build & push RC Docker images ──────────────────────────────────── + # Calls docker.yml as a reusable workflow so that the build, signing, and + # attestation logic stays in one place. The publish job exposes `vtag` + # (e.g. `v1.2.3-rc.1`) as an output so we can pass it as the tag input. + # RC images are signed with Cosign keyless signing; the OIDC identity + # will be `docker.yml@refs/heads/main` (the caller's ref) rather than a + # tag ref — see README.md § Docker for the correct verify command for RCs. + docker: + name: Build & Push RC Docker images + needs: [guard, publish] + if: needs.guard.outputs.should_run == 'true' + uses: ./.github/workflows/docker.yml + permissions: + contents: read + packages: write + id-token: write + attestations: write + with: + tag: ${{ needs.publish.outputs.vtag }} diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 22104edb4..7f797f9a0 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -77,7 +77,7 @@ Every workflow under `.github/workflows/` MUST declare a top-level `concurrency: - Per-PR scope (for `issue_comment`, `pull_request_review*`, `pull_request` meta events): `${{ github.workflow }}-${{ github.event.pull_request.number || github.event.issue.number }}` - `workflow_run` scope (e.g. `ci-report.yml`): `${{ github.workflow }}-${{ github.event.workflow_run.pull_requests[0].number || format('{0}/{1}', github.event.workflow_run.head_repository.full_name, github.event.workflow_run.head_branch) }}` — the fork fallback must be stable across reruns (never `workflow_run.id`, which is per-run-unique and defeats serialization). - Global single-slot (manual dispatch utilities): `${{ github.workflow }}` - - **Reusable workflows invoked via `workflow_call`:** do NOT use `${{ github.workflow }}` in the group key — in called-workflow context its evaluation is ambiguous and can resolve to the caller's name, which would deadlock against the caller's own group. Use a hardcoded literal prefix and a `github.event_name`-aware expression that falls through to `github.run_id` for reusable invocations (see `ci.yml` for the canonical form). + - **Reusable workflows invoked via `workflow_call`:** do NOT use `${{ github.workflow }}` in the group key — in called-workflow context its evaluation is ambiguous and can resolve to the caller's name, which would deadlock against the caller's own group. Use a hardcoded literal prefix and a `github.event_name`-aware expression that falls through to `github.run_id` for reusable invocations (see `ci.yml` for the canonical form). Approved literal prefixes: `CI-` (`ci.yml`) and `docker-build-push-` (`docker.yml`). The `check-workflow-concurrency.py` validation script must be updated whenever a new approved literal prefix is added. - **Merge queue (`merge_group`)**: when this event is added, use `${{ github.workflow }}-${{ github.event.merge_group.head_ref }}` with `cancel-in-progress: false` (every queue entry is a distinct ref; never cancel). - **`cancel-in-progress` policy:** @@ -127,6 +127,11 @@ Two publish workflows ship `gitnexus` to npm: the cycle from `latest`. - `N` is auto-incremented against existing `X.Y.Z-rc.*` entries on the registry. First rc for a given base is `rc.1`. + - After the npm publish succeeds, the workflow calls `docker.yml` as a + reusable workflow to build and push the corresponding RC Docker images + (e.g. `ghcr.io/abhigyanpatwari/gitnexus:1.7.0-rc.1`). The images are + signed with Cosign; the OIDC identity is `docker.yml@refs/heads/main` + (the caller's ref — see README.md § Docker for the verify command). Idempotency: the workflow pushes an `rc/` marker tag and a `v` release tag **atomically, before** calling `npm publish`. The guard @@ -140,6 +145,27 @@ Two publish workflows ship `gitnexus` to npm: # then redispatch the workflow with force: true ``` + **Docker-only partial failure:** if `publish` succeeds (npm tarball + tags + are live) but the `docker` job subsequently fails (e.g. GHCR flakiness), + the npm RC is already published and the `rc/` marker is in place. + Re-running `release-candidate.yml` with `force: true` will abort at the + "Version already exists on npm" guard. To recover without cutting a new RC: + + ```bash + # 1. Manually trigger only the docker workflow, passing the existing RC tag: + gh workflow run docker.yml --ref main -f tag=v + # (requires a workflow_dispatch trigger on docker.yml — see note below) + ``` + + Because `docker.yml` intentionally has no `workflow_dispatch` (images are + tag-driven by design), the practical recovery options are: + - Wait for the next commit on `main`, which will cut a new RC that includes + the Docker build. + - Manually run `docker build` + `docker push` locally and sign with Cosign + against the same digest. + - Delete `rc/` and `v` tags, then redispatch with `force: + true` to re-run the full RC pipeline (cuts a new RC number). + The rc workflow never moves `latest`. To verify after a change, inspect dist-tags: ```bash diff --git a/README.md b/README.md index e60d8c8c1..4c273ab47 100644 --- a/README.md +++ b/README.md @@ -400,11 +400,14 @@ docker compose --env-file .env up -d The Docker images are version-locked to the npm package: -- Both images are **only published from `vX.Y.Z` git tags**, and the workflow - refuses to build unless the tag exactly matches `gitnexus/package.json`'s - version. So `ghcr.io/abhigyanpatwari/gitnexus:1.6.2` is byte-for-byte the - same release as `npm install gitnexus@1.6.2` — no drift, no floating - builds from `main`. +- Stable images are **only published from `vX.Y.Z` git tags** (via `docker.yml` + triggered directly by the tag push), and the workflow refuses to build unless + the tag exactly matches `gitnexus/package.json`'s version. So + `ghcr.io/abhigyanpatwari/gitnexus:1.6.2` is byte-for-byte the same release + as `npm install gitnexus@1.6.2` — no drift, no floating builds from `main`. +- Release-candidate images (e.g. `:1.7.0-rc.1`) are published alongside each + RC npm release. They are built by `release-candidate.yml` calling `docker.yml` + as a reusable workflow after the RC tag is created and pushed. - `:latest` is auto-promoted only from non-prerelease tags by the Docker metadata action, so it always points at a real, npm-published version. @@ -416,6 +419,8 @@ typo-squatted registry), they cannot forge a Cosign signature tied to `abhigyanpatwari/GitNexus`'s `docker.yml`. Always verify before pulling into sensitive environments: +**Stable releases** — signed from the `v*` tag ref: + ```bash cosign verify ghcr.io/abhigyanpatwari/gitnexus:1.6.2 \ --certificate-identity-regexp '^https://github\.com/abhigyanpatwari/GitNexus/\.github/workflows/docker\.yml@refs/tags/v[0-9]+\.[0-9]+\.[0-9]+(-[a-zA-Z0-9.]+)?$' \ @@ -426,6 +431,15 @@ The regex pins the certificate identity to this repo's `docker.yml` workflow **run from a `v*` tag** — rejecting unsigned images, images signed by other workflows, and images signed from unprotected refs. +**Release candidates** — signed from `refs/heads/main` (the caller's ref when +`release-candidate.yml` invokes `docker.yml` as a reusable workflow): + +```bash +cosign verify ghcr.io/abhigyanpatwari/gitnexus:1.7.0-rc.1 \ + --certificate-identity 'https://github.com/abhigyanpatwari/GitNexus/.github/workflows/docker.yml@refs/heads/main' \ + --certificate-oidc-issuer https://token.actions.githubusercontent.com +``` + You can also inspect the build provenance and SBOM: ```bash From 9926804d75b6da7479eb67b13ccbf145af01cb18 Mon Sep 17 00:00:00 2001 From: Copilot <198982749+Copilot@users.noreply.github.com> Date: Sun, 19 Apr 2026 09:14:14 +0100 Subject: [PATCH 04/13] feat(cli): infer registry name from `git remote.origin.url` (#981) * Initial plan * Plan: smarter index name inference via git remote URL Agent-Logs-Url: https://github.com/abhigyanpatwari/GitNexus/sessions/95064d2d-b1da-4c89-9069-5b3e9cc2636a Co-authored-by: magyargergo <11230420+magyargergo@users.noreply.github.com> * feat(cli): infer registry name from git remote.origin.url (#979) Agent-Logs-Url: https://github.com/abhigyanpatwari/GitNexus/sessions/95064d2d-b1da-4c89-9069-5b3e9cc2636a Co-authored-by: magyargergo <11230420+magyargergo@users.noreply.github.com> * refactor: skip git subprocess when --name was supplied (review feedback) Agent-Logs-Url: https://github.com/abhigyanpatwari/GitNexus/sessions/95064d2d-b1da-4c89-9069-5b3e9cc2636a Co-authored-by: magyargergo <11230420+magyargergo@users.noreply.github.com> * style: prettier --write on run-analyze.ts Agent-Logs-Url: https://github.com/abhigyanpatwari/GitNexus/sessions/a4bf631d-ea6b-4d84-b426-29b1e5c3539f Co-authored-by: magyargergo <11230420+magyargergo@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: magyargergo <11230420+magyargergo@users.noreply.github.com> --- gitnexus/package-lock.json | 4 +- gitnexus/src/core/run-analyze.ts | 12 +- gitnexus/src/storage/git.ts | 52 +++++++ gitnexus/src/storage/repo-manager.ts | 67 ++++++--- gitnexus/test/unit/repo-manager.test.ts | 182 ++++++++++++++++++++++++ 5 files changed, 294 insertions(+), 23 deletions(-) diff --git a/gitnexus/package-lock.json b/gitnexus/package-lock.json index f2605cb43..76afc1fde 100644 --- a/gitnexus/package-lock.json +++ b/gitnexus/package-lock.json @@ -1,12 +1,12 @@ { "name": "gitnexus", - "version": "1.6.1", + "version": "1.6.2", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "gitnexus", - "version": "1.6.1", + "version": "1.6.2", "hasInstallScript": true, "license": "PolyForm-Noncommercial-1.0.0", "dependencies": { diff --git a/gitnexus/src/core/run-analyze.ts b/gitnexus/src/core/run-analyze.ts index b17fb8e57..5c2191003 100644 --- a/gitnexus/src/core/run-analyze.ts +++ b/gitnexus/src/core/run-analyze.ts @@ -30,7 +30,7 @@ import { registerRepo, cleanupOldKuzuFiles, } from '../storage/repo-manager.js'; -import { getCurrentCommit, hasGitDir } from '../storage/git.js'; +import { getCurrentCommit, hasGitDir, getInferredRepoName } from '../storage/git.js'; import type { CachedEmbedding } from './embeddings/types.js'; import { generateAIContextFiles } from '../cli/ai-context.js'; import { EMBEDDING_TABLE_NAME } from './lbug/schema.js'; @@ -152,7 +152,7 @@ export async function runFullAnalysis( // Non-git folders have currentCommit = '' — always rebuild since we can't detect changes if (currentCommit !== '') { return { - repoName: path.basename(repoPath), + repoName: options.registryName ?? getInferredRepoName(repoPath) ?? path.basename(repoPath), repoPath, stats: existingMeta.stats ?? {}, alreadyUpToDate: true, @@ -339,7 +339,11 @@ export async function runFullAnalysis( // pipeline `force` above. The CLI maps it from // `--allow-duplicate-name` only; `--force` and `--skills` both // trigger pipeline re-run but never bypass the registry guard. - await registerRepo(repoPath, meta, { + // The returned name is the one actually written to the registry + // (after applying the precedence chain in registerRepo) — reuse it + // so AGENTS.md / skill files reference the same name MCP clients + // will look up (#979). + const projectName = await registerRepo(repoPath, meta, { name: options.registryName, allowDuplicateName: options.allowDuplicateName, }); @@ -349,8 +353,6 @@ export async function runFullAnalysis( await addToGitignore(repoPath); } - const projectName = path.basename(repoPath); - // ── Generate AI context files (best-effort) ─────────────────────── let aggregatedClusterCount = 0; if (pipelineResult.communityResult?.communities) { diff --git a/gitnexus/src/storage/git.ts b/gitnexus/src/storage/git.ts index b0e9e6d3e..c8d05ac4b 100644 --- a/gitnexus/src/storage/git.ts +++ b/gitnexus/src/storage/git.ts @@ -53,6 +53,58 @@ export const hasGitDir = (dirPath: string): boolean => { } }; +/** + * Read `remote.origin.url` from a git repository, or `null` if not a + * git repo, has no `origin` remote, or git is unavailable. + * + * Used by the registry-name inference path (#979) to recover a + * meaningful repo name when `path.basename(repoPath)` is generic + * (e.g. monorepo subprojects, git worktrees, Gas-Town-style + * `/refinery/rig/` layouts). + */ +export const getRemoteOriginUrl = (repoPath: string): string | null => { + try { + const url = execSync('git config --get remote.origin.url', { + cwd: repoPath, + stdio: ['ignore', 'pipe', 'ignore'], + }) + .toString() + .trim(); + return url || null; + } catch { + return null; + } +}; + +/** + * Parse a repository name out of a git remote URL. Handles the common + * SSH (`git@host:owner/repo.git`), HTTPS (`https://host/owner/repo.git`), + * `git://`, `ssh://`, and `file://` shapes. Returns `null` for empty / + * unparseable input. + * + * The heuristic: strip a trailing `.git` and trailing slashes, then + * take the segment after the last `/` or `:`. + */ +export const parseRepoNameFromUrl = (url: string | null | undefined): string | null => { + if (!url) return null; + const trimmed = url.trim(); + if (!trimmed) return null; + // Strip `.git` suffix (case-insensitive) and any trailing slashes. + const withoutSuffix = trimmed.replace(/\.git\/*$/i, '').replace(/\/+$/, ''); + // Last path segment, splitting on either `/` or `:` (covers SSH form). + const m = withoutSuffix.match(/[/:]([^/:]+)$/); + const candidate = m ? m[1] : withoutSuffix; + return candidate || null; +}; + +/** + * Convenience wrapper: derive a registry-friendly name from the repo's + * `origin` remote, or `null` when it cannot be inferred. + */ +export const getInferredRepoName = (repoPath: string): string | null => { + return parseRepoNameFromUrl(getRemoteOriginUrl(repoPath)); +}; + export interface DiffHunk { startLine: number; endLine: number; diff --git a/gitnexus/src/storage/repo-manager.ts b/gitnexus/src/storage/repo-manager.ts index 4ba17b21b..1b151cec1 100644 --- a/gitnexus/src/storage/repo-manager.ts +++ b/gitnexus/src/storage/repo-manager.ts @@ -9,6 +9,7 @@ import fs from 'fs/promises'; import path from 'path'; import os from 'os'; +import { getInferredRepoName } from './git.js'; export interface RepoMeta { repoPath: string; @@ -304,21 +305,33 @@ export class RegistryNameCollisionError extends Error { } /** Returns true when a previously-registered entry's `name` differs from - * `path.basename(entry.path)` — i.e. a user explicitly aliased it via - * `analyze --name ` on a prior run. Used to preserve the alias - * across re-analyses that omit `--name`. */ -const hasCustomAlias = (entry: RegistryEntry): boolean => { - return entry.name !== path.basename(path.resolve(entry.path)); + * both `path.basename(entry.path)` and the git-remote-derived name — + * i.e. a user explicitly aliased it via `analyze --name ` on a + * prior run. Used to preserve the alias across re-analyses that omit + * `--name`. The remote-derived name is treated as an inference, not a + * custom alias, so re-analyses keep tracking remote renames. + * + * `inferredName` is passed in (rather than re-derived) so callers can + * avoid a second `git config` subprocess invocation. */ +const hasCustomAlias = (entry: RegistryEntry, inferredName: string | null): boolean => { + const resolved = path.resolve(entry.path); + if (entry.name === path.basename(resolved)) return false; + if (inferredName && entry.name === inferredName) return false; + return true; }; /** * Register (add or update) a repo in the global registry. * Called after `gitnexus analyze` completes. * - * Name resolution precedence (#829): + * Name resolution precedence (#829, #979): * 1. explicit `opts.name` (from `analyze --name `) * 2. preserved alias on an existing entry for this path - * 3. `path.basename(repoPath)` (the original default) + * 3. `git config --get remote.origin.url` repo name (#979 — recovers + * a meaningful name for monorepo subprojects, git worktrees, and + * Gas-Town-style `/refinery/rig/` layouts where the basename + * is generic) + * 4. `path.basename(repoPath)` (the original default) * * Duplicate-name guard: if another path already uses the resolved * `name`, throw {@link RegistryNameCollisionError} unless @@ -326,12 +339,16 @@ const hasCustomAlias = (entry: RegistryEntry): boolean => { * `name`; un-aliased basename collisions continue to register silently * so existing users who don't know about `--name` see no behaviour * change. + * + * Returns the `name` that was actually written to the registry — the + * caller can re-use it to keep AGENTS.md / skill files aligned with the + * MCP-visible repo name (#979). */ export const registerRepo = async ( repoPath: string, meta: RepoMeta, opts?: RegisterRepoOptions, -): Promise => { +): Promise => { const resolved = path.resolve(repoPath); const { storagePath } = getStoragePaths(resolved); @@ -343,17 +360,34 @@ export const registerRepo = async ( }); const existing = existingIdx >= 0 ? entries[existingIdx] : null; - // Precedence: explicit --name > preserved alias > basename. - const name = - opts?.name ?? (existing && hasCustomAlias(existing) ? existing.name : path.basename(resolved)); + // Precedence: explicit --name > preserved alias > remote-inferred > basename. + // Skip the `git config` subprocess entirely when --name was passed — + // the remote isn't consulted in that case. + let name: string; + let isPreservedAlias = false; + if (opts?.name !== undefined) { + name = opts.name; + } else { + // Compute the remote-derived name at most once. It feeds both the + // alias-preservation check (`hasCustomAlias` needs it to distinguish + // a sticky user alias from a previously-stored remote inference) and + // the fallback name when neither --name nor a preserved alias apply. + const inferred = getInferredRepoName(resolved); + if (existing && hasCustomAlias(existing, inferred)) { + name = existing.name; + isPreservedAlias = true; + } else { + name = inferred ?? path.basename(resolved); + } + } // Duplicate-name guard: only fire when the user EXPLICITLY asked for // this name (via opts.name or a preserved alias). Unqualified basename - // collisions are preserved for backward-compat — they still register, - // and the user sees the ambiguity at `-r` / `list` resolution time - // (which is already improved by the disambiguated error messages and - // list output this PR also ships). - const explicitName = opts?.name !== undefined || (existing && hasCustomAlias(existing)); + // and remote-inferred collisions are preserved for backward-compat — + // they still register, and the user sees the ambiguity at `-r` / `list` + // resolution time (which is already improved by the disambiguated error + // messages and list output #829 ships). + const explicitName = opts?.name !== undefined || isPreservedAlias; if (explicitName && !opts?.allowDuplicateName) { const collidingEntry = entries.find( (e, i) => @@ -382,6 +416,7 @@ export const registerRepo = async ( } await writeRegistry(entries); + return name; }; /** diff --git a/gitnexus/test/unit/repo-manager.test.ts b/gitnexus/test/unit/repo-manager.test.ts index b9eabccb4..83827fc15 100644 --- a/gitnexus/test/unit/repo-manager.test.ts +++ b/gitnexus/test/unit/repo-manager.test.ts @@ -18,6 +18,8 @@ import { RegistryNameCollisionError, type RepoMeta, } from '../../src/storage/repo-manager.js'; +import { parseRepoNameFromUrl, getInferredRepoName } from '../../src/storage/git.js'; +import { execSync } from 'child_process'; import { createTempDir } from '../helpers/test-db.js'; // ─── getStoragePath ────────────────────────────────────────────────── @@ -271,3 +273,183 @@ describe('registerRepo name override + collision guard (#829)', () => { } }); }); + +// ─── parseRepoNameFromUrl + getInferredRepoName (#979) ─────────────── + +describe('parseRepoNameFromUrl', () => { + it('parses HTTPS URLs and strips .git', () => { + expect(parseRepoNameFromUrl('https://github.com/owner/lume_spark.git')).toBe('lume_spark'); + expect(parseRepoNameFromUrl('https://github.com/owner/lume_spark')).toBe('lume_spark'); + }); + + it('parses SSH URLs (git@host:owner/repo.git)', () => { + expect(parseRepoNameFromUrl('git@github.com:owner/lume_spark.git')).toBe('lume_spark'); + expect(parseRepoNameFromUrl('git@gitlab.com:group/sub/lume_spark.git')).toBe('lume_spark'); + }); + + it('parses ssh:// and git:// URLs', () => { + expect(parseRepoNameFromUrl('ssh://git@host.example/owner/lume_spark.git')).toBe('lume_spark'); + expect(parseRepoNameFromUrl('git://host.example/owner/lume_spark.git')).toBe('lume_spark'); + }); + + it('parses local file:// URLs', () => { + expect(parseRepoNameFromUrl('file:///srv/git/lume_spark.git')).toBe('lume_spark'); + }); + + it('handles trailing slashes and mixed-case .git', () => { + expect(parseRepoNameFromUrl('https://github.com/owner/lume_spark.GIT/')).toBe('lume_spark'); + expect(parseRepoNameFromUrl('https://github.com/owner/lume_spark/')).toBe('lume_spark'); + }); + + it('returns null for empty / null / undefined / unparseable input', () => { + expect(parseRepoNameFromUrl('')).toBeNull(); + expect(parseRepoNameFromUrl(' ')).toBeNull(); + expect(parseRepoNameFromUrl(null)).toBeNull(); + expect(parseRepoNameFromUrl(undefined)).toBeNull(); + }); +}); + +describe('getInferredRepoName + registerRepo (#979 — git remote inference)', () => { + let tmpHome: Awaited>; + let savedGitnexusHome: string | undefined; + + const meta: RepoMeta = { + repoPath: '', + lastCommit: 'abc1234', + indexedAt: '2026-04-19T00:00:00.000Z', + stats: { files: 1, nodes: 1 }, + }; + + /** Initialise a real git repo at `dir` with the given remote URL. */ + const initGitRepo = (dir: string, remoteUrl: string | null) => { + execSync('git init -q', { cwd: dir }); + execSync('git config user.email "test@example.com"', { cwd: dir }); + execSync('git config user.name "Test"', { cwd: dir }); + if (remoteUrl) { + execSync(`git remote add origin ${remoteUrl}`, { cwd: dir }); + } + }; + + beforeEach(async () => { + tmpHome = await createTempDir('gitnexus-registry-home-979-'); + savedGitnexusHome = process.env.GITNEXUS_HOME; + process.env.GITNEXUS_HOME = tmpHome.dbPath; + }); + + afterEach(async () => { + if (savedGitnexusHome === undefined) delete process.env.GITNEXUS_HOME; + else process.env.GITNEXUS_HOME = savedGitnexusHome; + await tmpHome.cleanup(); + }); + + it('getInferredRepoName returns null when there is no .git directory', async () => { + const tmp = await createTempDir('gitnexus-no-git-'); + try { + expect(getInferredRepoName(tmp.dbPath)).toBeNull(); + } finally { + await tmp.cleanup(); + } + }); + + it('getInferredRepoName returns null when origin is unset', async () => { + const tmp = await createTempDir('gitnexus-no-origin-'); + try { + initGitRepo(tmp.dbPath, null); + expect(getInferredRepoName(tmp.dbPath)).toBeNull(); + } finally { + await tmp.cleanup(); + } + }); + + it('getInferredRepoName returns the remote repo name when origin is set', async () => { + const tmp = await createTempDir('gitnexus-with-origin-'); + try { + initGitRepo(tmp.dbPath, 'https://github.com/owner/lume_spark.git'); + expect(getInferredRepoName(tmp.dbPath)).toBe('lume_spark'); + } finally { + await tmp.cleanup(); + } + }); + + it('registerRepo derives name from git remote when basename is generic (Gas-Town repro)', async () => { + // Reproduce /refinery/rig/.git layout: leaf basename is "rig", + // but origin URL says "lume_spark". The new precedence MUST pick up + // the remote-derived name instead of the basename. + const root = await createTempDir('gitnexus-gastown-'); + try { + const rigPath = path.join(root.dbPath, 'lume_spark', 'refinery', 'rig'); + await fs.mkdir(rigPath, { recursive: true }); + initGitRepo(rigPath, 'git@github.com:gastown/lume_spark.git'); + + const name = await registerRepo(rigPath, meta); + expect(name).toBe('lume_spark'); + expect(name).not.toBe('rig'); + + const entries = await listRegisteredRepos(); + expect(entries).toHaveLength(1); + expect(entries[0].name).toBe('lume_spark'); + } finally { + await root.cleanup(); + } + }); + + it('two analyze calls of differently-remoted "rig" leaves no longer collide', async () => { + // Without the remote inference both would register as "rig"; with + // inference they pick up their distinct remotes — the original issue. + const root = await createTempDir('gitnexus-gastown-2-'); + try { + const rigA = path.join(root.dbPath, 'lume_spark', 'refinery', 'rig'); + const rigB = path.join(root.dbPath, 'gemba', 'refinery', 'rig'); + await fs.mkdir(rigA, { recursive: true }); + await fs.mkdir(rigB, { recursive: true }); + initGitRepo(rigA, 'git@github.com:gastown/lume_spark.git'); + initGitRepo(rigB, 'git@github.com:gastown/gemba.git'); + + const nameA = await registerRepo(rigA, meta); + const nameB = await registerRepo(rigB, meta); + expect(nameA).toBe('lume_spark'); + expect(nameB).toBe('gemba'); + + const entries = await listRegisteredRepos(); + expect(entries.map((e) => e.name).sort()).toEqual(['gemba', 'lume_spark']); + } finally { + await root.cleanup(); + } + }); + + it('explicit --name still wins over remote inference', async () => { + const tmp = await createTempDir('gitnexus-name-wins-'); + try { + initGitRepo(tmp.dbPath, 'https://github.com/owner/from-remote.git'); + const name = await registerRepo(tmp.dbPath, meta, { name: 'user-alias' }); + expect(name).toBe('user-alias'); + } finally { + await tmp.cleanup(); + } + }); + + it('preserved alias still wins over remote inference on re-analyze', async () => { + const tmp = await createTempDir('gitnexus-preserve-alias-'); + try { + initGitRepo(tmp.dbPath, 'https://github.com/owner/from-remote.git'); + // First analyze sets the alias… + await registerRepo(tmp.dbPath, meta, { name: 'sticky-alias' }); + // …second analyze with no opts must keep it (not silently switch + // to the remote-derived name). + const name = await registerRepo(tmp.dbPath, meta); + expect(name).toBe('sticky-alias'); + } finally { + await tmp.cleanup(); + } + }); + + it('falls back to basename when no .git / no remote is available', async () => { + const tmp = await createTempDir('gitnexus-fallback-basename-'); + try { + const name = await registerRepo(tmp.dbPath, meta); + expect(name).toBe(path.basename(tmp.dbPath)); + } finally { + await tmp.cleanup(); + } + }); +}); From d976038dc835bdc5d0edc26c67081a76bfc37dd3 Mon Sep 17 00:00:00 2001 From: Copilot <198982749+Copilot@users.noreply.github.com> Date: Sun, 19 Apr 2026 09:41:30 +0100 Subject: [PATCH 05/13] fix: guard RC docker job against empty vtag and add early validation in docker.yml (#983) * Initial plan * fix: guard docker job and add tag validation in docker.yml - Add `&& needs.publish.outputs.vtag != ''` to the `docker` job's `if:` in release-candidate.yml so it is skipped when publish produces no vtag, preventing an opaque buildx "tag is needed" error. - Add an early "Validate tag input" step in docker.yml that fails fast with a clear ::error:: message when inputs.tag is empty, covering direct workflow_call invocations that bypass the release-candidate guard. Agent-Logs-Url: https://github.com/abhigyanpatwari/GitNexus/sessions/b9afe2df-85ea-4a87-bf30-77f0e945a64d Co-authored-by: magyargergo <11230420+magyargergo@users.noreply.github.com> * fix: scope docker.yml tag validation to workflow_call only Direct tag-push triggers (on: push, tags: v*) populate the tag from GITHUB_REF and have inputs.tag empty, so the unconditional validation step would fail every direct tag-push run. Restrict the new step to workflow_call invocations, which is the only path where an empty tag is actually a problem. Agent-Logs-Url: https://github.com/abhigyanpatwari/GitNexus/sessions/4b7e3bfa-15c0-4186-affa-95cd71e50153 Co-authored-by: magyargergo <11230420+magyargergo@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: magyargergo <11230420+magyargergo@users.noreply.github.com> --- .github/workflows/docker.yml | 11 +++++++++++ .github/workflows/release-candidate.yml | 2 +- 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/.github/workflows/docker.yml b/.github/workflows/docker.yml index ce583e537..1b83a1e4b 100644 --- a/.github/workflows/docker.yml +++ b/.github/workflows/docker.yml @@ -60,6 +60,17 @@ jobs: slug: gitnexus steps: + - name: Validate tag input + if: github.event_name == 'workflow_call' + shell: bash + env: + TAG_INPUT: ${{ inputs.tag }} + run: | + if [ -z "${TAG_INPUT}" ]; then + echo "::error::No tag provided to docker.yml — refusing to build/push." + exit 1 + fi + # When triggered by workflow_call the caller passes the RC tag as an input; # we check out that tag so the Dockerfile and package.json match the built image. # For tag-push events github.ref is already the tag ref — no override needed. diff --git a/.github/workflows/release-candidate.yml b/.github/workflows/release-candidate.yml index 3e35a58ee..61782da1c 100644 --- a/.github/workflows/release-candidate.yml +++ b/.github/workflows/release-candidate.yml @@ -377,7 +377,7 @@ jobs: docker: name: Build & Push RC Docker images needs: [guard, publish] - if: needs.guard.outputs.should_run == 'true' + if: needs.guard.outputs.should_run == 'true' && needs.publish.outputs.vtag != '' uses: ./.github/workflows/docker.yml permissions: contents: read From 2b7cff5fd275c6c01f9f5379ffddf8d01ac6b40a Mon Sep 17 00:00:00 2001 From: evolution Date: Mon, 20 Apr 2026 15:25:31 +0800 Subject: [PATCH 06/13] feat(embeddings): structural chunking with data-driven CHUNKING_RULES dispatch (#987) * feat(embeddings): structural chunking with data-driven CHUNKING_RULES dispatch Replace hardcoded label comparisons with a CHUNKING_RULES lookup table that drives chunking strategy and text generation. Key changes: - Data-driven dispatch: CHUNKING_RULES table maps labels to chunking mode (ast-function / ast-declaration), prefix/suffix, field grouping, and structural text mode - Struct support: add Struct to AST declaration chunking with field grouping (same as Class) - Multi-chunk context: preceding chunk tail (prevTail) injected into embedding text for cross-chunk coherence - Version-gated hashes: EMBEDDING_TEXT_VERSION prefix in content hashes invalidates stale vectors when text template changes - Compact container context: first declaration line preserved in every structural chunk for identity * fix(embeddings): address PR review findings for CHUNKING_RULES refactor - Remove LABEL_ENUM from STRUCTURAL_LABELS to avoid wasted AST parses - Add maintenance note about extractStructuralNames and EMBEDDING_TEXT_VERSION - Clarify CHUNK_MODE_CHARACTER is a no-op in CHUNKING_RULES - Strengthen EMBEDDING_TEXT_VERSION test assertion to exact value --------- Co-authored-by: wangjichao --- gitnexus/src/core/embeddings/chunker.ts | 77 +++++---- .../src/core/embeddings/embedding-pipeline.ts | 26 ++- .../src/core/embeddings/text-generator.ts | 71 +++++--- gitnexus/src/core/embeddings/types.ts | 151 ++++++++++++++---- gitnexus/test/unit/chunker.test.ts | 36 ++++- gitnexus/test/unit/embedding-chunking.test.ts | 67 +++++++- gitnexus/test/unit/embedding-pipeline.test.ts | 126 ++++++++++++++- gitnexus/test/unit/text-generator.test.ts | 59 +++++++ 8 files changed, 520 insertions(+), 93 deletions(-) diff --git a/gitnexus/src/core/embeddings/chunker.ts b/gitnexus/src/core/embeddings/chunker.ts index 114a5f68c..073abfb9c 100644 --- a/gitnexus/src/core/embeddings/chunker.ts +++ b/gitnexus/src/core/embeddings/chunker.ts @@ -13,6 +13,12 @@ import { characterChunk } from './character-chunk.js'; import type { Chunk } from './character-chunk.js'; import { ensureAndParse, findDeclarationNode, findFunctionNode } from './ast-utils.js'; import { buildLineIndex, resolveChunkLines } from './line-index.js'; +import { + CHUNKING_RULES, + CHUNK_MODE_AST_DECLARATION, + CHUNK_MODE_AST_FUNCTION, + type ChunkingRule, +} from './types.js'; /** * Main chunkNode function: dispatches by label @@ -40,31 +46,39 @@ export const chunkNode = async ( ]; } - // Only function-like labels get AST chunking - if (label === 'Function' || label === 'Method' || label === 'Constructor') { - try { - const astChunks = await astChunk(content, filePath, startLine, endLine, chunkSize, overlap); - if (astChunks.length > 0) return astChunks; - } catch { - // AST parsing failed — fall through to character fallback - } + const rule = CHUNKING_RULES[label]; + if (!rule) { + return characterChunk(content, startLine, endLine, chunkSize, overlap); } - if (label === 'Class' || label === 'Interface') { - try { - const declarationChunks = await declarationChunk( - label, + try { + if (rule.mode === CHUNK_MODE_AST_FUNCTION) { + const astChunks = await astChunk( content, filePath, startLine, endLine, chunkSize, overlap, + rule, + ); + if (astChunks.length > 0) return astChunks; + } + + if (rule.mode === CHUNK_MODE_AST_DECLARATION) { + const declarationChunks = await declarationChunk( + content, + filePath, + startLine, + endLine, + chunkSize, + overlap, + rule, ); if (declarationChunks.length > 0) return declarationChunks; - } catch { - // AST parsing failed — fall through to character fallback } + } catch { + // AST parsing failed — fall through to character fallback } // Character-based fallback for everything else @@ -83,6 +97,7 @@ const astChunk = async ( endLine: number, chunkSize: number, overlap: number, + rule: ChunkingRule, ): Promise => { const tree = await ensureAndParse(content, filePath); if (!tree) return []; @@ -121,8 +136,8 @@ const astChunk = async ( statements, targetNode.startIndex, targetNode.endIndex, - true, - true, + rule.includePrefix, + rule.includeSuffix, ); }; @@ -145,13 +160,13 @@ const FIELD_LIKE_MEMBER_TYPES = new Set([ ]); const declarationChunk = async ( - label: 'Class' | 'Interface', content: string, filePath: string, startLine: number, endLine: number, chunkSize: number, overlap: number, + rule: ChunkingRule, ): Promise => { const tree = await ensureAndParse(content, filePath); if (!tree) return []; @@ -162,7 +177,7 @@ const declarationChunk = async ( const bodyNode = getDeclarationBodyNode(targetNode); if (!bodyNode) return []; - const members = collectDeclarationUnits(bodyNode, label); + const members = collectDeclarationUnits(bodyNode, rule.groupFields); if (members.length === 0) return []; return chunkByUnits( @@ -174,8 +189,8 @@ const declarationChunk = async ( members, targetNode.startIndex, targetNode.endIndex, - false, - false, + rule.includePrefix, + rule.includeSuffix, ); }; @@ -237,14 +252,22 @@ const chunkByUnits = ( if (candidateEndOffset - chunkStartOffset > chunkSize) { const oversizedUnit = units[chunkStartUnitIdx]; + const oversizedStartOffset = + chunkStartUnitIdx === 0 && includeContainerPrefixOnFirstChunk + ? containerStartOffset + : oversizedUnit.startIndex; + const oversizedEndOffset = + chunkStartUnitIdx === units.length - 1 && includeContainerSuffixOnLastChunk + ? containerEndOffset + : oversizedUnit.endIndex; const oversizedLineRange = resolveChunkLines( lineOffsets, - oversizedUnit.startIndex, - oversizedUnit.endIndex, + oversizedStartOffset, + oversizedEndOffset, baseStartLine, ); const oversizedChunks = characterChunk( - content.slice(oversizedUnit.startIndex, oversizedUnit.endIndex), + content.slice(oversizedStartOffset, oversizedEndOffset), oversizedLineRange.startLine, oversizedLineRange.endLine, chunkSize, @@ -252,8 +275,8 @@ const chunkByUnits = ( ).map((chunk, offsetIdx) => ({ ...chunk, chunkIndex: chunks.length + offsetIdx, - startOffset: chunk.startOffset + oversizedUnit.startIndex, - endOffset: chunk.endOffset + oversizedUnit.startIndex, + startOffset: chunk.startOffset + oversizedStartOffset, + endOffset: chunk.endOffset + oversizedStartOffset, })); chunks.push(...oversizedChunks); chunkStartUnitIdx += 1; @@ -325,7 +348,7 @@ const getDeclarationBodyNode = (node: any): any | null => { const collectDeclarationUnits = ( bodyNode: any, - label: 'Class' | 'Interface', + groupFields: boolean, ): Array<{ startIndex: number; endIndex: number }> => { const members: Array<{ startIndex: number; endIndex: number; groupable: boolean }> = []; @@ -335,7 +358,7 @@ const collectDeclarationUnits = ( members.push({ startIndex: child.startIndex, endIndex: child.endIndex, - groupable: label === 'Class' && FIELD_LIKE_MEMBER_TYPES.has(child.type), + groupable: groupFields && FIELD_LIKE_MEMBER_TYPES.has(child.type), }); } diff --git a/gitnexus/src/core/embeddings/embedding-pipeline.ts b/gitnexus/src/core/embeddings/embedding-pipeline.ts index 302903f8b..be16789c2 100644 --- a/gitnexus/src/core/embeddings/embedding-pipeline.ts +++ b/gitnexus/src/core/embeddings/embedding-pipeline.ts @@ -30,6 +30,7 @@ import { DEFAULT_EMBEDDING_CONFIG, EMBEDDABLE_LABELS, isShortLabel, + LABEL_METHOD, LABELS_WITH_EXPORTED, STRUCTURAL_LABELS, collectBestChunks, @@ -43,6 +44,12 @@ import { import { loadVectorExtension } from '../lbug/lbug-adapter.js'; const isDev = process.env.NODE_ENV === 'development'; +/** + * Bump this when the embedding text template changes in a way that should + * invalidate existing vectors, such as metadata/header shape changes, + * structural container context changes, or preceding-context formatting rules. + */ +export const EMBEDDING_TEXT_VERSION = 'v2'; /** * Compute a stable content fingerprint for an embeddable node. @@ -57,12 +64,13 @@ export const contentHashForNode = ( // Hash must be deterministic across runs, so exclude methodNames/fieldNames // which are populated during the batch loop via AST extraction. // Using only node.content ensures the hash stays stable. + // NOTE: A change to extractStructuralNames behavior requires bumping EMBEDDING_TEXT_VERSION. const text = generateEmbeddingText( { ...node, methodNames: undefined, fieldNames: undefined }, node.content, config, ); - return createHash('sha1').update(text).digest('hex'); + return createHash('sha1').update(EMBEDDING_TEXT_VERSION).update('\n').update(text).digest('hex'); }; /** @@ -83,7 +91,7 @@ const queryEmbeddableNodes = async ( try { let query: string; - if (label === 'Method') { + if (label === LABEL_METHOD) { // Method has parameterCount and returnType query = ` MATCH (n:Method) @@ -115,7 +123,7 @@ const queryEmbeddableNodes = async ( const rows = await executeQuery(query); for (const row of rows) { - const hasExportedColumn = label === 'Method' || LABELS_WITH_EXPORTED.has(label); + const hasExportedColumn = label === LABEL_METHOD || LABELS_WITH_EXPORTED.has(label); allNodes.push({ id: row.id ?? row[0], name: row.name ?? row[1], @@ -126,7 +134,7 @@ const queryEmbeddableNodes = async ( endLine: row.endLine ?? row[6], isExported: hasExportedColumn ? (row.isExported ?? row[7]) : undefined, description: row.description ?? (hasExportedColumn ? row[8] : row[7]), - ...(label === 'Method' + ...(label === LABEL_METHOD ? { parameterCount: row.parameterCount ?? row[9], returnType: row.returnType ?? row[10], @@ -415,8 +423,15 @@ export const runEmbeddingPipeline = async ( } } + let prevTail = ''; for (const chunk of chunks) { - const text = generateEmbeddingText(node, chunk.text, finalConfig); + const text = generateEmbeddingText( + node, + chunk.text, + finalConfig, + chunk.chunkIndex, + prevTail, + ); allTexts.push(text); allUpdates.push({ nodeId: node.id, @@ -425,6 +440,7 @@ export const runEmbeddingPipeline = async ( endLine: chunk.endLine, contentHash: hash, }); + prevTail = overlap > 0 ? chunk.text.slice(-overlap) : ''; } } diff --git a/gitnexus/src/core/embeddings/text-generator.ts b/gitnexus/src/core/embeddings/text-generator.ts index 5b96f6b5e..74e90e9ce 100644 --- a/gitnexus/src/core/embeddings/text-generator.ts +++ b/gitnexus/src/core/embeddings/text-generator.ts @@ -10,7 +10,12 @@ */ import type { EmbeddableNode, EmbeddingConfig } from './types.js'; -import { DEFAULT_EMBEDDING_CONFIG, isShortLabel } from './types.js'; +import { + CHUNKING_RULES, + DEFAULT_EMBEDDING_CONFIG, + STRUCTURAL_TEXT_MODE_DECLARATION, + isShortLabel, +} from './types.js'; /** * Truncate description to max length at sentence/word boundary @@ -95,47 +100,62 @@ const generateCodeBodyText = ( node: EmbeddableNode, codeBody: string, config: Partial, + prevTail?: string, ): string => { const header = buildMetadataHeader(node, config); - const cleaned = cleanContent(codeBody); - return `${header}\n\n${cleaned}`; + const parts = [header]; + if (prevTail) { + parts.push(`[preceding context]: ...${cleanContent(prevTail)}`); + } + parts.push('', cleanContent(codeBody)); + return parts.join('\n'); }; -/** - * Generate embedding text for Class nodes - * Signature + properties + method name list only (no method bodies) - * Method/field names come from AST extractors via node.methodNames/node.fieldNames. - */ -const generateClassText = ( - node: EmbeddableNode, - codeBody: string, - config: Partial, -): string => { - return generateStructuralTypeText(node, codeBody, config); +const getCompactContainerContext = ( + cleanedContent: string, + declarationOnly: string, +): string | undefined => { + const source = declarationOnly || cleanedContent; + const nlIdx = source.indexOf('\n'); + const firstLine = (nlIdx === -1 ? source : source.substring(0, nlIdx)).trim(); + return firstLine ? `Container: ${firstLine}` : undefined; }; const generateStructuralTypeText = ( node: EmbeddableNode, codeBody: string, config: Partial, + chunkIndex?: number, + prevTail?: string, ): string => { const header = buildMetadataHeader(node, config); const parts: string[] = [header]; + const isFirstChunk = chunkIndex === undefined || chunkIndex === 0; + const cleanedContent = cleanContent(node.content); + const declarationOnly = extractDeclarationOnly(cleanedContent); + const compactContainerContext = getCompactContainerContext(cleanedContent, declarationOnly); - if (node.methodNames?.length) { + if (compactContainerContext) { + parts.push(compactContainerContext); + } + + if (prevTail) { + parts.push(`[preceding context]: ...${cleanContent(prevTail)}`); + } + + if (isFirstChunk && node.methodNames?.length) { parts.push(`Methods: ${node.methodNames.join(', ')}`); } - if (node.fieldNames?.length) { + if (isFirstChunk && node.fieldNames?.length) { parts.push(`Properties: ${node.fieldNames.join(', ')}`); } - const declarationOnly = extractDeclarationOnly(cleanContent(node.content)); - if (declarationOnly) { + if (isFirstChunk && declarationOnly) { parts.push('', declarationOnly); } const cleanedChunk = cleanContent(codeBody); - if (cleanedChunk && cleanedChunk !== cleanContent(node.content)) { + if (cleanedChunk && cleanedChunk !== cleanedContent) { parts.push('', cleanedChunk); } @@ -229,6 +249,8 @@ export const generateEmbeddingText = ( node: EmbeddableNode, codeBody: string, config: Partial = {}, + chunkIndex?: number, + prevTail?: string, ): string => { if (isShortLabel(node.label)) { const header = buildMetadataHeader(node, config); @@ -236,15 +258,12 @@ export const generateEmbeddingText = ( return `${header}\n\n${cleaned}`; } - if (node.label === 'Class') { - return generateClassText(node, codeBody, config); + const chunkingRule = CHUNKING_RULES[node.label]; + if (chunkingRule?.structuralTextMode === STRUCTURAL_TEXT_MODE_DECLARATION) { + return generateStructuralTypeText(node, codeBody, config, chunkIndex, prevTail); } - if (node.label === 'Interface') { - return generateStructuralTypeText(node, codeBody, config); - } - - return generateCodeBodyText(node, codeBody, config); + return generateCodeBodyText(node, codeBody, config, prevTail); }; /** diff --git a/gitnexus/src/core/embeddings/types.ts b/gitnexus/src/core/embeddings/types.ts index c24dcdf40..4156e9b64 100644 --- a/gitnexus/src/core/embeddings/types.ts +++ b/gitnexus/src/core/embeddings/types.ts @@ -4,35 +4,76 @@ * Type definitions for the embedding generation and semantic search system. */ +export const LABEL_FUNCTION = 'Function' as const; +export const LABEL_METHOD = 'Method' as const; +export const LABEL_CONSTRUCTOR = 'Constructor' as const; +export const LABEL_CLASS = 'Class' as const; +export const LABEL_INTERFACE = 'Interface' as const; +export const LABEL_STRUCT = 'Struct' as const; +export const LABEL_ENUM = 'Enum' as const; +export const LABEL_TRAIT = 'Trait' as const; +export const LABEL_IMPL = 'Impl' as const; +export const LABEL_MACRO = 'Macro' as const; +export const LABEL_NAMESPACE = 'Namespace' as const; +export const LABEL_TYPE_ALIAS = 'TypeAlias' as const; +export const LABEL_TYPEDEF = 'Typedef' as const; +export const LABEL_CONST = 'Const' as const; +export const LABEL_PROPERTY = 'Property' as const; +export const LABEL_RECORD = 'Record' as const; +export const LABEL_UNION = 'Union' as const; +export const LABEL_STATIC = 'Static' as const; +export const LABEL_VARIABLE = 'Variable' as const; +export const LABEL_CODE_ELEMENT = 'CodeElement' as const; + +export const CHUNK_MODE_AST_FUNCTION = 'ast-function' as const; +export const CHUNK_MODE_AST_DECLARATION = 'ast-declaration' as const; +// CHUNK_MODE_CHARACTER exists for type completeness but is a no-op in CHUNKING_RULES — +// omit the entry entirely to get character fallback via chunker.ts dispatch. +export const CHUNK_MODE_CHARACTER = 'character' as const; + +export const STRUCTURAL_TEXT_MODE_NONE = 'none' as const; +export const STRUCTURAL_TEXT_MODE_DECLARATION = 'declaration' as const; + +export interface ChunkingRule { + mode: + | typeof CHUNK_MODE_AST_FUNCTION + | typeof CHUNK_MODE_AST_DECLARATION + | typeof CHUNK_MODE_CHARACTER; + includePrefix: boolean; + includeSuffix: boolean; + groupFields: boolean; + structuralTextMode: typeof STRUCTURAL_TEXT_MODE_NONE | typeof STRUCTURAL_TEXT_MODE_DECLARATION; +} + /** * Node labels that need chunking (have code body, potentially long) */ export const CHUNKABLE_LABELS = [ - 'Function', - 'Method', - 'Constructor', - 'Class', - 'Interface', - 'Struct', - 'Enum', - 'Trait', - 'Impl', - 'Macro', - 'Namespace', + LABEL_FUNCTION, + LABEL_METHOD, + LABEL_CONSTRUCTOR, + LABEL_CLASS, + LABEL_INTERFACE, + LABEL_STRUCT, + LABEL_ENUM, + LABEL_TRAIT, + LABEL_IMPL, + LABEL_MACRO, + LABEL_NAMESPACE, ] as const; /** * Node labels that are short (no chunking needed, embed directly) */ export const SHORT_LABELS = [ - 'TypeAlias', - 'Typedef', - 'Const', - 'Property', - 'Record', - 'Union', - 'Static', - 'Variable', + LABEL_TYPE_ALIAS, + LABEL_TYPEDEF, + LABEL_CONST, + LABEL_PROPERTY, + LABEL_RECORD, + LABEL_UNION, + LABEL_STATIC, + LABEL_VARIABLE, ] as const; /** @@ -61,26 +102,78 @@ export const isShortLabel = (label: string): boolean => (SHORT_LABELS as readonly string[]).includes(label); /** - * Node labels that have structural names (methods/fields) extractable via AST + * Node labels that have structural names (methods/fields) extractable via AST. + * Only labels that consume methodNames/fieldNames in their embedding text should + * be listed here — extra entries trigger wasted AST parses with no effect on output. */ export const STRUCTURAL_LABELS: ReadonlySet = new Set([ - 'Class', - 'Struct', - 'Interface', - 'Enum', + LABEL_CLASS, + LABEL_STRUCT, + LABEL_INTERFACE, ]); /** * Node labels that have isExported column in their schema */ export const LABELS_WITH_EXPORTED = new Set([ - 'Function', - 'Class', - 'Interface', - 'Method', - 'CodeElement', + LABEL_FUNCTION, + LABEL_CLASS, + LABEL_INTERFACE, + LABEL_METHOD, + LABEL_CODE_ELEMENT, ]) as ReadonlySet; +/** + * Labels that need special chunking and/or structural text semantics. + * Any chunkable label omitted here intentionally falls back to characterChunk + * plus generateCodeBodyText (for example Enum/Trait/Impl/Macro/Namespace). + */ +type ChunkableLabel = (typeof CHUNKABLE_LABELS)[number]; +export const CHUNKING_RULES: Readonly>> = { + [LABEL_FUNCTION]: { + mode: CHUNK_MODE_AST_FUNCTION, + includePrefix: true, + includeSuffix: true, + groupFields: false, + structuralTextMode: STRUCTURAL_TEXT_MODE_NONE, + }, + [LABEL_METHOD]: { + mode: CHUNK_MODE_AST_FUNCTION, + includePrefix: true, + includeSuffix: true, + groupFields: false, + structuralTextMode: STRUCTURAL_TEXT_MODE_NONE, + }, + [LABEL_CONSTRUCTOR]: { + mode: CHUNK_MODE_AST_FUNCTION, + includePrefix: true, + includeSuffix: true, + groupFields: false, + structuralTextMode: STRUCTURAL_TEXT_MODE_NONE, + }, + [LABEL_CLASS]: { + mode: CHUNK_MODE_AST_DECLARATION, + includePrefix: true, + includeSuffix: false, + groupFields: true, + structuralTextMode: STRUCTURAL_TEXT_MODE_DECLARATION, + }, + [LABEL_INTERFACE]: { + mode: CHUNK_MODE_AST_DECLARATION, + includePrefix: true, + includeSuffix: false, + groupFields: false, + structuralTextMode: STRUCTURAL_TEXT_MODE_DECLARATION, + }, + [LABEL_STRUCT]: { + mode: CHUNK_MODE_AST_DECLARATION, + includePrefix: true, + includeSuffix: false, + groupFields: true, + structuralTextMode: STRUCTURAL_TEXT_MODE_DECLARATION, + }, +}; + /** * Embedding pipeline phases */ diff --git a/gitnexus/test/unit/chunker.test.ts b/gitnexus/test/unit/chunker.test.ts index 77ac839ba..91bda726a 100644 --- a/gitnexus/test/unit/chunker.test.ts +++ b/gitnexus/test/unit/chunker.test.ts @@ -209,11 +209,12 @@ describe('chunkNode', () => { const result = await chunkNode('Class', content, 'test.ts', 1, 6, 90, 0); expect(result).toHaveLength(2); + expect(result[0].text).toContain('class Parser {'); expect(result[0].text).toContain('options: ParserOptions;'); expect(result[0].text).toContain('cache: Map;'); expect(result[1].text).toContain('parseJSON()'); expect(result[1].text).toContain('validate()'); - expect(result[0].startLine).toBe(2); + expect(result[0].startLine).toBe(1); expect(result[1].startLine).toBe(4); }); @@ -237,11 +238,44 @@ describe('chunkNode', () => { const result = await chunkNode('Interface', content, 'test.ts', 10, 14, 500, 0); expect(result).toHaveLength(1); + expect(result[0].text).toContain('interface Handler {'); expect(result[0].text).toContain('handle(event: Event): void;'); expect(result[0].text).toContain('validate(input: string): boolean;'); expect(result[0].text).toContain('readonly name: string;'); }); + it('uses declaration-aware chunking for Struct labels', async () => { + const content = [ + 'struct User {', + ' name: String,', + ' email: String,', + ' age: u32,', + ' address: String,', + '}', + ].join('\n'); + const tree = makeDeclarationTree('struct_item', 'declaration_list', content, [ + 'name: String,', + 'email: String,', + 'age: u32,', + 'address: String,', + ]); + createParserForLanguage.mockResolvedValue({ + parse: vi.fn().mockReturnValue(tree), + }); + + const result = await chunkNode('Struct', content, 'test.rs', 40, 45, 45, 0); + + expect(result).toHaveLength(2); + expect(result[0].text).toContain('struct User {'); + expect(result[0].text).toContain('name: String,'); + expect(result[0].text).toContain('email: String'); + const combinedText = result.map((chunk) => chunk.text).join('\n'); + expect(combinedText).toContain('email: String'); + expect(combinedText).toContain('age: u32'); + expect(combinedText).toContain('address: String'); + expect(result[0].startLine).toBe(40); + }); + it('splits a function into multiple AST-aware chunks using snippet offsets', async () => { const content = [ 'function example() {', diff --git a/gitnexus/test/unit/embedding-chunking.test.ts b/gitnexus/test/unit/embedding-chunking.test.ts index 62fdd2b23..244efe63d 100644 --- a/gitnexus/test/unit/embedding-chunking.test.ts +++ b/gitnexus/test/unit/embedding-chunking.test.ts @@ -18,12 +18,19 @@ vi.mock('../../src/core/tree-sitter/parser-loader.js', () => ({ resolveLanguageKey: vi.fn((language: string) => language), })); -vi.mock('gitnexus-shared', () => ({ +const { getLanguageFromFilename } = vi.hoisted(() => ({ getLanguageFromFilename: vi.fn().mockReturnValue('typescript'), })); +vi.mock('gitnexus-shared', () => ({ + getLanguageFromFilename, +})); + import { chunkNode } from '../../src/core/embeddings/chunker.js'; +const CLASS_PREV_TAIL_SAMPLE = 30; +const STRUCT_PREV_TAIL_SAMPLE = 20; + type FakeNode = { type: string; startIndex: number; @@ -183,10 +190,18 @@ describe('embedding-chunking integration', () => { const chunks = await chunkNode(node.label, node.content, node.filePath, 20, 25, 90, 0); expect(chunks).toHaveLength(2); - const secondText = generateEmbeddingText(node, chunks[1].text); + const secondText = generateEmbeddingText( + node, + chunks[1].text, + {}, + chunks[1].chunkIndex, + chunks[0].text.slice(-CLASS_PREV_TAIL_SAMPLE), + ); expect(secondText).toContain('Class: Parser'); - expect(secondText).toContain('Methods: parseJSON, validate'); - expect(secondText).toContain('Properties: options, cache'); + expect(secondText).toContain('Container: class Parser {'); + expect(secondText).toContain('[preceding context]: ...'); + expect(secondText).not.toContain('Methods: parseJSON, validate'); + expect(secondText).not.toContain('Properties: options, cache'); expect(secondText).toContain('parseJSON(text: string)'); }); @@ -220,9 +235,53 @@ describe('embedding-chunking integration', () => { const text = generateEmbeddingText(node, chunks[0].text); expect(text).toContain('Interface: Handler'); expect(text).toContain('Methods: handle, validate'); + expect(text).toContain('Container: interface Handler {'); expect(text).toContain('readonly name: string;'); }); + it('struct chunks retain structural container context', async () => { + getLanguageFromFilename.mockReturnValue('rust'); + const node = makeNode({ + label: 'Struct', + name: 'User', + fieldNames: ['name', 'email', 'age', 'address'], + content: `struct User { + name: String, + email: String, + age: u32, + address: String, +}`, + startLine: 40, + endLine: 45, + filePath: 'src/user.rs', + }); + createParserForLanguage.mockResolvedValue({ + parse: vi.fn().mockReturnValue( + makeDeclarationTree('struct_item', 'declaration_list', node.content, [ + { text: 'name: String,', type: 'field_definition' }, + { text: 'email: String,', type: 'field_definition' }, + { text: 'age: u32,', type: 'field_definition' }, + { text: 'address: String,', type: 'field_definition' }, + ]), + ), + }); + + const chunks = await chunkNode(node.label, node.content, node.filePath, 40, 45, 45, 0); + expect(chunks).toHaveLength(2); + + const secondText = generateEmbeddingText( + node, + chunks[1].text, + {}, + chunks[1].chunkIndex, + chunks[0].text.slice(-STRUCT_PREV_TAIL_SAMPLE), + ); + expect(secondText).toContain('Struct: User'); + expect(secondText).toContain('Container: struct User {'); + expect(secondText).not.toContain('Properties: name, email, age, address'); + expect(secondText).toContain('age: u32,'); + }); + it('metadata is present in every chunk', () => { const longContent = 'x'.repeat(3000); const node = makeNode({ diff --git a/gitnexus/test/unit/embedding-pipeline.test.ts b/gitnexus/test/unit/embedding-pipeline.test.ts index 5276fd3da..caa160d1e 100644 --- a/gitnexus/test/unit/embedding-pipeline.test.ts +++ b/gitnexus/test/unit/embedding-pipeline.test.ts @@ -1,11 +1,17 @@ import { describe, it, expect, vi, beforeEach } from 'vitest'; import { createHash } from 'crypto'; -import { contentHashForNode } from '../../src/core/embeddings/embedding-pipeline.js'; +import { + contentHashForNode, + EMBEDDING_TEXT_VERSION, +} from '../../src/core/embeddings/embedding-pipeline.js'; import { generateEmbeddingText } from '../../src/core/embeddings/text-generator.js'; import type { EmbeddableNode, EmbeddingProgress } from '../../src/core/embeddings/types.js'; import { DEFAULT_EMBEDDING_CONFIG, EMBEDDABLE_LABELS } from '../../src/core/embeddings/types.js'; import { STALE_HASH_SENTINEL } from '../../src/core/lbug/schema.js'; +const CLASS_CHUNK_SIZE = 90; +const CLASS_OVERLAP = 10; + // ──────────────────────────────────────────────────────────────────────────── // contentHashForNode // ──────────────────────────────────────────────────────────────────────────── @@ -32,6 +38,8 @@ describe('contentHashForNode', () => { it('matches sha1(generateEmbeddingText(node, node.content))', () => { const node = makeNode(); const expected = createHash('sha1') + .update(EMBEDDING_TEXT_VERSION) + .update('\n') .update(generateEmbeddingText(node, node.content)) .digest('hex'); expect(contentHashForNode(node)).toBe(expected); @@ -56,6 +64,10 @@ describe('contentHashForNode', () => { const hashWithFullDefaults = contentHashForNode(node, DEFAULT_EMBEDDING_CONFIG); expect(hashWithEmptyConfig).toBe(hashWithFullDefaults); }); + + it('exports a text template version marker', () => { + expect(EMBEDDING_TEXT_VERSION).toBe('v2'); + }); }); // ──────────────────────────────────────────────────────────────────────────── @@ -439,6 +451,118 @@ describe('runEmbeddingPipeline incremental filter', () => { expect(vectorIndexCalls.length).toBeGreaterThanOrEqual(1); }); + it('does not inject preceding context when overlap is disabled', async () => { + const embedBatchSpy = vi + .fn() + .mockImplementation((texts: string[]) => + Promise.resolve(texts.map(() => new Float32Array(384))), + ); + vi.doMock('../../src/core/embeddings/embedder.js', () => ({ + initEmbedder: vi.fn().mockResolvedValue(undefined), + embedBatch: embedBatchSpy, + embedText: vi.fn().mockResolvedValue(new Float32Array(384)), + embeddingToArray: vi.fn().mockImplementation((emb: Float32Array) => Array.from(emb)), + isEmbedderReady: vi.fn().mockReturnValue(true), + })); + vi.doMock('../../src/core/lbug/lbug-adapter.js', () => ({ + loadVectorExtension: vi.fn().mockResolvedValue(undefined), + })); + + const node = makeNode({ + label: 'Class', + name: 'Parser', + content: `class Parser { + options: ParserOptions; + cache: Map; + parseJSON() { return JSON.parse("{}"); } + validate() { return true; } +}`, + startLine: 1, + endLine: 6, + }); + + const executeQuery = mockExecuteQuery([node]); + const executeWithReusedStatement = mockExecuteWithReusedStatement(); + + const { runEmbeddingPipeline } = + await import('../../src/core/embeddings/embedding-pipeline.js'); + + await runEmbeddingPipeline( + executeQuery, + executeWithReusedStatement, + onProgress, + { chunkSize: 90, overlap: 0 }, + undefined, + undefined, + new Map(), + ); + + const embeddedTexts = embedBatchSpy.mock.calls.flatMap((call) => call[0] as string[]); + const laterChunks = embeddedTexts.slice(1); + expect(laterChunks.length).toBeGreaterThan(0); + for (const text of laterChunks) { + expect(text).not.toContain('[preceding context]:'); + } + }); + + it('truncates preceding context to the configured overlap size', async () => { + const embedBatchSpy = vi + .fn() + .mockImplementation((texts: string[]) => + Promise.resolve(texts.map(() => new Float32Array(384))), + ); + vi.doMock('../../src/core/embeddings/embedder.js', () => ({ + initEmbedder: vi.fn().mockResolvedValue(undefined), + embedBatch: embedBatchSpy, + embedText: vi.fn().mockResolvedValue(new Float32Array(384)), + embeddingToArray: vi.fn().mockImplementation((emb: Float32Array) => Array.from(emb)), + isEmbedderReady: vi.fn().mockReturnValue(true), + })); + vi.doMock('../../src/core/lbug/lbug-adapter.js', () => ({ + loadVectorExtension: vi.fn().mockResolvedValue(undefined), + })); + + const node = makeNode({ + label: 'Class', + name: 'Parser', + content: `class Parser { + options: ParserOptions; + cache: Map; + parseJSON() { return JSON.parse("{}"); } + validate() { return true; } +}`, + startLine: 1, + endLine: 6, + }); + + const executeQuery = mockExecuteQuery([node]); + const executeWithReusedStatement = mockExecuteWithReusedStatement(); + + const { runEmbeddingPipeline } = + await import('../../src/core/embeddings/embedding-pipeline.js'); + + await runEmbeddingPipeline( + executeQuery, + executeWithReusedStatement, + onProgress, + { chunkSize: CLASS_CHUNK_SIZE, overlap: CLASS_OVERLAP }, + undefined, + undefined, + new Map(), + ); + + const embeddedTexts = embedBatchSpy.mock.calls.flatMap((call) => call[0] as string[]); + const laterChunk = embeddedTexts.find((text) => text.includes('[preceding context]:')); + expect(laterChunk).toBeDefined(); + expect(laterChunk).toContain('[preceding context]: ...'); + const precedingContextLine = laterChunk + ?.split('\n') + .find((line) => line.startsWith('[preceding context]: ...')); + expect(precedingContextLine).toBeDefined(); + expect(precedingContextLine).toContain('ring, any>'); + expect(precedingContextLine).not.toContain('parseJSON() {'); + }); + it('throws when DELETE for stale nodes fails with non-trivial error', async () => { mockEmbedderSetup(); diff --git a/gitnexus/test/unit/text-generator.test.ts b/gitnexus/test/unit/text-generator.test.ts index 28e16044d..e411428df 100644 --- a/gitnexus/test/unit/text-generator.test.ts +++ b/gitnexus/test/unit/text-generator.test.ts @@ -148,6 +148,65 @@ describe('text-generator', () => { expect(text).toContain('class Parser {'); expect(text).toContain('parseJSON(text: string) { return JSON.parse(text); }'); }); + + it('generates Struct text with structural metadata', () => { + const node: EmbeddableNode = { + ...baseNode, + label: 'Struct', + name: 'User', + fieldNames: ['name', 'age'], + content: `struct User { + name: String, + age: u32, +}`, + }; + const text = generateEmbeddingText(node, node.content); + expect(text).toContain('Struct: User'); + expect(text).toContain('Properties: name, age'); + expect(text).toContain('Container: struct User {'); + expect(text).toContain('struct User {'); + }); + + it('keeps compact container context on later structural chunks', () => { + const node: EmbeddableNode = { + ...baseNode, + label: 'Class', + name: 'Parser', + methodNames: ['parseJSON', 'validate'], + fieldNames: ['options', 'cache'], + content: `class Parser { + options: ParserOptions; + cache: Map; + parseJSON(text: string) { return JSON.parse(text); } + validate() { return true; } +}`, + }; + const text = generateEmbeddingText( + node, + 'validate() { return true; }', + {}, + 1, + 'parseJSON(text: string) { return JSON.parse(text); }', + ); + expect(text).toContain('Class: Parser'); + expect(text).toContain('Container: class Parser {'); + expect(text).toContain('[preceding context]: ...parseJSON(text: string)'); + expect(text).not.toContain('Methods: parseJSON, validate'); + expect(text).not.toContain('Properties: options, cache'); + }); + + it('adds preceding context to non-structural chunk text', () => { + const text = generateEmbeddingText( + baseNode, + 'return JSON.parse(text);', + {}, + 1, + 'function parseJSON(text: string): Result {', + ); + expect(text).toContain('Function: parseJSON'); + expect(text).toContain('[preceding context]: ...function parseJSON'); + expect(text).toContain('return JSON.parse(text);'); + }); }); describe('Constructor label', () => { From f53e2820261da292c345beaf919d99094c009d8c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gerg=C5=91=20Magyar?= Date: Mon, 20 Apr 2026 08:59:50 +0100 Subject: [PATCH 07/13] Update Discord link in README.md --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 4c273ab47..af10557f4 100644 --- a/README.md +++ b/README.md @@ -9,7 +9,7 @@

Join the official Discord to discuss ideas, issues etc!

- + Discord From 5c3f56df7db397a9adb0007b49e40f3c48828395 Mon Sep 17 00:00:00 2001 From: Andrew Barnes Date: Mon, 20 Apr 2026 04:26:26 -0400 Subject: [PATCH 08/13] docs: add --skip-git to CLI command list (#750) --- README.md | 1 + 1 file changed, 1 insertion(+) diff --git a/README.md b/README.md index af10557f4..9e20c274c 100644 --- a/README.md +++ b/README.md @@ -194,6 +194,7 @@ gitnexus analyze --force # Force full re-index gitnexus analyze --skills # Generate repo-specific skill files from detected communities gitnexus analyze --skip-embeddings # Skip embedding generation (faster) gitnexus analyze --skip-agents-md # Preserve custom AGENTS.md/CLAUDE.md gitnexus section edits +gitnexus analyze --skip-git # Index folders that are not Git repositories gitnexus analyze --embeddings # Enable embedding generation (slower, better search) gitnexus analyze --verbose # Log skipped files when parsers are unavailable gitnexus mcp # Start MCP server (stdio) — serves all indexed repos From 00966630c4e2645fda4909913a9deed10963002d Mon Sep 17 00:00:00 2001 From: ivkond Date: Mon, 20 Apr 2026 13:55:07 +0300 Subject: [PATCH 09/13] =?UTF-8?q?feat:=20cross-repo=20impact=20analysis=20?= =?UTF-8?q?(#794)=20=E2=80=94=20@repo=20MCP=20routing=20+=20group=20resour?= =?UTF-8?q?ces=20(#984)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .gitignore | 3 + AGENTS.md | 11 +- ARCHITECTURE.md | 13 +- gitnexus/CHANGELOG.md | 6 + gitnexus/src/cli/ai-context.ts | 2 +- gitnexus/src/cli/group.ts | 77 +++ gitnexus/src/core/group/cross-impact.ts | 562 ++++++++++++++++++ gitnexus/src/core/group/group-path-utils.ts | 42 ++ gitnexus/src/core/group/resolve-at-member.ts | 34 ++ gitnexus/src/core/group/service.ts | 330 ++++++++-- gitnexus/src/core/group/types.ts | 33 + gitnexus/src/core/lbug/lbug-adapter.ts | 36 ++ gitnexus/src/core/run-analyze.ts | 18 +- gitnexus/src/core/search/bm25-index.ts | 72 ++- gitnexus/src/mcp/local/local-backend.ts | 156 ++++- gitnexus/src/mcp/resources.ts | 149 ++++- gitnexus/src/mcp/tools.ts | 144 +++-- .../test/integration/group/group-cli.test.ts | 47 ++ .../integration/group/group-impact.test.ts | 74 +++ gitnexus/test/unit/group/cross-impact.test.ts | 196 ++++++ .../test/unit/group/group-path-utils.test.ts | 87 +++ .../group/group-service-group-mode.test.ts | 129 ++++ gitnexus/test/unit/group/group-tools.test.ts | 10 +- gitnexus/test/unit/group/service.test.ts | 173 ++++++ .../test/unit/mcp/group-repo-routing.test.ts | 230 +++++++ gitnexus/test/unit/resources.test.ts | 86 ++- gitnexus/test/unit/tools.test.ts | 42 +- 27 files changed, 2584 insertions(+), 178 deletions(-) create mode 100644 gitnexus/src/core/group/cross-impact.ts create mode 100644 gitnexus/src/core/group/group-path-utils.ts 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/cross-impact.test.ts create mode 100644 gitnexus/test/unit/group/group-path-utils.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/.gitignore b/.gitignore index 95c9164e0..92027b529 100644 --- a/.gitignore +++ b/.gitignore @@ -103,3 +103,6 @@ gitnexus/vendor/**/node_modules/ local_docs/ +# Local agent scratch / review prompts (never commit) +.tmp/ +.agents/ diff --git a/AGENTS.md b/AGENTS.md index f4fbcadc0..6e916ff0b 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -47,6 +47,7 @@ Commands and gotchas live under **Repo reference** below and in **[CONTRIBUTING. | Date | Version | Change | |------|---------|--------| +| 2026-04-19 | 1.5.0 | Cross-repo impact (#794): `impact`/`query`/`context` accept `repo: "@"` + `service`. Removed `group_query`/`group_contracts`/`group_status` MCP tools; added `gitnexus://group/{name}/contracts` and `gitnexus://group/{name}/status` resources. | | 2026-04-16 | 1.4.0 | Fixed: web UI description, pre-commit behavior, MCP tools (7->16), added gitnexus-shared, removed stale vite-plugin-wasm gotcha. | | 2026-04-13 | 1.3.0 | Updated GitNexus index stats after DAG refactor. | | 2026-03-24 | 1.2.0 | Fixed gitnexus:start block duplication. | @@ -107,10 +108,12 @@ Indexed as **GitNexus** (4325 symbols, 10556 relationships, 300 execution flows) | `tool_map` | MCP/RPC tool definitions | `gitnexus_tool_map({})` | | `shape_check` | Response shape vs consumer access | `gitnexus_shape_check({route: "/api/users"})` | | `group_list` | List repo groups | `gitnexus_group_list({})` | -| `group_query` | Cross-repo search in a group | `gitnexus_group_query({name: "myGroup", query: "auth"})` | | `group_sync` | Rebuild group Contract Registry | `gitnexus_group_sync({name: "myGroup"})` | -| `group_contracts` | Inspect group contracts | `gitnexus_group_contracts({name: "myGroup"})` | -| `group_status` | Group staleness report | `gitnexus_group_status({name: "myGroup"})` | +| `query` (group mode) | Cross-repo search in a group (RRF-merged) | `gitnexus_query({repo: "@myGroup", query: "auth"})` | +| `context` (group mode) | 360° view across all member repos | `gitnexus_context({repo: "@myGroup", name: "validateUser"})` | +| `impact` (group mode) | Cross-repo blast radius via Contract Bridge | `gitnexus_impact({repo: "@myGroup", target: "X", direction: "upstream"})` | + +> Group mode: pass `repo: "@"` to fan out across all member repos, or `repo: "@/"` to target a single member (path keys from `group.yaml`). Optional `service: ""` filters by service root. Group-level state (contracts, staleness) lives in the resources table below — there are **no** `group_query` / `group_context` / `group_impact` / `group_contracts` / `group_status` MCP tools. ## Impact Risk Levels @@ -128,6 +131,8 @@ Indexed as **GitNexus** (4325 symbols, 10556 relationships, 300 execution flows) | `gitnexus://repo/GitNexus/clusters` | All functional areas | | `gitnexus://repo/GitNexus/processes` | All execution flows | | `gitnexus://repo/GitNexus/process/{name}` | Step-by-step execution trace | +| `gitnexus://group/{name}/contracts` | Group Contract Registry (provider/consumer rows + cross-links) | +| `gitnexus://group/{name}/status` | Per-member index + Contract Registry staleness report | ## Self-Check Before Finishing diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index d934cd8b6..ceb2f9583 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -42,10 +42,14 @@ Monorepo: **CLI/MCP** (`gitnexus/`) + **browser UI** (`gitnexus-web/`). | `tool_map` | MCP/RPC tool definitions and handlers | | `shape_check` | Response shape vs consumer property access mismatches | | `group_list` | List repo groups or details for one group | -| `group_query` | Cross-repo search in a group (reciprocal rank fusion) | -| `group_sync` | Rebuild group Contract Registry (`contracts.json`) | -| `group_contracts` | Inspect group contracts and cross-links | -| `group_status` | Index and Contract Registry staleness per repo in a group | +| `group_sync` | Rebuild group Contract Registry (`contracts.json`) and bridge graph | + +`query`, `context`, and `impact` are group-aware: pass `repo: "@"` (or `"@/"` to scope to one member) plus optional `service: ""`. Group-mode `query` merges per-repo results via Reciprocal Rank Fusion; group-mode `impact` runs the local walk in the chosen member and fans out across boundaries via the Contract Bridge (`gitnexus/src/core/group/cross-impact.ts`). The previously-planned `group_query`, `group_context`, `group_impact`, `group_contracts`, `group_status` MCP tools are intentionally not introduced — group-level state is exposed via resources instead: + +| Resource URI | Purpose | +|--------------|---------| +| `gitnexus://group/{name}/contracts` | Contract Registry (provider/consumer rows + cross-links) | +| `gitnexus://group/{name}/status` | Per-member index + Contract Registry staleness | ## Where to change what @@ -55,6 +59,7 @@ Monorepo: **CLI/MCP** (`gitnexus/`) + **browser UI** (`gitnexus-web/`). | Parsing/graph construction | `src/core/ingestion/pipeline-phases/` + `pipeline.ts` | | Graph schema/DB | `src/core/lbug/` (`schema.ts`, `lbug-adapter.ts`) | | MCP tools/resources | `src/mcp/server.ts`, `tools.ts`, `resources.ts` | +| Cross-repo groups (sync, contracts, `@` routing) | `src/core/group/` (`service.ts`, `cross-impact.ts`, `sync.ts`, `bridge-db.ts`) | | Search ranking | `src/core/search/` (BM25, hybrid fusion) | | Embeddings | `src/core/embeddings/` + `src/core/run-analyze.ts` | | Wiki generation | `src/core/wiki/` | diff --git a/gitnexus/CHANGELOG.md b/gitnexus/CHANGELOG.md index 203089d93..e4a5041ba 100644 --- a/gitnexus/CHANGELOG.md +++ b/gitnexus/CHANGELOG.md @@ -2,6 +2,12 @@ All notable changes to GitNexus will be documented in this file. +## [Unreleased] + +### Performance + +- **`analyze` ~33% faster** — moved FTS index creation from the analyze pipeline to first-use lazy initialisation. The 5 `CREATE_FTS_INDEX` calls cost ~440 ms each in LadybugDB regardless of table size (≈2 s fixed overhead) and dominated runtime on small repos and slow CI runners. The cost now amortises across the first `query`/`context` call in a session via a new `ensureFTSIndex` helper. Mini-repo `analyze` measured locally on Windows: 6.4 s → 4.0 s warm; on CI Windows runners (≈3× slower) restores comfortable headroom against the 30 s e2e test budget. + ## [1.6.2] - 2026-04-18 ### Added diff --git a/gitnexus/src/cli/ai-context.ts b/gitnexus/src/cli/ai-context.ts index 984d1432a..984a16f7b 100644 --- a/gitnexus/src/cli/ai-context.ts +++ b/gitnexus/src/cli/ai-context.ts @@ -121,7 +121,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 \`, \`npx gitnexus group impact --target --repo \`. +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 \`@\` or \`@/\` (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 \`, \`npx gitnexus group impact --target --repo \`. ` : '' diff --git a/gitnexus/src/cli/group.ts b/gitnexus/src/cli/group.ts index 70ca9537a..eb0dffc3d 100644 --- a/gitnexus/src/cli/group.ts +++ b/gitnexus/src/cli/group.ts @@ -184,6 +184,83 @@ 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 new file mode 100644 index 000000000..8739584c7 --- /dev/null +++ b/gitnexus/src/core/group/cross-impact.ts @@ -0,0 +1,562 @@ +/** + * Cross-repo impact (Phase 1 local walk + Phase 2 bridge fan-out). + * All bridge Cypher for this feature lives in this module. + */ + +import fsp from 'node:fs/promises'; +import path from 'node:path'; +import type { + BridgeHandle, + ContractType, + CrossRepoImpact, + GroupConfig, + GroupImpactResult, + MatchType, + OutOfScopeLink, +} from './types.js'; +import type { GroupRepoHandle, GroupToolPort } from './service.js'; +import { loadGroupConfig } from './config-parser.js'; +import { + fileMatchesServicePrefix, + normalizeServicePrefix, + repoInSubgroup, +} from './group-path-utils.js'; +import { getGroupDir } from './storage.js'; +import { closeBridgeDb, openBridgeDbReadOnly, queryBridge, readBridgeMeta } from './bridge-db.js'; +import { BRIDGE_SCHEMA_VERSION } from './bridge-schema.js'; + +/** Cross-boundary hops beyond this value are clamped (multi-hop reserved for future work). */ +export const MAX_SUPPORTED_CROSS_DEPTH = 1; + +/** Default wall-clock budget for the Phase 1 `impact` leg when callers omit `timeoutMs`. */ +export const DEFAULT_LOCAL_IMPACT_TIMEOUT_MS = 30_000; + +const CY_NEIGHBORS_UPSTREAM = ` +MATCH (consumer:Contract)-[l:ContractLink]->(provider:Contract) +WHERE provider.repo = $localRepo + AND provider.symbolUid IN $uids + AND provider.role = 'provider' +RETURN consumer.repo AS neighborRepo, + consumer.symbolUid AS neighborUid, + consumer.filePath AS neighborFilePath, + l.matchType AS matchType, + l.confidence AS confidence, + l.contractId AS contractId, + consumer.type AS contractType +`; + +const CY_NEIGHBORS_DOWNSTREAM = ` +MATCH (consumer:Contract)-[l:ContractLink]->(provider:Contract) +WHERE consumer.repo = $localRepo + AND consumer.symbolUid IN $uids + AND consumer.role = 'consumer' +RETURN provider.repo AS neighborRepo, + provider.symbolUid AS neighborUid, + provider.filePath AS neighborFilePath, + l.matchType AS matchType, + l.confidence AS confidence, + l.contractId AS contractId, + provider.type AS contractType +`; + +type BridgeNeighborRow = { + neighborRepo: string; + neighborUid: string; + neighborFilePath?: string; + matchType: string; + confidence: number; + contractId: string; + contractType: string; +}; + +export interface RunGroupImpactDeps { + port: GroupToolPort; + gitnexusDir: string; +} + +function parseDirection(raw: unknown): 'upstream' | 'downstream' | null { + if (raw === 'upstream' || raw === 'downstream') return raw; + return null; +} + +function clampCrossDepth(raw: unknown): { depth: number; warning?: string } { + const n = typeof raw === 'number' && Number.isFinite(raw) ? Math.floor(raw) : 1; + const d = n < 1 ? 1 : n; + if (d > MAX_SUPPORTED_CROSS_DEPTH) { + return { + depth: MAX_SUPPORTED_CROSS_DEPTH, + warning: `crossDepth was ${d}; multi-hop cross-boundary traversal beyond ${MAX_SUPPORTED_CROSS_DEPTH} is not implemented yet. Using crossDepth ${MAX_SUPPORTED_CROSS_DEPTH}.`, + }; + } + return { depth: d }; +} + +export function validateGroupImpactParams(params: Record): + | { + ok: true; + name: string; + repoPath: string; + target: string; + direction: 'upstream' | 'downstream'; + maxDepth: number; + crossDepth: number; + crossDepthWarning?: string; + relationTypes?: string[]; + includeTests: boolean; + minConfidence: number; + service?: string; + subgroup?: string; + timeoutMs: number; + } + | { ok: false; error: string } { + const name = String(params.name ?? '').trim(); + const repoPath = String(params.repo ?? '').trim(); + const target = String(params.target ?? '').trim(); + 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' }; + + let maxDepth = typeof params.maxDepth === 'number' && params.maxDepth > 0 ? params.maxDepth : 3; + if (maxDepth > 32) maxDepth = 32; + + const { depth: crossDepth, warning: crossDepthWarning } = clampCrossDepth(params.crossDepth); + + const relationTypes = Array.isArray(params.relationTypes) + ? params.relationTypes.filter((t): t is string => typeof t === 'string') + : undefined; + + const includeTests = Boolean(params.includeTests); + let minConfidence = typeof params.minConfidence === 'number' ? params.minConfidence : 0; + if (minConfidence < 0) minConfidence = 0; + if (minConfidence > 1) minConfidence = 1; + + const service = normalizeServicePrefix(params.service); + const subgroup = typeof params.subgroup === 'string' ? params.subgroup : undefined; + + let timeoutMs = + typeof params.timeoutMs === 'number' && params.timeoutMs > 0 + ? params.timeoutMs + : typeof params.timeout === 'number' && params.timeout > 0 + ? params.timeout + : DEFAULT_LOCAL_IMPACT_TIMEOUT_MS; + if (timeoutMs > 3_600_000) timeoutMs = 3_600_000; + + return { + ok: true, + name, + repoPath, + target, + direction, + maxDepth, + crossDepth, + crossDepthWarning, + relationTypes, + includeTests, + minConfidence, + service, + subgroup, + timeoutMs, + }; +} + +async function resolveGroupRepo( + port: GroupToolPort, + config: GroupConfig, + repoPath: string, +): Promise { + const registryName = config.repos[repoPath]; + if (!registryName) { + return { error: `Unknown repo path "${repoPath}" in this group.` }; + } + try { + return await port.resolveRepo(registryName); + } catch (e) { + return { error: e instanceof Error ? e.message : String(e) }; + } +} + +async function safeLocalImpact( + port: GroupToolPort, + repo: GroupRepoHandle, + impactParams: Parameters[1], + timeoutMs: number, +): Promise<{ value: unknown; timedOut: boolean }> { + let timer: ReturnType | undefined; + const impactP = port.impact(repo, impactParams).catch((err) => ({ + error: err instanceof Error ? err.message : String(err), + })); + const timeoutP = new Promise<'timeout'>((resolve) => { + timer = setTimeout(() => resolve('timeout'), timeoutMs); + }); + const won = await Promise.race([ + impactP.then((v) => ({ tag: 'impact' as const, v })), + timeoutP.then(() => ({ tag: 'timeout' as const })), + ]); + if (timer !== undefined) clearTimeout(timer); + if (won.tag === 'timeout') { + return { + value: { error: 'Local impact timed out', partial: true }, + timedOut: true, + }; + } + return { value: won.v, timedOut: false }; +} + +export function collectImpactSymbolUids( + local: unknown, + servicePrefix: string | undefined, +): { uids: string[]; targetFilePath?: string } { + const uids = new Set(); + let targetFilePath: string | undefined; + const obj = local as Record | null; + if (!obj || typeof obj !== 'object') return { uids: [], targetFilePath }; + + const target = obj.target as { id?: string; filePath?: string } | undefined; + if (target?.id) { + targetFilePath = typeof target.filePath === 'string' ? target.filePath : undefined; + if (fileMatchesServicePrefix(targetFilePath, servicePrefix)) { + uids.add(String(target.id)); + } + } + + const byDepth = obj.byDepth as Record | undefined; + if (byDepth && typeof byDepth === 'object') { + for (const items of Object.values(byDepth)) { + if (!Array.isArray(items)) continue; + for (const it of items) { + const row = it as { id?: string; filePath?: string }; + if (row?.id && fileMatchesServicePrefix(row.filePath, servicePrefix)) { + uids.add(String(row.id)); + } + } + } + } + return { uids: [...uids], targetFilePath }; +} + +function extractProcessNames(impact: unknown): string[] { + const o = impact as { affected_processes?: Array<{ name?: string }> }; + if (!o?.affected_processes) return []; + return o.affected_processes.map((p) => String(p.name ?? '')).filter(Boolean); +} + +function mergeRisk(localRisk: string, cross: CrossRepoImpact[]): string { + const highConf = cross.some((c) => c.contract.confidence >= 0.85); + if (localRisk === 'CRITICAL') return 'CRITICAL'; + if (cross.length >= 3) return 'CRITICAL'; + if (highConf) return 'HIGH'; + if (cross.length > 0 && (localRisk === 'LOW' || localRisk === 'UNKNOWN')) return 'MEDIUM'; + return localRisk; +} + +async function ensureBridgeReady( + groupDir: string, +): Promise<{ handle: BridgeHandle } | { error: string }> { + const meta = await readBridgeMeta(groupDir); + if (meta.version > 0 && meta.version !== BRIDGE_SCHEMA_VERSION) { + return { + error: `Bridge schema version mismatch (meta.json has ${meta.version}, expected ${BRIDGE_SCHEMA_VERSION}). Run gitnexus group sync for this group.`, + }; + } + const dbPath = path.join(groupDir, 'bridge.lbug'); + try { + await fsp.access(dbPath); + } catch { + return { + error: `No bridge.lbug in this group directory. Run gitnexus group sync (schema ${BRIDGE_SCHEMA_VERSION}).`, + }; + } + const handle = await openBridgeDbReadOnly(groupDir); + if (!handle) { + return { + error: `Could not open bridge.lbug read-only (schema ${BRIDGE_SCHEMA_VERSION}). Run gitnexus group sync.`, + }; + } + return { handle }; +} + +function rowToNeighbor(r: Record): BridgeNeighborRow | null { + const neighborRepo = String(r.neighborRepo ?? r[0] ?? ''); + const neighborUid = String(r.neighborUid ?? r[1] ?? ''); + if (!neighborRepo || !neighborUid) return null; + return { + neighborRepo, + neighborUid, + neighborFilePath: + r.neighborFilePath !== undefined ? String(r.neighborFilePath) : String(r[2] ?? ''), + matchType: String(r.matchType ?? r[3] ?? 'exact'), + confidence: Number(r.confidence ?? r[4] ?? 0), + contractId: String(r.contractId ?? r[5] ?? ''), + contractType: String(r.contractType ?? r[6] ?? 'custom'), + }; +} + +export async function runGroupImpact( + deps: RunGroupImpactDeps, + params: Record, +): Promise { + const parsed = validateGroupImpactParams(params); + if (parsed.ok === false) return { error: parsed.error }; + + const { + name, + repoPath, + target, + direction, + maxDepth, + crossDepth: _crossDepth, + crossDepthWarning, + relationTypes, + includeTests, + minConfidence, + service: servicePrefix, + subgroup, + timeoutMs, + } = parsed; + + const groupDir = getGroupDir(deps.gitnexusDir, name); + let config: GroupConfig; + try { + config = await loadGroupConfig(groupDir); + } catch (e) { + return { error: e instanceof Error ? e.message : String(e) }; + } + + const resolved = await resolveGroupRepo(deps.port, config, repoPath); + if ('error' in resolved) return { error: resolved.error }; + + const impactParams: Parameters[1] = { + target, + direction, + maxDepth, + relationTypes: relationTypes && relationTypes.length > 0 ? relationTypes : undefined, + includeTests, + minConfidence, + }; + + // Single shared deadline for Phase 1 (local walk) + Phase 2 (bridge fan-out). + // Phase 1 still gets the full budget; Phase 2 only uses whatever wall-clock + // time is left, so total work cannot exceed `timeoutMs`. + const deadline = Date.now() + Math.max(0, timeoutMs); + + const { value: local, timedOut: localTimedOut } = await safeLocalImpact( + deps.port, + resolved, + impactParams, + timeoutMs, + ); + + if (localTimedOut) { + const base = local as Record; + return { + local, + group: name, + cross: [], + outOfScope: [], + truncated: true, + truncatedRepos: [], + summary: { + direct: 0, + processes_affected: 0, + modules_affected: 0, + cross_repo_hits: 0, + }, + risk: 'UNKNOWN', + timeoutMs, + truncationReason: 'timeout', + crossDepthWarning, + }; + } + + const localObj = local as Record | null; + if (localObj?.error && typeof localObj.error === 'string') { + const empty: GroupImpactResult = { + local, + group: name, + cross: [], + outOfScope: [], + truncated: false, + truncatedRepos: [], + summary: { + direct: 0, + processes_affected: 0, + modules_affected: 0, + cross_repo_hits: 0, + }, + risk: 'UNKNOWN', + timeoutMs, + crossDepthWarning, + }; + return empty; + } + + if (servicePrefix) { + const tf = (localObj?.target as { filePath?: string } | undefined)?.filePath; + if (!fileMatchesServicePrefix(tf, servicePrefix)) { + return { + local: {}, + group: name, + cross: [], + outOfScope: [], + truncated: false, + truncatedRepos: [], + summary: { + direct: 0, + processes_affected: 0, + modules_affected: 0, + cross_repo_hits: 0, + }, + risk: 'LOW', + timeoutMs, + crossDepthWarning, + }; + } + } + + const { uids } = collectImpactSymbolUids(local, servicePrefix); + if (uids.length === 0) { + const s = (local as { summary?: Record })?.summary || {}; + return { + local, + group: name, + cross: [], + outOfScope: [], + truncated: Boolean((local as { partial?: boolean }).partial), + truncatedRepos: [], + summary: { + direct: s.direct ?? 0, + processes_affected: s.processes_affected ?? 0, + modules_affected: s.modules_affected ?? 0, + cross_repo_hits: 0, + }, + risk: String((local as { risk?: string }).risk ?? 'LOW'), + timeoutMs, + truncationReason: (local as { partial?: boolean }).partial ? 'partial' : undefined, + crossDepthWarning, + }; + } + + const bridgePrep = await ensureBridgeReady(groupDir); + if ('error' in bridgePrep) return { error: bridgePrep.error }; + + const handle = bridgePrep.handle; + const cross: CrossRepoImpact[] = []; + const outOfScope: OutOfScopeLink[] = []; + const truncatedRepos: string[] = []; + + try { + const cypher = direction === 'upstream' ? CY_NEIGHBORS_UPSTREAM : CY_NEIGHBORS_DOWNSTREAM; + const rows = await queryBridge>(handle, cypher, { + localRepo: repoPath, + uids, + }); + + const neighbors: BridgeNeighborRow[] = []; + for (const raw of rows) { + const n = rowToNeighbor(raw); + if (n) neighbors.push(n); + } + neighbors.sort((a, b) => b.confidence - a.confidence); + + const seen = new Set(); + + for (const n of neighbors) { + if (servicePrefix && !fileMatchesServicePrefix(n.neighborFilePath, servicePrefix)) { + continue; + } + if (!repoInSubgroup(n.neighborRepo, subgroup)) { + // CrossLink convention: consumer -> provider + outOfScope.push({ + from: direction === 'upstream' ? n.neighborRepo : repoPath, + to: direction === 'upstream' ? repoPath : n.neighborRepo, + contractId: n.contractId, + confidence: n.confidence, + }); + continue; + } + + const key = `${n.neighborRepo}\0${n.neighborUid}\0${n.contractId}`; + if (seen.has(key)) continue; + seen.add(key); + + if (Date.now() > deadline) { + truncatedRepos.push(n.neighborRepo); + continue; + } + + const regName = config.repos[n.neighborRepo]; + if (!regName) continue; + + let neighborHandle: GroupRepoHandle; + try { + neighborHandle = await deps.port.resolveRepo(regName); + } catch { + truncatedRepos.push(n.neighborRepo); + continue; + } + + const fan = await deps.port.impactByUid(neighborHandle.id, n.neighborUid, direction, { + maxDepth, + relationTypes: relationTypes ?? [], + minConfidence, + includeTests, + }); + if (fan == null) { + truncatedRepos.push(n.neighborRepo); + continue; + } + + cross.push({ + repo: regName, + repo_path: n.neighborRepo, + contract: { + id: n.contractId, + type: n.contractType as ContractType, + match_type: (n.matchType as MatchType) || 'exact', + confidence: n.confidence, + }, + by_depth: ((fan as { byDepth?: unknown }).byDepth ?? {}) as Record, + affected_processes: extractProcessNames(fan), + }); + } + } finally { + await closeBridgeDb(handle); + } + + const localSum = (local as { summary?: Record })?.summary || {}; + const localRisk = String((local as { risk?: string }).risk ?? 'LOW'); + const localPartial = Boolean((local as { partial?: boolean }).partial); + const truncated = truncatedRepos.length > 0 || localPartial; + + const result: GroupImpactResult = { + local, + group: name, + cross, + outOfScope, + truncated, + truncatedRepos: [...new Set(truncatedRepos)], + summary: { + direct: localSum.direct ?? 0, + processes_affected: localSum.processes_affected ?? 0, + modules_affected: localSum.modules_affected ?? 0, + cross_repo_hits: cross.length, + }, + risk: mergeRisk(localRisk, cross), + timeoutMs, + truncationReason: truncated ? 'partial' : undefined, + crossDepthWarning, + }; + return result; +} + +export { normalizeServicePrefix, fileMatchesServicePrefix } from './group-path-utils.js'; diff --git a/gitnexus/src/core/group/group-path-utils.ts b/gitnexus/src/core/group/group-path-utils.ts new file mode 100644 index 000000000..ab6a1effb --- /dev/null +++ b/gitnexus/src/core/group/group-path-utils.ts @@ -0,0 +1,42 @@ +/** + * Shared service-path normalization for group tools (`service` monorepo filter) + * and subgroup membership checks. + * + * Inputs may originate from tree-sitter, the OS file API, or user-supplied + * MCP arguments, so both `\` and `/` separators are accepted. Internally we + * normalize to POSIX-style `/` for case-sensitive segment comparisons. + */ + +function toPosix(p: string): string { + return p.replace(/\\/g, '/'); +} + +export function normalizeServicePrefix(service: unknown): string | undefined { + if (service === undefined || service === null) return undefined; + const s = toPosix(String(service)).trim().replace(/\/+$/, ''); + return s.length > 0 ? s : undefined; +} + +export function fileMatchesServicePrefix( + filePath: string | undefined, + prefix: string | undefined, +): boolean { + if (!prefix) return true; + if (!filePath) return false; + const normalized = toPosix(filePath); + return normalized === prefix || normalized.startsWith(`${prefix}/`); +} + +/** + * True if `repoPath` is at or beneath `subgroup` (member-path prefix in + * `group.yaml`). Empty / missing `subgroup` matches every repo. + * + * @param exact When set, requires an exact equality match (no descendant repos). + */ +export function repoInSubgroup(repoPath: string, subgroup?: string, exact?: boolean): boolean { + if (!subgroup?.trim()) return true; + const s = toPosix(subgroup).replace(/\/+$/, ''); + const r = toPosix(repoPath); + if (exact) return r === s; + return r === s || r.startsWith(`${s}/`); +} 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 1530cd6dd..afbb66e0e 100644 --- a/gitnexus/src/core/group/service.ts +++ b/gitnexus/src/core/group/service.ts @@ -3,10 +3,24 @@ * DB access is injected via GroupToolPort so this module stays free of LocalBackend private API. */ +import fsp from 'node:fs/promises'; +import path from 'node:path'; import { checkStaleness } from '../git-staleness.js'; import { loadGroupConfig } from './config-parser.js'; +import { + fileMatchesServicePrefix, + normalizeServicePrefix, + repoInSubgroup, +} from './group-path-utils.js'; import { getDefaultGitnexusDir, getGroupDir, listGroups, readContractRegistry } from './storage.js'; import { syncGroup } from './sync.js'; +import type { + ContractRegistry, + CrossLink, + GroupConfig, + GroupContextResult, + StoredContract, +} from './types.js'; export interface GroupRepoHandle { id: string; @@ -52,12 +66,149 @@ export interface GroupToolPort { includeTests: boolean; }, ): Promise; + context( + repo: GroupRepoHandle, + params: { + name?: string; + uid?: string; + file_path?: string; + include_content?: boolean; + }, + ): Promise; } -function repoInSubgroup(repoPath: string, subgroup?: string): boolean { - if (!subgroup?.trim()) return true; - const s = subgroup.replace(/\/+$/, ''); - return repoPath === s || repoPath.startsWith(`${s}/`); +function isStoredContract(raw: unknown): raw is StoredContract { + if (!raw || typeof raw !== 'object') return false; + const o = raw as Record; + return ( + typeof o.contractId === 'string' && + typeof o.type === 'string' && + typeof o.repo === 'string' && + typeof o.role === 'string' && + (o.role === 'provider' || o.role === 'consumer') && + typeof o.symbolUid === 'string' && + typeof o.symbolName === 'string' && + typeof o.confidence === 'number' && + o.meta !== undefined && + typeof o.meta === 'object' && + o.meta !== null && + o.symbolRef !== undefined && + typeof o.symbolRef === 'object' && + o.symbolRef !== null && + typeof (o.symbolRef as Record).filePath === 'string' && + typeof (o.symbolRef as Record).name === 'string' + ); +} + +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; + const from = o.from as Record | undefined; + const to = o.to as Record | undefined; + if (!from || !to) return false; + if (typeof from.repo !== 'string' || typeof to.repo !== 'string') return false; + return typeof o.contractId === 'string' && typeof o.type === 'string'; +} + +async function loadContractRegistryResilient( + groupDir: string, +): Promise< + { ok: true; registry: ContractRegistry; skippedCorrupt: number } | { ok: false; error: string } +> { + const filePath = path.join(groupDir, 'contracts.json'); + let raw: string; + try { + raw = await fsp.readFile(filePath, 'utf-8'); + } catch (e) { + if ((e as NodeJS.ErrnoException).code === 'ENOENT') { + return { ok: false, error: `No contracts.json for this group. Run group_sync first.` }; + } + return { ok: false, error: e instanceof Error ? e.message : String(e) }; + } + + let root: unknown; + try { + root = JSON.parse(raw); + } catch { + return { ok: false, error: 'contracts.json is not valid JSON' }; + } + + if (!root || typeof root !== 'object' || Array.isArray(root)) { + return { ok: false, error: 'contracts.json has an invalid root object' }; + } + + const base = root as Record; + const contractsRaw = base.contracts; + const crossRaw = base.crossLinks; + let skippedCorrupt = 0; + + const contracts: StoredContract[] = []; + if (Array.isArray(contractsRaw)) { + for (const row of contractsRaw) { + try { + if (isStoredContract(row)) { + contracts.push(row); + } else { + skippedCorrupt++; + console.warn('[group] skipping corrupt contract row in contracts.json'); + } + } catch { + skippedCorrupt++; + console.warn('[group] skipping corrupt contract row in contracts.json'); + } + } + } + + const crossLinks: CrossLink[] = []; + if (Array.isArray(crossRaw)) { + for (const row of crossRaw) { + try { + if (isCrossLink(row)) { + crossLinks.push(row); + } else { + skippedCorrupt++; + console.warn('[group] skipping corrupt crossLinks row in contracts.json'); + } + } catch { + skippedCorrupt++; + console.warn('[group] skipping corrupt crossLinks row in contracts.json'); + } + } + } + + const registry: ContractRegistry = { + version: typeof base.version === 'number' ? base.version : 0, + generatedAt: typeof base.generatedAt === 'string' ? base.generatedAt : '', + repoSnapshots: + base.repoSnapshots && typeof base.repoSnapshots === 'object' && base.repoSnapshots !== null + ? (base.repoSnapshots as Record) + : {}, + missingRepos: Array.isArray(base.missingRepos) ? (base.missingRepos as string[]) : [], + contracts, + crossLinks, + }; + + return { ok: true, registry, skippedCorrupt }; } export class GroupService { @@ -103,10 +254,14 @@ export class GroupService { const name = String(params.name ?? '').trim(); if (!name) return { error: 'name is required' }; const groupDir = getGroupDir(getDefaultGitnexusDir(), name); - const registry = await readContractRegistry(groupDir); - if (!registry) { - return { error: `No contracts.json for group "${name}". Run group_sync first.` }; + const loaded = await loadContractRegistryResilient(groupDir); + if (loaded.ok === false) { + if (loaded.error.includes('No contracts.json')) { + return { error: `No contracts.json for group "${name}". Run group_sync first.` }; + } + return { error: loaded.error }; } + const { registry, skippedCorrupt } = loaded; let contracts = registry.contracts; if (params.type) contracts = contracts.filter((c) => c.type === params.type); if (params.repo) contracts = contracts.filter((c) => c.repo === params.repo); @@ -119,41 +274,151 @@ export class GroupService { ); contracts = contracts.filter((c) => !matchedIds.has(`${c.repo}::${c.contractId}`)); } - return { contracts, crossLinks: registry.crossLinks }; + const out: Record = { contracts, crossLinks: registry.crossLinks }; + if (skippedCorrupt > 0) out.skippedCorrupt = skippedCorrupt; + return out; + } + + async groupImpact(params: Record): Promise { + const { runGroupImpact } = await import('./cross-impact.js'); + return runGroupImpact({ port: this.port, gitnexusDir: getDefaultGitnexusDir() }, params); + } + + async groupContext(params: Record): Promise { + const name = String(params.name ?? '').trim(); + const target = typeof params.target === 'string' ? params.target.trim() : ''; + 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; + const subgroupExact = params.subgroupExact === true; + + if (!name) { + return { group: '', error: 'name is required', results: [] }; + } + if (!uid && !target) { + return { group: name, error: 'target or uid is required', results: [] }; + } + + const groupDir = getGroupDir(getDefaultGitnexusDir(), name); + let config: GroupConfig; + try { + config = await loadGroupConfig(groupDir); + } catch (e) { + return { + group: name, + target: target || uid, + service: servicePrefix, + error: e instanceof Error ? e.message : String(e), + results: [], + }; + } + + const memberEntries = Object.entries(config.repos).filter(([repoPath]) => + repoInSubgroup(repoPath, subgroup, subgroupExact), + ); + + // Per-repo work is independent (each repo opens its own DB handle and the + // group-level result preserves repo iteration order via the indexed map). + // Errors are caught per repo so one slow/failed member does not block the rest. + const results: GroupContextResult['results'] = await Promise.all( + memberEntries.map(async ([repoPath, registryName]) => { + try { + const repoObj = await this.port.resolveRepo(registryName); + const payload = await this.port.context(repoObj, { + name: target || undefined, + uid, + file_path, + include_content, + }); + + if (servicePrefix) { + const st = (payload as { status?: string })?.status; + const sym = (payload as { symbol?: { filePath?: string } })?.symbol; + if (st === 'found' && !fileMatchesServicePrefix(sym?.filePath, servicePrefix)) { + return { repoPath, registryName, payload: {} }; + } + } + + return { repoPath, registryName, payload }; + } catch (e) { + return { + repoPath, + registryName, + payload: { error: e instanceof Error ? e.message : String(e) }, + }; + } + }), + ); + + return { + group: name, + target: target || uid, + service: servicePrefix, + results, + }; } async groupQuery(params: Record): Promise { 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; + 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; - try { - const repoObj = await this.port.resolveRepo(registryName); - const queryResult = (await this.port.query(repoObj, { - query: queryText, - limit, - max_symbols: 10, - include_content: false, - })) as { processes?: Array> }; - const processes = queryResult.processes || []; - const scored = processes.map((p, idx) => ({ - ...p, - _rrf_score: 1 / (idx + 1 + 60), - _repo: repoPath, - })); - perRepo.push({ repo: repoPath, score: 0, processes: scored }); - } catch { - perRepo.push({ repo: repoPath, score: 0, processes: [] }); - } - } + const memberEntries = Object.entries(config.repos).filter(([repoPath]) => + repoInSubgroup(repoPath, subgroup, subgroupExact), + ); + + // Per-repo query is independent; run them concurrently and isolate + // failures so one slow/failed member does not block the rest. + const perRepo = await Promise.all( + memberEntries.map(async ([repoPath, registryName]) => { + try { + const repoObj = await this.port.resolveRepo(registryName); + const queryResult = (await this.port.query(repoObj, { + query: queryText, + limit, + max_symbols: 10, + include_content: false, + })) 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), + _repo: repoPath, + })); + return { repo: repoPath, score: 0, processes: scored as unknown[] }; + } catch { + return { repo: repoPath, score: 0, processes: [] as unknown[] }; + } + }), + ); const allProcesses = perRepo.flatMap((r) => r.processes as Array>); allProcesses.sort((a, b) => (b._rrf_score as number) - (a._rrf_score as number)); @@ -184,13 +449,10 @@ export class GroupService { } > = {}; - const fsp = await import('node:fs/promises'); - const pathMod = await import('node:path'); - for (const [repoPath, registryName] of Object.entries(config.repos)) { try { const repoObj = await this.port.resolveRepo(registryName); - const metaPath = pathMod.join(repoObj.storagePath, 'meta.json'); + const metaPath = path.join(repoObj.storagePath, 'meta.json'); const metaRaw = await fsp.readFile(metaPath, 'utf-8').catch(() => '{}'); const meta = JSON.parse(metaRaw) as { lastCommit?: string; indexedAt?: string }; diff --git a/gitnexus/src/core/group/types.ts b/gitnexus/src/core/group/types.ts index b9ba97582..793d3d0ad 100644 --- a/gitnexus/src/core/group/types.ts +++ b/gitnexus/src/core/group/types.ts @@ -96,6 +96,9 @@ export interface RepoHandle { storagePath: string; } +/** Why local impact or fan-out stopped early (e.g. wall-clock budget exhausted). */ +export type GroupImpactTruncationReason = 'timeout' | 'partial'; + export interface GroupImpactResult { local: unknown; group: string; @@ -110,6 +113,36 @@ export interface GroupImpactResult { cross_repo_hits: number; }; risk: string; + /** + * Milliseconds budget applied to the **Phase 1 local impact** leg (`safeLocalImpact`). + * If the walk hits this wall first, expect `truncationReason: 'timeout'` and a partial `local` payload. + */ + timeoutMs?: number; + /** Present when local impact or fan-out stopped early (timeout, graph cap, etc.). */ + truncationReason?: GroupImpactTruncationReason; + /** + * Human-readable note when `crossDepth` was clamped (e.g. multi-hop not implemented yet). + */ + crossDepthWarning?: string; +} + +/** One repo’s `context` tool payload in a group-scoped context run. */ +export interface GroupContextRepoEntry { + repoPath: string; + registryName: string; + payload: unknown; +} + +/** + * Aggregated group `context`: explicit per-repo rows (no merged symbol payloads). + * Use top-level `error` only for unrecoverable failures, not for “no matches” or service scope misses. + */ +export interface GroupContextResult { + group: string; + target?: string; + service?: string; + error?: string; + results: GroupContextRepoEntry[]; } export interface CrossRepoImpact { diff --git a/gitnexus/src/core/lbug/lbug-adapter.ts b/gitnexus/src/core/lbug/lbug-adapter.ts index f019960c5..e4190c323 100644 --- a/gitnexus/src/core/lbug/lbug-adapter.ts +++ b/gitnexus/src/core/lbug/lbug-adapter.ts @@ -144,6 +144,18 @@ let currentDbPath: string | null = null; let ftsLoaded = false; let vectorExtensionLoaded = false; +/** + * In-process cache of FTS indexes that have been ensured against the current + * connection. Prevents repeated `CALL CREATE_FTS_INDEX` round-trips inside a + * single CLI/MCP session — the first call to `ensureFTSIndex` for a given + * `(tableName, indexName)` pays the LadybugDB cost (~440 ms even when the + * index already exists on disk), subsequent calls are a Set lookup. Cleared + * by `closeLbug` so a re-init starts fresh. + * + * Key format: `${tableName}:${indexName}`. + */ +const ensuredFTSIndexes = new Set(); + /** * Check if an error indicates a missing column or table (schema-level problem) * rather than a transient/connection error. Used for legacy DB fallback logic. @@ -1037,6 +1049,7 @@ export const closeLbug = async (): Promise => { currentDbPath = null; ftsLoaded = false; vectorExtensionLoaded = false; + ensuredFTSIndexes.clear(); }; export const isLbugReady = (): boolean => conn !== null && db !== null; @@ -1219,6 +1232,29 @@ export const createFTSIndex = async ( } }; +/** + * Lazy-create an FTS index, caching the fact in-process. + * + * Used by `queryFTS` so that `analyze` doesn't pay the ~440 ms × 5 fixed + * LadybugDB cost up-front (it dominates analyze on small repos). Instead, + * the cost is moved to the first `query`/`context` call in a session, + * where it's amortised across many lookups. + * + * Safe to call repeatedly — the in-process Set guarantees only the first + * call hits LadybugDB. `closeLbug` clears the cache so re-init starts fresh. + */ +export const ensureFTSIndex = async ( + tableName: string, + indexName: string, + properties: string[], + stemmer: string = 'porter', +): Promise => { + const key = `${tableName}:${indexName}`; + if (ensuredFTSIndexes.has(key)) return; + await createFTSIndex(tableName, indexName, properties, stemmer); + ensuredFTSIndexes.add(key); +}; + /** * Query a full-text search index * @param tableName - The node table name diff --git a/gitnexus/src/core/run-analyze.ts b/gitnexus/src/core/run-analyze.ts index 5c2191003..e61c20f21 100644 --- a/gitnexus/src/core/run-analyze.ts +++ b/gitnexus/src/core/run-analyze.ts @@ -19,7 +19,6 @@ import { executeQuery, executeWithReusedStatement, closeLbug, - createFTSIndex, loadCachedEmbeddings, } from './lbug/lbug-adapter.js'; import { @@ -215,17 +214,12 @@ export async function runFullAnalysis( }); // ── Phase 3: FTS (85–90%) ───────────────────────────────────────── - progress('fts', 85, 'Creating search indexes...'); - - try { - await createFTSIndex('File', 'file_fts', ['name', 'content']); - await createFTSIndex('Function', 'function_fts', ['name', 'content']); - await createFTSIndex('Class', 'class_fts', ['name', 'content']); - await createFTSIndex('Method', 'method_fts', ['name', 'content']); - await createFTSIndex('Interface', 'interface_fts', ['name', 'content']); - } catch { - // Non-fatal — FTS is best-effort - } + // FTS indexes are created lazily on first `query`/`context` call instead + // of eagerly here. On small repos / CI runners the LadybugDB + // CREATE_FTS_INDEX cost is ~440 ms × 5 (≈2 s) regardless of table size, + // which dominated `analyze` runtime and pushed Windows CI past its + // 30 s test budget. Lazy creation is implemented in + // `core/search/bm25-index.ts` via `ensureFTSIndex`. // ── Phase 3.5: Re-insert cached embeddings ──────────────────────── if (cachedEmbeddings.length > 0) { diff --git a/gitnexus/src/core/search/bm25-index.ts b/gitnexus/src/core/search/bm25-index.ts index ae433ad28..040440999 100644 --- a/gitnexus/src/core/search/bm25-index.ts +++ b/gitnexus/src/core/search/bm25-index.ts @@ -3,9 +3,15 @@ * * Uses LadybugDB's built-in full-text search indexes for keyword-based search. * Always reads from the database (no cached state to drift). + * + * FTS indexes are created lazily on first query (via `ensureFTSIndex`) — see + * `lbug-adapter.ts` for the rationale. This keeps `analyze` fast (the + * ~440 ms × 5 LadybugDB CREATE_FTS_INDEX cost dominates pipeline time on + * small repos / CI runners) at the cost of paying that overhead on the + * first `query`/`context` call in a session. */ -import { queryFTS } from '../lbug/lbug-adapter.js'; +import { queryFTS, ensureFTSIndex } from '../lbug/lbug-adapter.js'; export interface BM25SearchResult { filePath: string; @@ -13,6 +19,56 @@ export interface BM25SearchResult { rank: number; } +/** + * FTS schema served by `searchFTSFromLbug`. Centralised so that both the + * CLI/pipeline path and the MCP pool path use identical (table, index, + * properties) tuples and the lazy-create logic stays in one place. + */ +const FTS_INDEXES: ReadonlyArray<{ + table: string; + indexName: string; + properties: readonly string[]; +}> = [ + { table: 'File', indexName: 'file_fts', properties: ['name', 'content'] }, + { table: 'Function', indexName: 'function_fts', properties: ['name', 'content'] }, + { table: 'Class', indexName: 'class_fts', properties: ['name', 'content'] }, + { table: 'Method', indexName: 'method_fts', properties: ['name', 'content'] }, + { table: 'Interface', indexName: 'interface_fts', properties: ['name', 'content'] }, +]; + +/** + * Per-process cache for the MCP pool path: tracks which `(repoId, table)` + * pairs have been ensured. The CLI/pipeline path gets its own cache inside + * `lbug-adapter.ts` keyed by table/index, scoped to the singleton connection. + */ +const ensuredPoolFTS = new Set(); + +async function ensureFTSIndexViaExecutor( + executor: (cypher: string) => Promise, + repoId: string, + table: string, + indexName: string, + properties: readonly string[], +): Promise { + const key = `${repoId}:${table}:${indexName}`; + if (ensuredPoolFTS.has(key)) return; + const propList = properties.map((p) => `'${p}'`).join(', '); + try { + await executor( + `CALL CREATE_FTS_INDEX('${table}', '${indexName}', [${propList}], stemmer := 'porter')`, + ); + } catch (e: any) { + // 'already exists' is the happy path (index persists on disk between + // process invocations) — anything else we swallow because FTS is + // best-effort: queryFTS itself returns [] on missing-index errors. + const msg = String(e?.message ?? ''); + if (!msg.includes('already exists')) { + // Best-effort — continue without index, queryFTS will fall back to []. + } + } + ensuredPoolFTS.add(key); +} + /** * Execute a single FTS query via a custom executor (for MCP connection pool). * Returns the same shape as core queryFTS (from LadybugDB adapter). @@ -75,6 +131,13 @@ export const searchFTSFromLbug = async ( // The MCP pool supports multiple connections, but FTS is best run serially. const { executeQuery } = await import('../lbug/pool-adapter.js'); const executor = (cypher: string) => executeQuery(repoId, cypher); + + // Lazy-create FTS indexes on first query for this repo (analyze no longer + // creates them up-front, so we ensure them here). Cached per-process. + for (const { table, indexName, properties } of FTS_INDEXES) { + await ensureFTSIndexViaExecutor(executor, repoId, table, indexName, properties); + } + fileResults = await queryFTSViaExecutor(executor, 'File', 'file_fts', query, limit); functionResults = await queryFTSViaExecutor(executor, 'Function', 'function_fts', query, limit); classResults = await queryFTSViaExecutor(executor, 'Class', 'class_fts', query, limit); @@ -87,7 +150,12 @@ export const searchFTSFromLbug = async ( limit, ); } else { - // Use core lbug adapter (CLI / pipeline context) — also sequential for safety + // Use core lbug adapter (CLI / pipeline context) — also sequential for safety. + // Lazy-create FTS indexes on first query (analyze no longer does it). + for (const { table, indexName, properties } of FTS_INDEXES) { + await ensureFTSIndex(table, indexName, [...properties]).catch(() => {}); + } + fileResults = await queryFTS('File', 'file_fts', query, limit, false).catch(() => []); functionResults = await queryFTS('Function', 'function_fts', query, limit, false).catch( () => [], diff --git a/gitnexus/src/mcp/local/local-backend.ts b/gitnexus/src/mcp/local/local-backend.ts index 55157cf05..129cda678 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'; import { collectBestChunks } from '../../core/embeddings/types.js'; import { EMBEDDING_TABLE_NAME, EMBEDDING_INDEX_NAME } from '../../core/lbug/schema.js'; import { PhaseTimer } from '../../core/search/phase-timer.js'; @@ -218,6 +219,7 @@ export class LocalBackend { impact: (r, p) => this.impact(r as RepoHandle, p), query: (r, p) => this.query(r as RepoHandle, p), impactByUid: (id, uid, d, o) => this.impactByUid(id, uid, d, o), + context: (r, p) => this.context(r as RepoHandle, p), }; this.groupToolSvc = new GroupService(port); } @@ -499,8 +501,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': @@ -2835,17 +2846,103 @@ 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') { + const impactArgs: Record = { + 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 queryArgs: Record = { + name: groupName, + 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 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 = { + name: groupName, + 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}`); + } + private async groupList(params: Record): Promise { return this.getGroupService().groupList(params); } @@ -2854,18 +2951,45 @@ export class LocalBackend { return this.getGroupService().groupSync(params); } - private async groupContracts(params: Record): Promise { - return this.getGroupService().groupContracts(params); + /** + * MCP resource body for `gitnexus://group/{name}/contracts` (Issue #794). + */ + async readGroupContractsResource( + groupName: string, + filter: { type?: string; repo?: string; unmatchedOnly?: boolean }, + ): Promise { + try { + const params: Record = { name: groupName }; + if (filter.type !== undefined) params.type = filter.type; + if (filter.repo !== undefined) params.repo = filter.repo; + if (filter.unmatchedOnly === true) params.unmatchedOnly = true; + const raw = await this.getGroupService().groupContracts(params); + return LocalBackend.formatGroupResourcePayload(raw); + } catch (e) { + return `error: ${e instanceof Error ? e.message : String(e)}`; + } } - private async groupQuery(params: Record): Promise { - await this.refreshRepos(); - return this.getGroupService().groupQuery(params); + /** + * MCP resource body for `gitnexus://group/{name}/status` (Issue #794). + */ + async readGroupStatusResource(groupName: string): Promise { + try { + const raw = await this.getGroupService().groupStatus({ name: groupName }); + return LocalBackend.formatGroupResourcePayload(raw); + } catch (e) { + return `error: ${e instanceof Error ? e.message : String(e)}`; + } } - private async groupStatus(params: Record): Promise { - await this.refreshRepos(); - return this.getGroupService().groupStatus(params); + private static formatGroupResourcePayload(raw: unknown): string { + if (raw && typeof raw === 'object' && 'error' in raw) { + const err = (raw as { error?: unknown }).error; + if (typeof err === 'string' && err.length > 0) { + return `error: ${err}`; + } + } + return JSON.stringify(raw, null, 2); } /** diff --git a/gitnexus/src/mcp/resources.ts b/gitnexus/src/mcp/resources.ts index dce81bab4..88e7a99cc 100644 --- a/gitnexus/src/mcp/resources.ts +++ b/gitnexus/src/mcp/resources.ts @@ -84,38 +84,140 @@ export function getResourceTemplates(): ResourceTemplate[] { description: 'Step-by-step execution trace', mimeType: 'text/yaml', }, + { + uriTemplate: 'gitnexus://group/{name}/contracts', + name: 'Group Contract Registry', + description: + 'Cross-repo contract registry for a repository group. Optional query: type, repo, unmatchedOnly (true|false).', + mimeType: 'text/yaml', + }, + { + uriTemplate: 'gitnexus://group/{name}/status', + name: 'Group Index Status', + description: 'Per-repo index and contract-registry staleness for a repository group', + mimeType: 'text/yaml', + }, ]; } -/** - * Parse a resource URI to extract the repo name and resource type. - */ -function parseUri(uri: string): { repoName?: string; resourceType: string; param?: string } { - if (uri === 'gitnexus://repos') return { resourceType: 'repos' }; - if (uri === 'gitnexus://setup') return { resourceType: 'setup' }; +/** Query parameters for `gitnexus://group/{name}/contracts` */ +export type GroupContractsResourceFilter = { + type?: string; + repo?: string; + unmatchedOnly?: boolean; +}; - // Repo-scoped: gitnexus://repo/{name}/context - const repoMatch = uri.match(/^gitnexus:\/\/repo\/([^/]+)\/(.+)$/); - if (repoMatch) { - const repoName = decodeURIComponent(repoMatch[1]); - const rest = repoMatch[2]; +/** Normalized parse result for GitNexus MCP resource URIs */ +export type ParsedGitnexusResource = + | { kind: 'repos' } + | { kind: 'setup' } + | { + kind: 'repo'; + repoName: string; + resourceType: string; + param?: string; + } + | { + kind: 'group'; + groupName: string; + resourceType: 'contracts'; + contractsFilter: GroupContractsResourceFilter; + } + | { kind: 'group'; groupName: string; resourceType: 'status' }; + +function parseUnmatchedOnlyParam(raw: string | null): boolean | undefined { + if (raw === null) return undefined; + const v = raw.trim().toLowerCase(); + if (v === 'true' || v === '1') return true; + if (v === 'false' || v === '0') return false; + return undefined; +} + +/** + * Parse a GitNexus resource URI (repos, setup, per-repo, or per-group templates). + * Used by `readResource` and tests (round-trip / dispatch coverage). + */ +export function parseResourceUri(uri: string): ParsedGitnexusResource { + if (uri === 'gitnexus://repos') return { kind: 'repos' }; + if (uri === 'gitnexus://setup') return { kind: 'setup' }; + + let u: URL; + try { + u = new URL(uri); + } catch { + throw new Error(`Unknown resource URI: ${uri}`); + } + + if (u.protocol !== 'gitnexus:') { + throw new Error(`Unknown resource URI: ${uri}`); + } + + if (u.hostname === 'group') { + const segments = u.pathname + .replace(/^\/+|\/+$/g, '') + .split('/') + .filter(Boolean); + if (segments.length < 2) { + throw new Error( + `Invalid group resource URI (expected gitnexus://group/{name}/contracts or .../status): ${uri}`, + ); + } + const tail = segments[segments.length - 1]!; + if (tail !== 'contracts' && tail !== 'status') { + throw new Error(`Unknown group resource path in URI: ${uri}`); + } + const groupName = segments + .slice(0, -1) + .map((s) => decodeURIComponent(s)) + .join('/'); + if (!groupName) { + throw new Error(`Invalid group resource URI (empty group name): ${uri}`); + } + if (tail === 'status') { + return { kind: 'group', groupName, resourceType: 'status' }; + } + const contractsFilter: GroupContractsResourceFilter = {}; + const type = u.searchParams.get('type'); + if (type && type.trim()) contractsFilter.type = type.trim(); + const repo = u.searchParams.get('repo'); + if (repo && repo.trim()) contractsFilter.repo = repo.trim(); + if (u.searchParams.has('unmatchedOnly')) { + const coerced = parseUnmatchedOnlyParam(u.searchParams.get('unmatchedOnly')); + if (coerced !== undefined) contractsFilter.unmatchedOnly = coerced; + } + return { kind: 'group', groupName, resourceType: 'contracts', contractsFilter }; + } + + if (u.hostname === 'repo') { + const segments = u.pathname + .replace(/^\/+|\/+$/g, '') + .split('/') + .filter(Boolean); + if (segments.length < 2) { + throw new Error(`Unknown resource URI: ${uri}`); + } + const repoName = decodeURIComponent(segments[0]!); + const restEncoded = segments.slice(1); + const rest = restEncoded.map((s) => decodeURIComponent(s)).join('/'); if (rest.startsWith('cluster/')) { return { + kind: 'repo', repoName, resourceType: 'cluster', - param: decodeURIComponent(rest.replace('cluster/', '')), + param: rest.replace(/^cluster\//, ''), }; } if (rest.startsWith('process/')) { return { + kind: 'repo', repoName, resourceType: 'process', - param: decodeURIComponent(rest.replace('process/', '')), + param: rest.replace(/^process\//, ''), }; } - return { repoName, resourceType: rest }; + return { kind: 'repo', repoName, resourceType: rest }; } throw new Error(`Unknown resource URI: ${uri}`); @@ -125,18 +227,23 @@ function parseUri(uri: string): { repoName?: string; resourceType: string; param * Read a resource and return its content */ export async function readResource(uri: string, backend: LocalBackend): Promise { - const parsed = parseUri(uri); + const parsed = parseResourceUri(uri); - // Global repos list — no repo context needed - if (parsed.resourceType === 'repos') { + if (parsed.kind === 'repos') { return getReposResource(backend); } - // Setup resource — returns AGENTS.md content for all repos - if (parsed.resourceType === 'setup') { + if (parsed.kind === 'setup') { return getSetupResource(backend); } + if (parsed.kind === 'group') { + if (parsed.resourceType === 'contracts') { + return backend.readGroupContractsResource(parsed.groupName, parsed.contractsFilter); + } + return backend.readGroupStatusResource(parsed.groupName); + } + const repoName = parsed.repoName; switch (parsed.resourceType) { @@ -241,6 +348,10 @@ async function getContextResource(backend: LocalBackend, repoName?: string): Pro lines.push(` - gitnexus://repo/${context.projectName}/processes: All execution flows`); lines.push(` - gitnexus://repo/${context.projectName}/cluster/{name}: Module details`); lines.push(` - gitnexus://repo/${context.projectName}/process/{name}: Process trace`); + lines.push( + ' - gitnexus://group/{name}/contracts: Group contract registry (optional ?type=&repo=&unmatchedOnly=)', + ); + lines.push(' - gitnexus://group/{name}/status: Group index / contract staleness'); return lines.join('\n'); } diff --git a/gitnexus/src/mcp/tools.ts b/gitnexus/src/mcp/tools.ts index 7beb2e970..491c24557 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: 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: { @@ -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). In group mode (@repo), prefix-matches symbol file paths; ignored for a normal repo name. 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 ranked candidates (each with a relevance score) for you to pick from. Use uid for zero-ambiguity lookup, or narrow the search with file_path and/or kind hints. -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). 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: { @@ -178,7 +204,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). Applies in group mode (@repo) only; ignored for a normal repo name. Empty string is rejected server-side.', }, }, required: [], @@ -273,7 +306,11 @@ TIP: Default traversal uses CALLS/IMPORTS/EXTENDS/IMPLEMENTS. For class members, Handles disambiguation: when multiple symbols share the target name, returns ranked candidates (each with a relevance score) instead of silently picking one. Use target_uid for zero-ambiguity lookup, or narrow with file_path and/or kind hints. 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). 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: { @@ -298,8 +335,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', @@ -308,10 +355,42 @@ 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). Applies when "repo" is group mode (@…); ignored for a normal repo name. 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'], @@ -429,49 +508,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/cross-impact.test.ts b/gitnexus/test/unit/group/cross-impact.test.ts new file mode 100644 index 000000000..3d78ff1cf --- /dev/null +++ b/gitnexus/test/unit/group/cross-impact.test.ts @@ -0,0 +1,196 @@ +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import * as fs from 'node:fs'; +import * as path from 'node:path'; +import * as os from 'node:os'; +import { + validateGroupImpactParams, + runGroupImpact, + MAX_SUPPORTED_CROSS_DEPTH, + DEFAULT_LOCAL_IMPACT_TIMEOUT_MS, + collectImpactSymbolUids, + fileMatchesServicePrefix, +} from '../../../src/core/group/cross-impact.js'; +import type { GroupToolPort } from '../../../src/core/group/service.js'; +import { writeBridgeMeta } from '../../../src/core/group/bridge-db.js'; +import { BRIDGE_SCHEMA_VERSION } from '../../../src/core/group/bridge-schema.js'; + +function tmpGroup(): { tmpDir: string; groupDir: string; cleanup: () => void } { + const tmpDir = path.join(os.tmpdir(), `gitnexus-ci-${Date.now()}-${Math.random()}`); + const groupDir = path.join(tmpDir, 'groups', 'g1'); + fs.mkdirSync(groupDir, { recursive: true }); + fs.writeFileSync( + path.join(groupDir, 'group.yaml'), + `version: 1 +name: g1 +description: "" +repos: + app/backend: reg-be + app/frontend: reg-fe +links: [] +packages: {} +detect: + http: true + grpc: true + topics: true + shared_libs: true + embedding_fallback: true +matching: + bm25_threshold: 0.7 + embedding_threshold: 0.65 + max_candidates_per_step: 3 +`, + ); + return { + tmpDir, + groupDir, + cleanup: () => fs.rmSync(tmpDir, { recursive: true, force: true }), + }; +} + +describe('cross-impact', () => { + beforeEach(() => { + vi.restoreAllMocks(); + }); + + it('test_validateGroupImpactParams_rejects_bad_direction', () => { + const r = validateGroupImpactParams({ + name: 'g', + repo: 'a', + target: 't', + direction: 'sideways', + }); + expect(r.ok).toBe(false); + if (!r.ok) expect(r.error).toContain('direction'); + }); + + it('test_validateGroupImpactParams_clamps_crossDepth_and_warns', () => { + const r = validateGroupImpactParams({ + name: 'g', + repo: 'a', + target: 't', + direction: 'upstream', + crossDepth: 99, + }); + expect(r.ok).toBe(true); + if (r.ok) { + expect(r.crossDepth).toBe(MAX_SUPPORTED_CROSS_DEPTH); + expect(r.crossDepthWarning).toBeDefined(); + } + }); + + it('test_validateGroupImpactParams_default_timeout', () => { + const r = validateGroupImpactParams({ + name: 'g', + repo: 'a', + target: 't', + direction: 'downstream', + }); + expect(r.ok).toBe(true); + if (r.ok) expect(r.timeoutMs).toBe(DEFAULT_LOCAL_IMPACT_TIMEOUT_MS); + }); + + it('test_collectImpactSymbolUids_respects_service_prefix', () => { + const local = { + target: { id: 'a', filePath: 'services/auth/x.ts' }, + byDepth: { + 1: [{ id: 'b', filePath: 'other/y.ts' }], + }, + }; + const uids = collectImpactSymbolUids(local, 'services/auth').uids; + expect(uids).toContain('a'); + expect(uids).not.toContain('b'); + }); + + it('test_fileMatchesServicePrefix', () => { + expect(fileMatchesServicePrefix('services/auth/a.ts', 'services/auth')).toBe(true); + expect(fileMatchesServicePrefix('services/aut', 'services/auth')).toBe(false); + }); + + it('test_runGroupImpact_local_timeout_returns_truncation', async () => { + const { tmpDir, cleanup } = tmpGroup(); + vi.stubEnv('GITNEXUS_HOME', tmpDir); + try { + let impactCalls = 0; + const port: GroupToolPort = { + resolveRepo: vi.fn(async () => ({ + id: 'be', + name: 'reg-be', + repoPath: '/r', + storagePath: '/r/.gitnexus', + })), + impact: vi.fn(async () => { + impactCalls++; + await new Promise((r) => setTimeout(r, 200)); + return { summary: { direct: 1 }, byDepth: { 1: [{ id: 'x' }] } }; + }), + query: vi.fn(), + impactByUid: vi.fn(), + context: vi.fn(), + }; + const r = await runGroupImpact( + { port, gitnexusDir: tmpDir }, + { + name: 'g1', + repo: 'app/backend', + target: 'Sym', + direction: 'upstream', + timeoutMs: 15, + }, + ); + expect(impactCalls).toBe(1); + expect('error' in r).toBe(false); + if (!('error' in r)) { + expect(r.truncationReason).toBe('timeout'); + expect(r.truncated).toBe(true); + } + } finally { + vi.unstubAllEnvs(); + cleanup(); + } + }); + + it('test_runGroupImpact_bridge_schema_mismatch_returns_error', async () => { + const { tmpDir, groupDir, cleanup } = tmpGroup(); + vi.stubEnv('GITNEXUS_HOME', tmpDir); + await writeBridgeMeta(groupDir, { + version: BRIDGE_SCHEMA_VERSION + 9, + generatedAt: new Date().toISOString(), + missingRepos: [], + }); + try { + const port: GroupToolPort = { + resolveRepo: vi.fn(async () => ({ + id: 'be', + name: 'reg-be', + repoPath: '/r', + storagePath: '/r/.gitnexus', + })), + impact: vi.fn(async () => ({ + target: { id: 'u1', filePath: 'src/a.ts' }, + summary: { direct: 1, processes_affected: 0, modules_affected: 0 }, + byDepth: { 1: [{ id: 'u1', filePath: 'src/a.ts' }] }, + risk: 'LOW', + })), + query: vi.fn(), + impactByUid: vi.fn(), + context: vi.fn(), + }; + const r = await runGroupImpact( + { port, gitnexusDir: tmpDir }, + { + name: 'g1', + repo: 'app/backend', + target: 'Sym', + direction: 'upstream', + }, + ); + expect('error' in r).toBe(true); + if ('error' in r) { + expect(r.error).toContain('schema'); + } + } finally { + vi.unstubAllEnvs(); + cleanup(); + } + }); +}); diff --git a/gitnexus/test/unit/group/group-path-utils.test.ts b/gitnexus/test/unit/group/group-path-utils.test.ts new file mode 100644 index 000000000..0f57e0adc --- /dev/null +++ b/gitnexus/test/unit/group/group-path-utils.test.ts @@ -0,0 +1,87 @@ +import { describe, it, expect } from 'vitest'; +import { + fileMatchesServicePrefix, + normalizeServicePrefix, + repoInSubgroup, +} from '../../../src/core/group/group-path-utils.js'; + +describe('group-path-utils', () => { + describe('normalizeServicePrefix', () => { + it('returns undefined for null/undefined/empty', () => { + expect(normalizeServicePrefix(undefined)).toBeUndefined(); + expect(normalizeServicePrefix(null)).toBeUndefined(); + expect(normalizeServicePrefix('')).toBeUndefined(); + expect(normalizeServicePrefix(' ')).toBeUndefined(); + }); + + it('strips trailing slashes', () => { + expect(normalizeServicePrefix('services/auth/')).toBe('services/auth'); + expect(normalizeServicePrefix('services/auth///')).toBe('services/auth'); + }); + + it('normalizes Windows-style backslashes to POSIX', () => { + expect(normalizeServicePrefix('services\\auth')).toBe('services/auth'); + expect(normalizeServicePrefix('app\\backend\\')).toBe('app/backend'); + }); + }); + + describe('fileMatchesServicePrefix', () => { + it('returns true when prefix is empty/undefined', () => { + expect(fileMatchesServicePrefix('any/file.ts', undefined)).toBe(true); + expect(fileMatchesServicePrefix('any/file.ts', '')).toBe(true); + }); + + it('returns false when filePath is missing but prefix is set', () => { + expect(fileMatchesServicePrefix(undefined, 'services/auth')).toBe(false); + }); + + it('matches exact prefix and descendants', () => { + expect(fileMatchesServicePrefix('services/auth', 'services/auth')).toBe(true); + expect(fileMatchesServicePrefix('services/auth/a.ts', 'services/auth')).toBe(true); + }); + + it('rejects partial-segment matches', () => { + expect(fileMatchesServicePrefix('services/aut', 'services/auth')).toBe(false); + expect(fileMatchesServicePrefix('services/authz/a.ts', 'services/auth')).toBe(false); + }); + + it('matches Windows-style file paths against POSIX prefix', () => { + expect(fileMatchesServicePrefix('services\\auth\\a.ts', 'services/auth')).toBe(true); + expect(fileMatchesServicePrefix('services\\authz\\a.ts', 'services/auth')).toBe(false); + }); + }); + + describe('repoInSubgroup', () => { + it('matches every repo when subgroup is empty/undefined', () => { + expect(repoInSubgroup('any/repo', undefined)).toBe(true); + expect(repoInSubgroup('any/repo', '')).toBe(true); + expect(repoInSubgroup('any/repo', ' ')).toBe(true); + }); + + it('matches exact path and descendants by default', () => { + expect(repoInSubgroup('app/backend', 'app/backend')).toBe(true); + expect(repoInSubgroup('app/backend/sub', 'app/backend')).toBe(true); + expect(repoInSubgroup('app/frontend', 'app/backend')).toBe(false); + }); + + it('strips trailing slashes from subgroup', () => { + expect(repoInSubgroup('app/backend', 'app/backend/')).toBe(true); + expect(repoInSubgroup('app/backend/x', 'app/backend///')).toBe(true); + }); + + it('with exact=true matches only the exact repo', () => { + expect(repoInSubgroup('app/backend', 'app/backend', true)).toBe(true); + expect(repoInSubgroup('app/backend/sub', 'app/backend', true)).toBe(false); + }); + + it('rejects partial-segment matches', () => { + expect(repoInSubgroup('app/backendz', 'app/backend')).toBe(false); + }); + + it('normalizes Windows-style paths on both sides', () => { + expect(repoInSubgroup('app\\backend', 'app/backend')).toBe(true); + expect(repoInSubgroup('app/backend/x', 'app\\backend')).toBe(true); + expect(repoInSubgroup('app\\backend\\sub', 'app\\backend', true)).toBe(false); + }); + }); +}); 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/group/service.test.ts b/gitnexus/test/unit/group/service.test.ts index e4b10443c..8c5c0ed6d 100644 --- a/gitnexus/test/unit/group/service.test.ts +++ b/gitnexus/test/unit/group/service.test.ts @@ -44,6 +44,10 @@ function makePort(overrides: Partial = {}): GroupToolPort { impact: vi.fn(async () => ({ symbols: [] })), query: vi.fn(async () => ({ processes: [] })), impactByUid: vi.fn(async () => null), + context: vi.fn(async () => ({ + status: 'found', + symbol: { filePath: 'services/auth/x.ts', uid: 'u1', name: 'X' }, + })), ...overrides, }; } @@ -233,6 +237,46 @@ describe('GroupService', () => { cleanup(); } }); + + it('test_groupContracts_skips_corrupt_contract_rows', async () => { + const { groupDir, cleanup, tmpDir } = makeTmpGroup(); + try { + vi.stubEnv('GITNEXUS_HOME', tmpDir); + const badJson = `{ + "version": 1, + "generatedAt": "2026-01-01T00:00:00.000Z", + "repoSnapshots": {}, + "missingRepos": [], + "contracts": [ + { "not": "a-contract" }, + { + "contractId": "http::GET::/ok", + "type": "http", + "repo": "app/backend", + "role": "provider", + "symbolUid": "u", + "symbolRef": { "filePath": "a.ts", "name": "f" }, + "symbolName": "f", + "confidence": 1, + "meta": {} + } + ], + "crossLinks": [] + }`; + fs.writeFileSync(path.join(groupDir, 'contracts.json'), badJson, 'utf-8'); + + const svc = new GroupService(makePort()); + const result = (await svc.groupContracts({ name: 'test-group' })) as { + contracts: unknown[]; + skippedCorrupt?: number; + }; + expect(result.contracts).toHaveLength(1); + expect(result.skippedCorrupt).toBe(1); + } finally { + vi.unstubAllEnvs(); + cleanup(); + } + }); }); describe('groupSync', () => { @@ -328,6 +372,135 @@ 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', () => { + it('test_groupImpact_returns_validation_error', async () => { + const svc = new GroupService(makePort()); + const r = (await svc.groupImpact({})) as { error: string }; + expect(r.error).toContain('name'); + }); + }); + + describe('groupContext', () => { + it('test_groupContext_requires_target_or_uid', async () => { + const svc = new GroupService(makePort()); + const r = await svc.groupContext({ name: 'test-group' }); + expect(r.error).toContain('target'); + }); + + it('test_groupContext_iterates_repos', async () => { + const { cleanup, tmpDir } = makeTmpGroup(); + try { + vi.stubEnv('GITNEXUS_HOME', tmpDir); + const port = makePort(); + const svc = new GroupService(port); + const r = await svc.groupContext({ name: 'test-group', target: 'MySym' }); + expect(r.group).toBe('test-group'); + expect(r.results).toHaveLength(2); + expect(port.context).toHaveBeenCalledTimes(2); + } finally { + vi.unstubAllEnvs(); + cleanup(); + } + }); + + 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 { + vi.stubEnv('GITNEXUS_HOME', tmpDir); + const port = makePort({ + context: vi.fn(async () => ({ + status: 'found', + symbol: { filePath: 'other/path/x.ts', uid: 'u1', name: 'X' }, + })), + }); + const svc = new GroupService(port); + const r = await svc.groupContext({ + name: 'test-group', + target: 'MySym', + service: 'services/auth', + }); + expect(r.results.every((x) => Object.keys(x.payload as object).length === 0)).toBe(true); + } finally { + vi.unstubAllEnvs(); + cleanup(); + } + }); }); describe('groupStatus', () => { 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..ddf7d03a5 --- /dev/null +++ b/gitnexus/test/unit/mcp/group-repo-routing.test.ts @@ -0,0 +1,230 @@ +/** + * 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 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', + subgroupExact: true, + }), + ); + }); + + 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('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: '' }); + 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/, + ); + }); + + 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/); + }); + + describe('Issue #794 manual smoke checklist (automated)', () => { + beforeEach(() => { + const groupDir = path.join(tmpDir, 'groups', 'myproduct'); + fs.mkdirSync(groupDir, { recursive: true }); + fs.writeFileSync( + path.join(groupDir, 'group.yaml'), + `version: 1 +name: myproduct +repos: + app/backend: test-backend + app/frontend: test-frontend +`, + ); + }); + + it.each([ + { + method: 'impact', + params: { repo: '@myproduct', target: 'UserService.login', service: 'app/backend' }, + spy: () => groupSpyImpact, + }, + { + method: 'query', + params: { repo: '@myproduct', query: 'login', service: 'app/backend' }, + spy: () => groupSpyQuery, + }, + { + method: 'context', + params: { repo: '@myproduct', target: 'UserService.login', service: 'app/backend' }, + spy: () => groupSpyContext, + }, + ])( + '$method with repo "@myproduct" routes to GroupService and forwards service', + async ({ method, params, spy }) => { + const backend = new LocalBackend(); + await backend.callTool(method, params); + expect(spy()).toHaveBeenCalledWith( + expect.objectContaining({ name: 'myproduct', service: 'app/backend' }), + ); + const callArg = spy().mock.calls[0][0] as Record; + expect( + typeof callArg.repo === 'string' ? (callArg.repo as string).startsWith('@') : false, + ).toBe(false); + }, + ); + }); +}); diff --git a/gitnexus/test/unit/resources.test.ts b/gitnexus/test/unit/resources.test.ts index 6b203c969..ccd8461f1 100644 --- a/gitnexus/test/unit/resources.test.ts +++ b/gitnexus/test/unit/resources.test.ts @@ -12,6 +12,7 @@ import { describe, it, expect, vi } from 'vitest'; import { getResourceDefinitions, getResourceTemplates, + parseResourceUri, readResource, } from '../../src/mcp/resources.js'; @@ -36,6 +37,12 @@ function createMockBackend(overrides: Partial> = {}): any { 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, }; } @@ -73,12 +80,12 @@ describe('getResourceDefinitions', () => { }); describe('getResourceTemplates', () => { - it('returns 6 dynamic templates', () => { + it('returns 8 dynamic templates', () => { const templates = getResourceTemplates(); - expect(templates).toHaveLength(6); + expect(templates).toHaveLength(8); }); - it('includes context, clusters, processes, schema, cluster detail, process detail', () => { + 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'); @@ -87,6 +94,8 @@ describe('getResourceTemplates', () => { 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', () => { @@ -99,6 +108,61 @@ describe('getResourceTemplates', () => { }); }); +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', () => { @@ -149,6 +213,22 @@ describe('readResource', () => { 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: { 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', () => { From d8587464762ea971b209a7df3a2c1c26d9e6e63d Mon Sep 17 00:00:00 2001 From: xiaohaoxing Date: Mon, 20 Apr 2026 19:07:19 +0800 Subject: [PATCH 10/13] fix(embeddings): replace recursive AST traversal with iterative DFS (#990) findFunctionNode and findDeclarationNode had no depth limit, causing stack overflow on deeply nested or auto-generated ASTs, especially when --stack-size is not applied (e.g. heap already large enough to skip ensureHeap re-exec). Co-authored-by: Claude Sonnet 4.6 --- gitnexus/src/core/embeddings/ast-utils.ts | 36 +++++++++++------------ 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/gitnexus/src/core/embeddings/ast-utils.ts b/gitnexus/src/core/embeddings/ast-utils.ts index fb51ac052..8456b249e 100644 --- a/gitnexus/src/core/embeddings/ast-utils.ts +++ b/gitnexus/src/core/embeddings/ast-utils.ts @@ -64,16 +64,16 @@ const FUNCTION_LIKE_TYPES = new Set([ * numbers don't apply. */ export const findFunctionNode = (root: any): any | null => { - if (FUNCTION_LIKE_TYPES.has(root.type)) return root; - - for (let i = 0; i < root.namedChildCount; i++) { - const child = root.namedChild(i); - if (!child) continue; - if (FUNCTION_LIKE_TYPES.has(child.type)) return child; - const found = findFunctionNode(child); - if (found) return found; + // Iterative DFS — avoids stack overflow on deeply nested ASTs. + const stack = [root]; + while (stack.length > 0) { + const node = stack.pop()!; + if (FUNCTION_LIKE_TYPES.has(node.type)) return node; + for (let i = node.namedChildCount - 1; i >= 0; i--) { + const child = node.namedChild(i); + if (child) stack.push(child); + } } - return null; }; @@ -98,15 +98,15 @@ export const findDeclarationNode = (root: any): any | null => { 'impl_item', // Rust: impl ]); - if (CLASS_LIKE_TYPES.has(root.type)) return root; - - for (let i = 0; i < root.namedChildCount; i++) { - const child = root.namedChild(i); - if (!child) continue; - if (CLASS_LIKE_TYPES.has(child.type)) return child; - const found = findDeclarationNode(child); - if (found) return found; + // Iterative DFS — avoids stack overflow on deeply nested ASTs. + const stack = [root]; + while (stack.length > 0) { + const node = stack.pop()!; + if (CLASS_LIKE_TYPES.has(node.type)) return node; + for (let i = node.namedChildCount - 1; i >= 0; i--) { + const child = node.namedChild(i); + if (child) stack.push(child); + } } - return null; }; From 8f41a1ba171a027ba231ea4b8b0bf615110dcc4c Mon Sep 17 00:00:00 2001 From: jisue0224 <166787294+jisue0224@users.noreply.github.com> Date: Tue, 21 Apr 2026 01:06:58 +0900 Subject: [PATCH 11/13] fix(bm25): return FTS-matched symbols instead of arbitrary LIMIT 3 nodes (#806) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(bm25): return FTS-matched symbols instead of arbitrary LIMIT 3 nodes Previously, bm25Search fetched up to 3 arbitrary symbols from the matched file using MATCH (n) WHERE n.filePath = $filePath LIMIT 3 (no ORDER BY). This meant the specific function or class that actually scored highest in the BM25 index could be completely absent from the results. Fix: propagate nodeId from each FTS hit through searchFTSFromLbug, then use those nodeIds in bm25Search to look up the exact matched nodes via WHERE n.id IN $nodeIds. Falls back to the old filePath-based lookup when nodeIds are unavailable. Also switches the per-file score aggregation from naive sum-of-all to sum-of-top-3, which prevents files with many mediocre matches (e.g. test files) from outranking files with a single highly-relevant symbol. * test(bm25): add unit tests for top-3 aggregation and nodeIds propagation Covers the new logic paths added in the previous commit: - top-3 score aggregation (file with 5+ matches → only top-3 contribute) - nodeIds propagation through BM25SearchResult - empty nodeId filtering - cross-table merge for the same file - result ranking by aggregated score Also fixes in-place entries.sort() mutation (bm25-index.ts:125) to use [...entries].sort() so the Map value is not silently modified. * style: apply prettier formatting * fix(test): use importOriginal to avoid missing export errors in vi.mock * fix(bm25): align queryFTSViaExecutor nodeId extraction to match lbug-adapter Use node.nodeId || node.id || '' in queryFTSViaExecutor to match the fallback logic in lbug-adapter.ts:1040. Without this, the MCP pool path could silently return empty nodeIds if LadybugDB surfaces the node id under node.nodeId rather than node.id. --------- Co-authored-by: jisue0224 <> --- gitnexus/src/core/search/bm25-index.ts | 30 ++++-- gitnexus/src/mcp/local/local-backend.ts | 34 ++++-- gitnexus/test/unit/bm25-search.test.ts | 138 +++++++++++++++++++++++- 3 files changed, 182 insertions(+), 20 deletions(-) diff --git a/gitnexus/src/core/search/bm25-index.ts b/gitnexus/src/core/search/bm25-index.ts index 040440999..92d8ec120 100644 --- a/gitnexus/src/core/search/bm25-index.ts +++ b/gitnexus/src/core/search/bm25-index.ts @@ -17,6 +17,7 @@ export interface BM25SearchResult { filePath: string; score: number; rank: number; + nodeIds?: string[]; } /** @@ -79,7 +80,7 @@ async function queryFTSViaExecutor( indexName: string, query: string, limit: number, -): Promise> { +): Promise> { // Escape single quotes and backslashes to prevent Cypher injection const escapedQuery = query.replace(/\\/g, '\\\\').replace(/'/g, "''"); const cypher = ` @@ -96,6 +97,7 @@ async function queryFTSViaExecutor( return { filePath: node.filePath || '', score: typeof score === 'number' ? score : parseFloat(score) || 0, + nodeId: node.nodeId || node.id || '', }; }); } catch { @@ -167,17 +169,13 @@ export const searchFTSFromLbug = async ( ); } - // Merge results by filePath, summing scores for same file - const merged = new Map(); + // Collect all node scores per filePath to track which nodes actually matched + const fileNodeScores = new Map>(); const addResults = (results: any[]) => { for (const r of results) { - const existing = merged.get(r.filePath); - if (existing) { - existing.score += r.score; - } else { - merged.set(r.filePath, { filePath: r.filePath, score: r.score }); - } + if (!fileNodeScores.has(r.filePath)) fileNodeScores.set(r.filePath, []); + fileNodeScores.get(r.filePath)!.push({ score: r.score, nodeId: r.nodeId }); } }; @@ -187,6 +185,19 @@ export const searchFTSFromLbug = async ( addResults(methodResults); addResults(interfaceResults); + // Sum the top-3 highest-scoring nodes per file and collect their nodeIds. + // Summing all nodes naively inflates scores for files with many mediocre + // matches (e.g. test files) over files with a single highly-relevant symbol. + const merged = new Map(); + for (const [filePath, entries] of fileNodeScores) { + const top3 = [...entries].sort((a, b) => b.score - a.score).slice(0, 3); + merged.set(filePath, { + filePath, + score: top3.reduce((acc, e) => acc + e.score, 0), + nodeIds: top3.map((e) => e.nodeId).filter((id) => id), + }); + } + // Sort by score descending and add rank const sorted = Array.from(merged.values()) .sort((a, b) => b.score - a.score) @@ -196,5 +207,6 @@ export const searchFTSFromLbug = async ( filePath: r.filePath, score: r.score, rank: index + 1, + nodeIds: r.nodeIds, })); }; diff --git a/gitnexus/src/mcp/local/local-backend.ts b/gitnexus/src/mcp/local/local-backend.ts index 129cda678..3414f5f5b 100644 --- a/gitnexus/src/mcp/local/local-backend.ts +++ b/gitnexus/src/mcp/local/local-backend.ts @@ -851,16 +851,30 @@ export class LocalBackend { for (const bm25Result of bm25Results) { const fullPath = bm25Result.filePath; try { - const symbols = await executeParameterized( - repo.id, - ` - MATCH (n) - WHERE n.filePath = $filePath - RETURN n.id AS id, n.name AS name, labels(n)[0] AS type, n.filePath AS filePath, n.startLine AS startLine, n.endLine AS endLine - LIMIT 3 - `, - { filePath: fullPath }, - ); + // Prefer direct nodeId lookup (exact FTS-matched nodes) over filePath fallback. + // Without this, LIMIT 3 on filePath returns arbitrary symbols rather than + // the nodes that actually scored highest in the BM25 index. + const nodeIds = bm25Result.nodeIds?.length ? bm25Result.nodeIds : null; + const symbols = nodeIds + ? await executeParameterized( + repo.id, + ` + MATCH (n) + WHERE n.id IN $nodeIds + RETURN n.id AS id, n.name AS name, labels(n)[0] AS type, n.filePath AS filePath, n.startLine AS startLine, n.endLine AS endLine + `, + { nodeIds }, + ) + : await executeParameterized( + repo.id, + ` + MATCH (n) + WHERE n.filePath = $filePath + RETURN n.id AS id, n.name AS name, labels(n)[0] AS type, n.filePath AS filePath, n.startLine AS startLine, n.endLine AS endLine + LIMIT 3 + `, + { filePath: fullPath }, + ); if (symbols.length > 0) { for (const sym of symbols) { diff --git a/gitnexus/test/unit/bm25-search.test.ts b/gitnexus/test/unit/bm25-search.test.ts index 1d8be757f..466df3395 100644 --- a/gitnexus/test/unit/bm25-search.test.ts +++ b/gitnexus/test/unit/bm25-search.test.ts @@ -1,6 +1,14 @@ -import { describe, it, expect } from 'vitest'; +import { describe, it, expect, vi, beforeEach } from 'vitest'; import { searchFTSFromLbug, type BM25SearchResult } from '../../src/core/search/bm25-index.js'; +vi.mock('../../src/core/lbug/lbug-adapter.js', async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + queryFTS: vi.fn().mockResolvedValue([]), + }; +}); + describe('BM25 search', () => { describe('searchFTSFromLbug', () => { it('returns empty array when LadybugDB is not initialized', async () => { @@ -32,5 +40,133 @@ describe('BM25 search', () => { expect(result.score).toBe(1.5); expect(result.rank).toBe(1); }); + + it('accepts optional nodeIds field', () => { + const result: BM25SearchResult = { + filePath: 'src/index.ts', + score: 1.5, + rank: 1, + nodeIds: ['func:id1', 'func:id2'], + }; + expect(result.nodeIds).toEqual(['func:id1', 'func:id2']); + }); + }); + + describe('score aggregation', () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + it('sums only top-3 scoring nodes per file when more than 3 match', async () => { + const { queryFTS } = await import('../../src/core/lbug/lbug-adapter.js'); + // File table: empty; Function table: 5 hits for the same file; rest: empty + vi.mocked(queryFTS) + .mockResolvedValueOnce([]) // File + .mockResolvedValueOnce([ + // Function — 5 hits, scores 10/9/8/7/6 + { filePath: 'src/views.py', score: 10, nodeId: 'func:node1', name: 'get_queryset' }, + { filePath: 'src/views.py', score: 9, nodeId: 'func:node2', name: 'post' }, + { filePath: 'src/views.py', score: 8, nodeId: 'func:node3', name: 'delete' }, + { filePath: 'src/views.py', score: 7, nodeId: 'func:node4', name: 'patch' }, + { filePath: 'src/views.py', score: 6, nodeId: 'func:node5', name: 'put' }, + ]) + .mockResolvedValueOnce([]) // Class + .mockResolvedValueOnce([]) // Method + .mockResolvedValueOnce([]); // Interface + + const results = await searchFTSFromLbug('queryset'); + + expect(results).toHaveLength(1); + expect(results[0].filePath).toBe('src/views.py'); + // Only top-3 scores (10+9+8=27), not naive sum of all 5 (10+9+8+7+6=40) + expect(results[0].score).toBe(27); + expect(results[0].nodeIds).toEqual(['func:node1', 'func:node2', 'func:node3']); + }); + + it('propagates nodeIds for files with fewer than 3 matching nodes', async () => { + const { queryFTS } = await import('../../src/core/lbug/lbug-adapter.js'); + vi.mocked(queryFTS) + .mockResolvedValueOnce([]) // File + .mockResolvedValueOnce([ + // Function — 2 hits + { filePath: 'src/models.py', score: 5, nodeId: 'func:m1', name: 'save' }, + { filePath: 'src/models.py', score: 3, nodeId: 'func:m2', name: 'delete' }, + ]) + .mockResolvedValueOnce([]) // Class + .mockResolvedValueOnce([]) // Method + .mockResolvedValueOnce([]); // Interface + + const results = await searchFTSFromLbug('model'); + + expect(results).toHaveLength(1); + expect(results[0].score).toBe(8); // 5+3 + expect(results[0].nodeIds).toEqual(['func:m1', 'func:m2']); + }); + + it('filters out empty nodeIds', async () => { + const { queryFTS } = await import('../../src/core/lbug/lbug-adapter.js'); + vi.mocked(queryFTS) + .mockResolvedValueOnce([]) // File + .mockResolvedValueOnce([ + // Function — nodes with no id + { filePath: 'src/utils.py', score: 5, nodeId: '', name: 'helper' }, + { filePath: 'src/utils.py', score: 3, nodeId: '', name: 'util' }, + ]) + .mockResolvedValueOnce([]) // Class + .mockResolvedValueOnce([]) // Method + .mockResolvedValueOnce([]); // Interface + + const results = await searchFTSFromLbug('util'); + + expect(results).toHaveLength(1); + expect(results[0].nodeIds).toEqual([]); + }); + + it('merges hits across multiple index tables for the same file', async () => { + const { queryFTS } = await import('../../src/core/lbug/lbug-adapter.js'); + vi.mocked(queryFTS) + .mockResolvedValueOnce([ + // File table + { filePath: 'src/auth.py', score: 4, nodeId: 'file:auth', name: 'auth.py' }, + ]) + .mockResolvedValueOnce([ + // Function table + { filePath: 'src/auth.py', score: 9, nodeId: 'func:login', name: 'login' }, + ]) + .mockResolvedValueOnce([ + // Class table + { filePath: 'src/auth.py', score: 7, nodeId: 'cls:User', name: 'User' }, + ]) + .mockResolvedValueOnce([]) // Method + .mockResolvedValueOnce([]); // Interface + + const results = await searchFTSFromLbug('auth'); + + expect(results).toHaveLength(1); + // All 3 hits (scores 9+7+4=20) — each from a different table, all top-3 + expect(results[0].score).toBe(20); + expect(results[0].nodeIds).toEqual(['func:login', 'cls:User', 'file:auth']); + }); + + it('ranks files by aggregated score descending', async () => { + const { queryFTS } = await import('../../src/core/lbug/lbug-adapter.js'); + vi.mocked(queryFTS) + .mockResolvedValueOnce([]) // File + .mockResolvedValueOnce([ + // Function — hits across two files + { filePath: 'src/low.py', score: 2, nodeId: 'func:a', name: 'a' }, + { filePath: 'src/high.py', score: 9, nodeId: 'func:b', name: 'b' }, + ]) + .mockResolvedValueOnce([]) // Class + .mockResolvedValueOnce([]) // Method + .mockResolvedValueOnce([]); // Interface + + const results = await searchFTSFromLbug('fn'); + + expect(results[0].filePath).toBe('src/high.py'); + expect(results[1].filePath).toBe('src/low.py'); + expect(results[0].rank).toBe(1); + expect(results[1].rank).toBe(2); + }); }); }); From c24bcc3bf182007cf89e6a5d06ce854845101f82 Mon Sep 17 00:00:00 2001 From: Sam Fakhreddine Date: Mon, 20 Apr 2026 10:12:25 -0600 Subject: [PATCH 12/13] fix: expose detect-changes in direct CLI (#892) Squashed commits: - test: fix risk_level mock case and prettier formatting in tool-direct-cli.test - test: add edge-case coverage for detectChangesCommand formatter --- gitnexus/src/cli/index.ts | 9 ++ gitnexus/src/cli/tool.ts | 52 ++++++++++ gitnexus/test/unit/cli-index-help.test.ts | 10 ++ gitnexus/test/unit/tool-direct-cli.test.ts | 110 +++++++++++++++++++++ 4 files changed, 181 insertions(+) create mode 100644 gitnexus/test/unit/tool-direct-cli.test.ts diff --git a/gitnexus/src/cli/index.ts b/gitnexus/src/cli/index.ts index dca5983e0..2b54f04f3 100644 --- a/gitnexus/src/cli/index.ts +++ b/gitnexus/src/cli/index.ts @@ -151,6 +151,15 @@ program .option('-r, --repo ', 'Target repository') .action(createLazyAction(() => import('./tool.js'), 'cypherCommand')); +program + .command('detect-changes') + .alias('detect_changes') + .description('Map git diff hunks to indexed symbols and affected execution flows') + .option('-s, --scope ', 'What to analyze: unstaged, staged, all, or compare', 'unstaged') + .option('-b, --base-ref ', 'Branch/commit for compare scope (e.g. main)') + .option('-r, --repo ', 'Target repository') + .action(createLazyAction(() => import('./tool.js'), 'detectChangesCommand')); + // ─── Eval Server (persistent daemon for SWE-bench) ───────────────── program diff --git a/gitnexus/src/cli/tool.ts b/gitnexus/src/cli/tool.ts index e5219d2d7..443f12f4c 100644 --- a/gitnexus/src/cli/tool.ts +++ b/gitnexus/src/cli/tool.ts @@ -164,3 +164,55 @@ export async function cypherCommand( }); output(result); } + +function formatDetectChangesResult(result: any): string { + if (result?.error) return `Error: ${result.error}`; + + const summary = result?.summary || {}; + if ((summary.changed_count || 0) === 0) { + return 'No changes detected.'; + } + + const lines: string[] = []; + lines.push(`Changes: ${summary.changed_files || 0} files, ${summary.changed_count || 0} symbols`); + lines.push(`Affected processes: ${summary.affected_count || 0}`); + lines.push(`Risk level: ${summary.risk_level || 'unknown'}`); + lines.push(''); + + const changed = result?.changed_symbols || []; + if (changed.length > 0) { + lines.push('Changed symbols:'); + for (const symbol of changed.slice(0, 15)) { + lines.push(` ${symbol.type} ${symbol.name} → ${symbol.filePath}`); + } + if (changed.length > 15) { + lines.push(` ... and ${changed.length - 15} more`); + } + lines.push(''); + } + + const affected = result?.affected_processes || []; + if (affected.length > 0) { + lines.push('Affected execution flows:'); + for (const processInfo of affected.slice(0, 10)) { + const steps = (processInfo.changed_steps || []).map((s: any) => s.symbol).join(', '); + lines.push(` • ${processInfo.name} (${processInfo.step_count} steps) — changed: ${steps}`); + } + } + + return lines.join('\n').trim(); +} + +export async function detectChangesCommand(options?: { + scope?: string; + baseRef?: string; + repo?: string; +}): Promise { + const backend = await getBackend(); + const result = await backend.callTool('detect_changes', { + scope: options?.scope || 'unstaged', + base_ref: options?.baseRef, + repo: options?.repo, + }); + output(formatDetectChangesResult(result)); +} diff --git a/gitnexus/test/unit/cli-index-help.test.ts b/gitnexus/test/unit/cli-index-help.test.ts index 96e3eab81..59109c8d9 100644 --- a/gitnexus/test/unit/cli-index-help.test.ts +++ b/gitnexus/test/unit/cli-index-help.test.ts @@ -43,6 +43,16 @@ describe('CLI help surface', () => { expect(result.stdout).toContain('--repo '); }); + it('detect-changes help exposes compare scope and base-ref flags', () => { + const result = runHelp('detect-changes'); + + expect(result.status).toBe(0); + expect(result.stdout).toContain('gitnexus detect-changes|detect_changes [options]'); + expect(result.stdout).toContain('--scope '); + expect(result.stdout).toContain('--base-ref '); + expect(result.stdout).toContain('--repo '); + }); + it('wiki help shows provider, review, and verbose flags', () => { const result = runHelp('wiki'); diff --git a/gitnexus/test/unit/tool-direct-cli.test.ts b/gitnexus/test/unit/tool-direct-cli.test.ts new file mode 100644 index 000000000..9ede6225b --- /dev/null +++ b/gitnexus/test/unit/tool-direct-cli.test.ts @@ -0,0 +1,110 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +const initMock = vi.fn(); +const callToolMock = vi.fn(); +const writeSyncMock = vi.fn(); + +vi.mock('../../src/mcp/local/local-backend.js', () => ({ + LocalBackend: class { + init = initMock; + callTool = callToolMock; + }, +})); + +vi.mock('node:fs', () => ({ + writeSync: writeSyncMock, +})); + +describe('direct CLI tool commands', () => { + beforeEach(() => { + vi.resetModules(); + initMock.mockReset(); + callToolMock.mockReset(); + writeSyncMock.mockReset(); + initMock.mockResolvedValue(true); + }); + + it('dispatches detect_changes with CLI-shaped arguments', async () => { + callToolMock.mockResolvedValue({ + summary: { + changed_files: 1, + changed_count: 2, + affected_count: 1, + risk_level: 'low', + }, + }); + const { detectChangesCommand } = await import('../../src/cli/tool.js'); + + await detectChangesCommand({ + scope: 'compare', + baseRef: 'main', + repo: 'gitnexus', + }); + + expect(callToolMock).toHaveBeenCalledWith('detect_changes', { + scope: 'compare', + base_ref: 'main', + repo: 'gitnexus', + }); + expect(writeSyncMock).toHaveBeenCalledWith(1, expect.stringContaining('Risk level: low')); + }); + + it('prints "No changes detected." when changed_count is 0', async () => { + callToolMock.mockResolvedValue({ + summary: { changed_files: 0, changed_count: 0, affected_count: 0, risk_level: 'low' }, + }); + const { detectChangesCommand } = await import('../../src/cli/tool.js'); + + await detectChangesCommand({}); + + expect(writeSyncMock).toHaveBeenCalledWith(1, expect.stringContaining('No changes detected.')); + }); + + it('prints error message when result contains an error', async () => { + callToolMock.mockResolvedValue({ error: 'index is stale' }); + const { detectChangesCommand } = await import('../../src/cli/tool.js'); + + await detectChangesCommand({}); + + expect(writeSyncMock).toHaveBeenCalledWith(1, expect.stringContaining('Error: index is stale')); + }); + + it('truncates changed_symbols list beyond 15 and shows overflow count', async () => { + const symbols = Array.from({ length: 17 }, (_, i) => ({ + type: 'function', + name: `fn${i}`, + filePath: `src/file${i}.ts`, + })); + callToolMock.mockResolvedValue({ + summary: { changed_files: 17, changed_count: 17, affected_count: 0, risk_level: 'low' }, + changed_symbols: symbols, + }); + const { detectChangesCommand } = await import('../../src/cli/tool.js'); + + await detectChangesCommand({}); + + const output: string = writeSyncMock.mock.calls[0][1]; + expect(output).toContain('function fn14 → src/file14.ts'); + expect(output).not.toContain('fn15'); + expect(output).toContain('... and 2 more'); + }); + + it('truncates affected_processes list beyond 10', async () => { + const processes = Array.from({ length: 12 }, (_, i) => ({ + name: `proc${i}`, + step_count: 3, + changed_steps: [{ symbol: `sym${i}` }], + })); + callToolMock.mockResolvedValue({ + summary: { changed_files: 1, changed_count: 1, affected_count: 12, risk_level: 'low' }, + affected_processes: processes, + }); + const { detectChangesCommand } = await import('../../src/cli/tool.js'); + + await detectChangesCommand({}); + + const output: string = writeSyncMock.mock.calls[0][1]; + expect(output).toContain('proc9'); + expect(output).not.toContain('proc10'); + }); +}); From 06967e2b660d3183752ee026bbc43dedc2db6a86 Mon Sep 17 00:00:00 2001 From: Jonas Vanderhaegen Date: Mon, 20 Apr 2026 18:35:31 +0200 Subject: [PATCH 13/13] feat(extractors): add PHP HTTP consumer detection (#993) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Extend the PHP tree-sitter plugin to emit consumer HttpDetections for three common PHP HTTP call shapes, matching Node plugin parity: - Laravel HTTP client: Http::get/post/put/delete/patch($url) - Guzzle / generic: $client->get/post/...($url) - file_get_contents($url) when the URL is absolute http(s):// String-literal URLs only. Paths built via binary concatenation (`$base . '/path'`), sprintf, or config lookups are intentionally deferred — they need constant-folding of the enclosing scope to be useful and are tracked as follow-up work. Refs #992 Co-authored-by: Jonas Vanderhaegen --- .../group/extractors/http-patterns/php.ts | 162 +++++++++++++++--- .../unit/group/http-route-extractor.test.ts | 80 +++++++++ 2 files changed, 222 insertions(+), 20 deletions(-) diff --git a/gitnexus/src/core/group/extractors/http-patterns/php.ts b/gitnexus/src/core/group/extractors/http-patterns/php.ts index ae91c141b..c1c40a09c 100644 --- a/gitnexus/src/core/group/extractors/http-patterns/php.ts +++ b/gitnexus/src/core/group/extractors/http-patterns/php.ts @@ -3,33 +3,92 @@ import { compilePatterns, runCompiledPatterns, unquoteLiteral, + type CompiledPatterns, type LanguagePatterns, + type PatternSpec, } from '../tree-sitter-scanner.js'; import type { HttpDetection, HttpLanguagePlugin } from './types.js'; /** - * PHP HTTP plugin — Laravel `Route::get/post/...` declarations. + * PHP HTTP plugin. + * + * Providers: + * - Laravel `Route::get/post/...` + * + * Consumers (string-literal URLs only): + * - Laravel HTTP client: `Http::get/post/put/delete/patch($url)` + * - Guzzle / generic object method: `$client->get/post/...($url)` + * - `file_get_contents($url)` * * The pipeline already uses `PHP.php_only` for ingesting plain `.php` * files (see `core/tree-sitter/parser-loader.ts`), and we do the same * here so Laravel route files are parsed with the right grammar dialect. + * + * Scope notes: consumer patterns match string literals only. URLs built + * via binary concatenation (`$base . '/path'`), `sprintf`, or config + * lookup (`config('services.foo.base').'/path'`) are intentionally left + * for a follow-up — they require constant-folding the surrounding + * scope to be meaningful. */ -const LARAVEL_PATTERNS = compilePatterns({ - name: 'php-laravel', - language: PHP.php_only, - patterns: [ - { - meta: {}, - query: ` - (scoped_call_expression - scope: (name) @scope (#eq? @scope "Route") - name: (name) @method (#match? @method "^(get|post|put|delete|patch)$") - arguments: (arguments . (argument (string) @path))) - `, - }, - ], -} satisfies LanguagePatterns>); +const LARAVEL_ROUTE_SPEC: PatternSpec> = { + meta: {}, + query: ` + (scoped_call_expression + scope: (name) @scope (#eq? @scope "Route") + name: (name) @method (#match? @method "^(get|post|put|delete|patch)$") + arguments: (arguments . (argument (string) @path))) + `, +}; + +const HTTP_FACADE_SPEC: PatternSpec> = { + meta: {}, + query: ` + (scoped_call_expression + scope: (name) @scope (#eq? @scope "Http") + name: (name) @method (#match? @method "^(get|post|put|delete|patch)$") + arguments: (arguments . (argument (string) @path))) + `, +}; + +const GUZZLE_MEMBER_SPEC: PatternSpec> = { + meta: {}, + query: ` + (member_call_expression + name: (name) @method (#match? @method "^(get|post|put|delete|patch)$") + arguments: (arguments . (argument (string) @path))) + `, +}; + +const FILE_GET_CONTENTS_SPEC: PatternSpec> = { + meta: {}, + query: ` + (function_call_expression + function: (name) @fn (#eq? @fn "file_get_contents") + arguments: (arguments . (argument (string) @path))) + `, +}; + +interface PhpPatternBundle { + laravelRoute: CompiledPatterns>; + httpFacade: CompiledPatterns>; + guzzleMember: CompiledPatterns>; + fileGetContents: CompiledPatterns>; +} + +const mk = (spec: PatternSpec>, suffix: string) => + compilePatterns({ + name: `php-${suffix}`, + language: PHP.php_only, + patterns: [spec], + } satisfies LanguagePatterns>); + +const PHP_PATTERNS: PhpPatternBundle = { + laravelRoute: mk(LARAVEL_ROUTE_SPEC, 'laravel-route'), + httpFacade: mk(HTTP_FACADE_SPEC, 'http-facade'), + guzzleMember: mk(GUZZLE_MEMBER_SPEC, 'guzzle-member'), + fileGetContents: mk(FILE_GET_CONTENTS_SPEC, 'file-get-contents'), +}; /** * Extract the inner text of a PHP `string` node. The tree-sitter-php @@ -39,11 +98,8 @@ const LARAVEL_PATTERNS = compilePatterns({ * child nodes. */ function phpStringText(node: import('tree-sitter').SyntaxNode): string | null { - // Most single-quoted strings expose their inner content through the - // full node text (including quotes), which unquoteLiteral strips. const direct = unquoteLiteral(node.text); if (direct !== null && direct !== node.text) return direct; - // Fall back to child string_content / string_value node if present. for (const child of node.children) { if (child.type === 'string_content' || child.type === 'string_value') { return child.text; @@ -52,13 +108,32 @@ function phpStringText(node: import('tree-sitter').SyntaxNode): string | null { return direct; } +/** + * HTTP client helpers (`Http::`, Guzzle) are almost always called with + * a path relative to a configured base URL, or a full URL. File paths + * are rare. Accept both relative (`/api/...`) and absolute (`http(s)://`). + */ +function isHttpClientPath(path: string): boolean { + return path.startsWith('/') || path.startsWith('http://') || path.startsWith('https://'); +} + +/** + * `file_get_contents` is used for both HTTP and filesystem reads. Only + * emit a consumer contract when the URL is an absolute HTTP(S) URL to + * avoid false positives for local file paths and stream wrappers + * (`php://input`, `file://`, `data:`, ...). + */ +function isHttpUrlLiteral(path: string): boolean { + return path.startsWith('http://') || path.startsWith('https://'); +} + export const PHP_HTTP_PLUGIN: HttpLanguagePlugin = { name: 'php-http', language: PHP.php_only, scan(tree) { const out: HttpDetection[] = []; - for (const match of runCompiledPatterns(LARAVEL_PATTERNS, tree)) { + for (const match of runCompiledPatterns(PHP_PATTERNS.laravelRoute, tree)) { const methodNode = match.captures.method; const pathNode = match.captures.path; if (!methodNode || !pathNode) continue; @@ -74,6 +149,53 @@ export const PHP_HTTP_PLUGIN: HttpLanguagePlugin = { }); } + for (const match of runCompiledPatterns(PHP_PATTERNS.httpFacade, tree)) { + const methodNode = match.captures.method; + const pathNode = match.captures.path; + if (!methodNode || !pathNode) continue; + const path = phpStringText(pathNode); + if (path === null || !isHttpClientPath(path)) continue; + out.push({ + role: 'consumer', + framework: 'laravel-http', + method: methodNode.text.toUpperCase(), + path, + name: null, + confidence: 0.7, + }); + } + + for (const match of runCompiledPatterns(PHP_PATTERNS.guzzleMember, tree)) { + const methodNode = match.captures.method; + const pathNode = match.captures.path; + if (!methodNode || !pathNode) continue; + const path = phpStringText(pathNode); + if (path === null || !isHttpClientPath(path)) continue; + out.push({ + role: 'consumer', + framework: 'guzzle', + method: methodNode.text.toUpperCase(), + path, + name: null, + confidence: 0.7, + }); + } + + for (const match of runCompiledPatterns(PHP_PATTERNS.fileGetContents, tree)) { + const pathNode = match.captures.path; + if (!pathNode) continue; + const path = phpStringText(pathNode); + if (path === null || !isHttpUrlLiteral(path)) continue; + out.push({ + role: 'consumer', + framework: 'file-get-contents', + method: 'GET', + path, + name: null, + confidence: 0.7, + }); + } + return out; }, }; diff --git a/gitnexus/test/unit/group/http-route-extractor.test.ts b/gitnexus/test/unit/group/http-route-extractor.test.ts index 8ff914fcc..e81f1566e 100644 --- a/gitnexus/test/unit/group/http-route-extractor.test.ts +++ b/gitnexus/test/unit/group/http-route-extractor.test.ts @@ -517,6 +517,86 @@ Route::delete('/users/{id}', [UserController::class, 'destroy']); }); }); + describe('consumer extraction — PHP', () => { + it('extracts Laravel Http facade calls', async () => { + const dir = path.join(tmpDir, 'php-http-facade'); + fs.mkdirSync(path.join(dir, 'app'), { recursive: true }); + fs.writeFileSync( + path.join(dir, 'app/Client.php'), + ` c.role === 'consumer'); + + expect(consumers.find((c) => c.contractId === 'http::GET::/api/users')).toBeDefined(); + expect( + consumers.find((c) => c.contractId === 'http::POST::/api/orders/{param}'), + ).toBeDefined(); + expect( + consumers.find((c) => c.contractId === 'http::DELETE::/api/users/{param}'), + ).toBeDefined(); + }); + + it('extracts Guzzle $client->method() calls', async () => { + const dir = path.join(tmpDir, 'php-guzzle'); + fs.mkdirSync(path.join(dir, 'src'), { recursive: true }); + fs.writeFileSync( + path.join(dir, 'src/ApiClient.php'), + `get('/api/health'); + $client->post('/api/orders/42'); + } +} +`, + ); + + const contracts = await extractor.extract(null, dir, makeRepo(dir)); + const consumers = contracts.filter((c) => c.role === 'consumer'); + + expect(consumers.find((c) => c.contractId === 'http::GET::/api/health')).toBeDefined(); + expect( + consumers.find((c) => c.contractId === 'http::POST::/api/orders/{param}'), + ).toBeDefined(); + }); + + it('extracts file_get_contents HTTP calls', async () => { + const dir = path.join(tmpDir, 'php-fgc'); + fs.mkdirSync(path.join(dir, 'src'), { recursive: true }); + fs.writeFileSync( + path.join(dir, 'src/fetch.php'), + ` c.role === 'consumer'); + + expect(consumers.find((c) => c.contractId === 'http::GET::/api/items/{param}')).toBeDefined(); + // file paths and stream wrappers must not emit consumer contracts + expect(consumers.find((c) => c.meta.path === '/tmp/local-file.txt')).toBeUndefined(); + }); + }); + describe('provider extraction — FastAPI', () => { it('extracts FastAPI @app.get decorator patterns', async () => { const dir = path.join(tmpDir, 'fastapi');