From fdf1effb2a55b42fc95fd05dc1dc6fc1eb65b19a Mon Sep 17 00:00:00 2001 From: achianuri Date: Mon, 11 May 2026 07:00:58 -0700 Subject: [PATCH 1/2] feat(cli): add --skip-skills and --index-only flags to analyze (resubmit of #742) (#1485) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(cli): add --skip-skills and --index-only flags to analyze command The `installSkills()` call in `generateAIContextFiles()` runs unconditionally, injecting 6 skill files into `.claude/skills/gitnexus/` even when `--skip-agents-md` is passed. This is problematic for bulk indexing operations on read-only mirrors or third-party repos. Add two new flags: - `--skip-skills`: suppress standard GitNexus skill file injection - `--index-only`: pure index mode that suppresses all file injection (AGENTS.md, CLAUDE.md, and skills), writing only to `.gitnexus/` This gives users three levels of control: - `--skip-agents-md` — suppress only root context files - `--skip-skills` — suppress only skill injection - `--index-only` — suppress everything (pure indexing) Discovery context: while bulk-indexing 176 repos with `--skip-agents-md`, all 144 indexed repos were contaminated with `.claude/skills/gitnexus/` files requiring manual cleanup. * fix(cli): address PR #742 review — gate community skills, drop dangling refs, add tests Bot review (#742) flagged three issues with the original commit: 1. `--index-only --skills` still wrote community-derived skill files to `.claude/skills/generated/`. The `--skills` branch in analyze.ts was not gated by `skipAll`, so the "skip all file injection" contract was violated. Gate `generateSkillFiles()` with `!skipAll` so `--index-only` truly wins over `--skills`. 2. `--skip-skills` without `--skip-agents-md` produced AGENTS.md / CLAUDE.md that still referenced `.claude/skills/gitnexus/*/SKILL.md` files that were never installed — every agent load incurred 6 failed reads. Pass `skipSkills` through to `generateGitNexusContent()` and omit the standard-skill rows (and the entire `## CLI` heading when the table is empty). Community skills, when present via `--skills`, are unaffected. 3. No filesystem tests for `skipSkills` / `indexOnly`. Add three regression guards to `test/unit/ai-context.test.ts`: - `.claude/skills/gitnexus/` is NOT created when skipSkills=true - Nothing is written when both skipAgentsMd and skipSkills are true (the resolved-flag state from --index-only) - AGENTS.md/CLAUDE.md routing table omits standard skill references when skipSkills=true, but preserves the load-bearing imperative sections (Always Do / Never Do / Resources) * test(cli): PR 1485 review follow-ups (help text, gate test, --skip-skills docs) - Assert --skip-skills and --index-only in analyze --help (skip-git-cli.test.ts). - Export shouldGenerateCommunitySkillFiles; unit-test index-only+skills gate. - Clarify --skip-skills does not suppress --skills community files; --index-only for full skip. Co-authored-by: Cursor * fix(cli): warn when --index-only silently overrides --skills Address review findings on PR 1485 follow-ups: - analyze.ts emits a one-line note when both --index-only and --skills are set, so users see why a pipeline re-index ran with no skill files written. - index.ts --skills help text now flags the --index-only override. - shouldGenerateCommunitySkillFiles JSDoc documents the dual role of the gate (community skills + AGENTS.md/CLAUDE.md re-generation). - skip-git-cli.test.ts pins the override-warning surface end-to-end. --------- Co-authored-by: Gergő Magyar Co-authored-by: Cursor --- gitnexus/src/cli/ai-context.ts | 43 +++++++-- gitnexus/src/cli/analyze.ts | 48 +++++++++- gitnexus/src/cli/index.ts | 13 ++- gitnexus/src/core/run-analyze.ts | 8 +- gitnexus/test/unit/ai-context.test.ts | 95 +++++++++++++++++++ .../analyze-community-skills-gate.test.ts | 27 ++++++ gitnexus/test/unit/skip-git-cli.test.ts | 32 +++++++ 7 files changed, 250 insertions(+), 16 deletions(-) create mode 100644 gitnexus/test/unit/analyze-community-skills-gate.test.ts diff --git a/gitnexus/src/cli/ai-context.ts b/gitnexus/src/cli/ai-context.ts index 41ba6c73e..8e18fed7d 100644 --- a/gitnexus/src/cli/ai-context.ts +++ b/gitnexus/src/cli/ai-context.ts @@ -28,6 +28,7 @@ interface RepoStats { export interface AIContextOptions { skipAgentsMd?: boolean; noStats?: boolean; + skipSkills?: boolean; } const GITNEXUS_START_MARKER = ''; @@ -94,6 +95,7 @@ function generateGitNexusContent( generatedSkills?: GeneratedSkillInfo[], groupNames?: string[], noStats?: boolean, + skipSkills?: boolean, ): string { const generatedRows = generatedSkills && generatedSkills.length > 0 @@ -105,14 +107,26 @@ function generateGitNexusContent( .join('\n') : ''; - const skillsTable = `| Task | Read this skill file | -|------|---------------------| -| Understand architecture / "How does X work?" | \`.claude/skills/gitnexus/gitnexus-exploring/SKILL.md\` | + // Standard skill rows reference files installed by installSkills(). When + // --skip-skills suppresses that install, these rows must be omitted — else + // AGENTS.md/CLAUDE.md would direct agents to read files that don't exist. + // Community skills (generatedRows) live in .claude/skills/generated/ and + // are independent of --skip-skills, so they remain when present. + const standardSkillsRows = skipSkills + ? '' + : `| Understand architecture / "How does X work?" | \`.claude/skills/gitnexus/gitnexus-exploring/SKILL.md\` | | Blast radius / "What breaks if I change X?" | \`.claude/skills/gitnexus/gitnexus-impact-analysis/SKILL.md\` | | Trace bugs / "Why is X failing?" | \`.claude/skills/gitnexus/gitnexus-debugging/SKILL.md\` | | Rename / extract / split / refactor | \`.claude/skills/gitnexus/gitnexus-refactoring/SKILL.md\` | | Tools, resources, schema reference | \`.claude/skills/gitnexus/gitnexus-guide/SKILL.md\` | -| Index, status, clean, wiki CLI commands | \`.claude/skills/gitnexus/gitnexus-cli/SKILL.md\` |${generatedRows ? '\n' + generatedRows : ''}`; +| Index, status, clean, wiki CLI commands | \`.claude/skills/gitnexus/gitnexus-cli/SKILL.md\` |`; + + const tableBody = [standardSkillsRows, generatedRows].filter(Boolean).join('\n'); + const skillsTable = tableBody + ? `| Task | Read this skill file | +|------|---------------------| +${tableBody}` + : ''; return `${GITNEXUS_START_MARKER} # GitNexus — Code Intelligence @@ -153,11 +167,15 @@ This repository is listed under GitNexus **group(s): ${groupNames.join(', ')}** ` : '' -}## CLI +}${ + skillsTable + ? `## CLI ${skillsTable} -${GITNEXUS_END_MARKER}`; +` + : '' + }${GITNEXUS_END_MARKER}`; } /** @@ -319,6 +337,7 @@ export async function generateAIContextFiles( generatedSkills, groupNames, options?.noStats, + options?.skipSkills, ); const createdFiles: string[] = []; @@ -337,10 +356,14 @@ export async function generateAIContextFiles( createdFiles.push('CLAUDE.md (skipped via --skip-agents-md)'); } - // Install skills to .claude/skills/gitnexus/ - const installedSkills = await installSkills(repoPath); - if (installedSkills.length > 0) { - createdFiles.push(`.claude/skills/gitnexus/ (${installedSkills.length} skills)`); + // Install skills to .claude/skills/gitnexus/ (unless --skip-skills) + if (!options?.skipSkills) { + const installedSkills = await installSkills(repoPath); + if (installedSkills.length > 0) { + createdFiles.push(`.claude/skills/gitnexus/ (${installedSkills.length} skills)`); + } + } else { + createdFiles.push('.claude/skills/gitnexus/ (skipped via --skip-skills)'); } return { files: createdFiles }; diff --git a/gitnexus/src/cli/analyze.ts b/gitnexus/src/cli/analyze.ts index 47745c575..d5a7638f9 100644 --- a/gitnexus/src/cli/analyze.ts +++ b/gitnexus/src/cli/analyze.ts @@ -119,6 +119,10 @@ export interface AnalyzeOptions { skipAgentsMd?: boolean; /** Omit volatile symbol/relationship counts from AGENTS.md and CLAUDE.md. */ noStats?: boolean; + /** Skip installing standard GitNexus skill files to .claude/skills/gitnexus/. */ + skipSkills?: boolean; + /** Pure index mode: skip all file injection (AGENTS.md, CLAUDE.md, skills). */ + indexOnly?: boolean; /** Index the folder even when no .git directory is present. */ skipGit?: boolean; /** @@ -150,6 +154,24 @@ export interface AnalyzeOptions { embeddingDevice?: string; } +/** + * Whether the post-index skill step should run. + * + * The gated block does two things in sequence: (1) generates the community + * skill files from `--skills`, and (2) re-runs `generateAIContextFiles` so + * AGENTS.md/CLAUDE.md can reference the freshly written skills. Both are + * suppressed together — `--index-only` drops the entire step, not just the + * community-skill write. Name retained for the test contract; see call site + * in `analyzeCommand` for the AGENTS.md/CLAUDE.md re-generation it also gates. + * + * Kept as a pure helper so the `--index-only --skills` contract is unit-tested + * without booting the full analyze pipeline (#742 review). + */ +export const shouldGenerateCommunitySkillFiles = ( + options: Pick | undefined, + pipelineResult: unknown, +): boolean => Boolean(options?.skills && pipelineResult && !options?.indexOnly); + export const analyzeCommand = async (inputPath?: string, options?: AnalyzeOptions) => { if (ensureHeap()) return; @@ -245,6 +267,18 @@ export const analyzeCommand = async (inputPath?: string, options?: AnalyzeOption console.log('\n GitNexus Analyzer\n'); + // `--index-only` is the stronger contract — it suppresses every form of file + // injection, including community skill writes that `--skills` would normally + // produce. Surface the override explicitly so users don't wonder why a + // pipeline re-index ran but no skill files appeared. The pipeline still + // re-runs (see `force: options?.force || options?.skills` below); the warning + // is purely about the dropped post-index write step. + if (options?.indexOnly && options?.skills) { + console.log( + ' Note: --index-only overrides --skills; community skill files will not be written.\n', + ); + } + let repoPath: string; if (inputPath) { repoPath = path.resolve(inputPath); @@ -399,6 +433,9 @@ export const analyzeCommand = async (inputPath?: string, options?: AnalyzeOption // ── Run shared analysis orchestrator ─────────────────────────────── try { + const skipAll = options?.indexOnly; + const skipAgentsMd = skipAll || options?.skipAgentsMd; + const skipSkills = skipAll || options?.skipSkills; const result = await runFullAnalysis( repoPath, { @@ -410,7 +447,8 @@ export const analyzeCommand = async (inputPath?: string, options?: AnalyzeOption embeddingsNodeLimit, dropEmbeddings: options?.dropEmbeddings, skipGit: options?.skipGit, - skipAgentsMd: options?.skipAgentsMd, + skipAgentsMd, + skipSkills, noStats: options?.noStats, registryName: options?.name, // Registry-collision bypass — its own CLI flag, intentionally NOT @@ -456,8 +494,10 @@ export const analyzeCommand = async (inputPath?: string, options?: AnalyzeOption // a healthy index. await assertAnalysisFinalized(repoPath); - // Skill generation (CLI-only, uses pipeline result from analysis) - if (options?.skills && result.pipelineResult) { + // Skill generation (CLI-only, uses pipeline result from analysis). + // Gated so `--index-only --skills` skips community skill writes too + // (`shouldGenerateCommunitySkillFiles` — see unit test). + if (shouldGenerateCommunitySkillFiles(options, result.pipelineResult)) { updateBar(99, 'Generating skill files...'); try { const { generateSkillFiles } = await import('./skill-gen.js'); @@ -497,7 +537,7 @@ export const analyzeCommand = async (inputPath?: string, options?: AnalyzeOption processes: s.processes, }, skillResult.skills, - { skipAgentsMd: options?.skipAgentsMd, noStats: options?.noStats }, + { skipAgentsMd, skipSkills, noStats: options?.noStats }, ); } } catch { diff --git a/gitnexus/src/cli/index.ts b/gitnexus/src/cli/index.ts index e4a455d40..d38675f03 100644 --- a/gitnexus/src/cli/index.ts +++ b/gitnexus/src/cli/index.ts @@ -33,9 +33,20 @@ program 'Drop existing embeddings on rebuild. By default, an `analyze` without `--embeddings` ' + 'preserves any embeddings already present in the index.', ) - .option('--skills', 'Generate repo-specific skill files from detected communities') + .option( + '--skills', + 'Generate repo-specific skill files from detected communities ' + + '(no-op when --index-only is also set).', + ) .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-skills', + 'Skip installing standard GitNexus skill files under .claude/skills/gitnexus/. ' + + 'Does not suppress community skills from --skills (those use .claude/skills/generated/). ' + + 'Use --index-only to skip all AI-context file injection.', + ) + .option('--index-only', 'Pure index mode: skip all file injection (AGENTS.md, CLAUDE.md, skills)') .option( '--skip-git', 'Treat the provided path/cwd as the index root and skip parent git-root discovery', diff --git a/gitnexus/src/core/run-analyze.ts b/gitnexus/src/core/run-analyze.ts index 7227fd945..fa2757f45 100644 --- a/gitnexus/src/core/run-analyze.ts +++ b/gitnexus/src/core/run-analyze.ts @@ -81,6 +81,8 @@ export interface AnalyzeOptions { skipAgentsMd?: boolean; /** Omit volatile symbol/relationship counts from AGENTS.md and CLAUDE.md. */ noStats?: boolean; + /** Skip installing standard GitNexus skill files to .claude/skills/gitnexus/. */ + skipSkills?: boolean; /** * User-provided alias for the registry `name` (#829). When set, * forwarded to `registerRepo` so the indexed repo is stored under @@ -527,7 +529,11 @@ export async function runFullAnalysis( processes: pipelineResult.processResult?.stats.totalProcesses, }, undefined, - { skipAgentsMd: options.skipAgentsMd, noStats: options.noStats }, + { + skipAgentsMd: options.skipAgentsMd, + skipSkills: options.skipSkills, + noStats: options.noStats, + }, ); } catch { // Best-effort — don't fail the entire analysis for context file issues diff --git a/gitnexus/test/unit/ai-context.test.ts b/gitnexus/test/unit/ai-context.test.ts index bbaecb932..71d7ddbdc 100644 --- a/gitnexus/test/unit/ai-context.test.ts +++ b/gitnexus/test/unit/ai-context.test.ts @@ -137,6 +137,101 @@ describe('generateAIContextFiles', () => { } }); + it('does not create .claude/skills/gitnexus/ when skipSkills is true (#742)', async () => { + // Regression guard for #742. The --skip-skills flag must prevent + // installSkills() from writing the 6 standard skill dirs into the + // analyzed repo. Per-test tmpdir so we start from a known-clean + // slate — the shared tmpDir from beforeAll may already contain + // .claude/skills/gitnexus/ from an earlier test. + const skipDir = await fs.mkdtemp(path.join(os.tmpdir(), 'gn-ai-ctx-skip-skills-')); + const skipStorage = path.join(skipDir, '.gitnexus'); + await fs.mkdir(skipStorage, { recursive: true }); + try { + const stats = { nodes: 50, edges: 100, processes: 5 }; + const result = await generateAIContextFiles( + skipDir, + skipStorage, + 'TestProject', + stats, + undefined, + { skipSkills: true }, + ); + + expect(result.files).toContain('.claude/skills/gitnexus/ (skipped via --skip-skills)'); + await expect( + fs.access(path.join(skipDir, '.claude', 'skills', 'gitnexus')), + ).rejects.toThrow(); + } finally { + await fs.rm(skipDir, { recursive: true, force: true }); + } + }); + + it('writes nothing when both skipAgentsMd and skipSkills are true (--index-only, #742)', async () => { + // Regression guard for #742. analyzeCommand() resolves --index-only + // into BOTH skipAgentsMd=true and skipSkills=true. This test pins + // the resolved-flag combination so a future regression that drops + // either guard fails here. Per-test tmpdir for the same reason as + // the skipSkills test above. + const idxDir = await fs.mkdtemp(path.join(os.tmpdir(), 'gn-ai-ctx-index-only-')); + const idxStorage = path.join(idxDir, '.gitnexus'); + await fs.mkdir(idxStorage, { recursive: true }); + try { + const stats = { nodes: 50, edges: 100, processes: 5 }; + const result = await generateAIContextFiles( + idxDir, + idxStorage, + 'TestProject', + stats, + undefined, + { skipAgentsMd: true, skipSkills: true }, + ); + + expect(result.files).toContain('AGENTS.md (skipped via --skip-agents-md)'); + expect(result.files).toContain('CLAUDE.md (skipped via --skip-agents-md)'); + expect(result.files).toContain('.claude/skills/gitnexus/ (skipped via --skip-skills)'); + + await expect(fs.access(path.join(idxDir, 'AGENTS.md'))).rejects.toThrow(); + await expect(fs.access(path.join(idxDir, 'CLAUDE.md'))).rejects.toThrow(); + await expect(fs.access(path.join(idxDir, '.claude', 'skills', 'gitnexus'))).rejects.toThrow(); + } finally { + await fs.rm(idxDir, { recursive: true, force: true }); + } + }); + + it('omits standard skill references from AGENTS.md/CLAUDE.md when skipSkills is true (#742)', async () => { + // The skills routing table in AGENTS.md/CLAUDE.md points agents at + // .claude/skills/gitnexus/*/SKILL.md files installed by installSkills(). + // When --skip-skills suppresses that install but AGENTS.md/CLAUDE.md + // are still written, the routing table must NOT name files that don't + // exist — otherwise every agent load incurs 6 failed reads and the + // routing instructions are worthless. Per-test tmpdir so the assertions + // are not contaminated by a CLAUDE.md from an earlier test. + const noStdDir = await fs.mkdtemp(path.join(os.tmpdir(), 'gn-ai-ctx-no-std-skills-')); + const noStdStorage = path.join(noStdDir, '.gitnexus'); + await fs.mkdir(noStdStorage, { recursive: true }); + try { + const stats = { nodes: 50, edges: 100, processes: 5 }; + await generateAIContextFiles(noStdDir, noStdStorage, 'TestProject', stats, undefined, { + skipSkills: true, + }); + + const content = await fs.readFile(path.join(noStdDir, 'CLAUDE.md'), 'utf-8'); + expect(content).not.toContain('gitnexus-exploring/SKILL.md'); + expect(content).not.toContain('gitnexus-impact-analysis/SKILL.md'); + expect(content).not.toContain('gitnexus-debugging/SKILL.md'); + expect(content).not.toContain('gitnexus-refactoring/SKILL.md'); + expect(content).not.toContain('gitnexus-guide/SKILL.md'); + expect(content).not.toContain('gitnexus-cli/SKILL.md'); + // The load-bearing imperative sections must still ship — only the + // routing rows are conditional. + expect(content).toContain('## Always Do'); + expect(content).toContain('## Never Do'); + expect(content).toContain('gitnexus://repo/TestProject/context'); + } finally { + await fs.rm(noStdDir, { recursive: true, force: true }); + } + }); + it('preserves manual AGENTS.md and CLAUDE.md edits when skipAgentsMd is enabled', async () => { const stats = { nodes: 42, edges: 84, processes: 3 }; const agentsPath = path.join(tmpDir, 'AGENTS.md'); diff --git a/gitnexus/test/unit/analyze-community-skills-gate.test.ts b/gitnexus/test/unit/analyze-community-skills-gate.test.ts new file mode 100644 index 000000000..35eaa5c77 --- /dev/null +++ b/gitnexus/test/unit/analyze-community-skills-gate.test.ts @@ -0,0 +1,27 @@ +import { describe, it, expect } from 'vitest'; +import { shouldGenerateCommunitySkillFiles } from '../../src/cli/analyze.js'; + +describe('shouldGenerateCommunitySkillFiles (#742 / PR 1485)', () => { + it('is false when --index-only is set even if --skills and pipelineResult are present', () => { + expect(shouldGenerateCommunitySkillFiles({ skills: true, indexOnly: true }, { ok: true })).toBe( + false, + ); + }); + + it('is false when pipelineResult is missing', () => { + expect(shouldGenerateCommunitySkillFiles({ skills: true, indexOnly: false }, null)).toBe(false); + expect(shouldGenerateCommunitySkillFiles({ skills: true }, undefined)).toBe(false); + }); + + it('is true when --skills is set, pipeline exists, and not index-only', () => { + expect( + shouldGenerateCommunitySkillFiles({ skills: true, indexOnly: false }, { communities: [] }), + ).toBe(true); + expect(shouldGenerateCommunitySkillFiles({ skills: true }, { x: 1 })).toBe(true); + }); + + it('is false when --skills is omitted', () => { + expect(shouldGenerateCommunitySkillFiles({ indexOnly: false }, { x: 1 })).toBe(false); + expect(shouldGenerateCommunitySkillFiles(undefined, { x: 1 })).toBe(false); + }); +}); diff --git a/gitnexus/test/unit/skip-git-cli.test.ts b/gitnexus/test/unit/skip-git-cli.test.ts index 069a9686f..80c07ed17 100644 --- a/gitnexus/test/unit/skip-git-cli.test.ts +++ b/gitnexus/test/unit/skip-git-cli.test.ts @@ -17,9 +17,41 @@ describe('--skip-git CLI flag', () => { expect(helpOutput).toContain('--skip-git'); expect(helpOutput).toContain('--skip-agents-md'); + expect(helpOutput).toContain('--skip-skills'); + expect(helpOutput).toContain('--index-only'); expect(helpOutput).not.toContain('--no-git'); }); + it('warns when --index-only overrides --skills (PR 1485)', () => { + // `--index-only` suppresses the post-index skill step that `--skills` + // would otherwise trigger. Without an explicit warning, the user sees a + // pipeline re-index complete and silently no skill files written — the + // silent-contradiction case flagged in PR 1485 review. + const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gn-index-only-skills-')); + const gitnexusHome = fs.mkdtempSync(path.join(os.tmpdir(), 'gn-index-only-skills-home-')); + // Make tmpDir a git repo so analyze accepts it without --skip-git. + execSync('git init', { cwd: tmpDir, stdio: 'ignore' }); + fs.writeFileSync(path.join(tmpDir, 'a.ts'), 'export const a = 1;\n'); + + const env = { + ...process.env, + HOME: gitnexusHome, + GITNEXUS_HOME: gitnexusHome, + GITNEXUS_LBUG_EXTENSION_INSTALL: 'never', + }; + + try { + const output = execSync( + `node "${cliPath}" analyze "${tmpDir}" --index-only --skills --skip-agents-md`, + { encoding: 'utf8', timeout: 60000, env }, + ); + expect(output).toContain('--index-only overrides --skills'); + } finally { + fs.rmSync(tmpDir, { recursive: true, force: true }); + fs.rmSync(gitnexusHome, { recursive: true, force: true }); + } + }); + it('rejects non-git folder without --skip-git', () => { const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gn-no-git-')); fs.writeFileSync(path.join(tmpDir, 'test.ts'), 'export const x = 1;'); From fcab1e2e8207a469ae4996b5a75ae346d98d4b7c Mon Sep 17 00:00:00 2001 From: Antheurus <91123607+Antheurus@users.noreply.github.com> Date: Mon, 11 May 2026 22:39:10 +0700 Subject: [PATCH 2/2] fix(augment): add CONTAINS fallback when FTS indexes unavailable (#1476) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(augment): add CONTAINS fallback when FTS indexes unavailable When the MCP server holds the KuzuDB write lock, the augment CLI opens the DB read-only. FTS indexes cannot be created in read-only mode, so searchFTSFromLbug returns ftsAvailable=false and an empty results array. The existing early-return path silently produced no enrichment. Add a Cypher name CONTAINS fallback that fires only when ftsAvailable is false and BM25 produced no symbol matches. This covers the read-only DB case (concurrent MCP server) and the first-run case (indexes not yet built). The fallback is wrapped in .catch(() => []) and cannot throw. When FTS indexes exist, this branch is never reached — behaviour is unchanged for users without a concurrent MCP server. * fix(augment): guard against CONTAINS '' and add no-FTS test coverage Blocker 1 — CONTAINS '' on whitespace-leading patterns: pattern.split(/\s+/)[0] returns "" when the input has leading whitespace (e.g. " ".split(/\s+/) → ["", ""]). In Kuzu, CONTAINS '' matches every node with a name property, injecting arbitrary graph nodes into LLM context. Fix: trim() before split, then guard on !firstWord || firstWord.length < 2. No behaviour change for normal non-empty patterns. Blocker 2 — zero test coverage on the FTS-unavailable code path: The new CONTAINS fallback block (engine.ts lines 146-166) was exercised by no existing test — all existing tests run with FTS indexes built. A second withTestLbugDB fixture is added with no ftsIndexes, forcing searchFTSFromLbug to return ftsAvailable: false, and asserts: 1. augment('login', ...) returns non-empty enrichment (fallback works) 2. augment(' ', ...) returns '' (CONTAINS '' guard holds) 3. augment('nxyz_notfound', ...) returns '' (no matching nodes) 4. executeQuery throwing returns '' (.catch(() => []) path) * fix(augment): extend CONTAINS '' guard to FTS happy path and consolidate The same split(/\s+/)[0] bug existed at line 125 (BM25 symbol filter, FTS-available path) — a leading-whitespace pattern produced CONTAINS '' there too, matching every node in BM25-matched files. Fix: hoist patternFirstWord computation with trim() and the length guard to the top of augment(), before any DB interaction. Both CONTAINS sites (BM25 symbol filter and CONTAINS fallback) now use the single pre-validated value. No behaviour change for normal patterns; the guard fires once for all callers instead of being duplicated. Also tighten the whitespace test in the no-FTS suite from 3 spaces to 4 spaces so it unambiguously exercises the patternFirstWord guard rather than straddling the outer pattern.length < 3 boundary. * test(augment): negative-safety test for ftsAvailable=true gate Asserts the CONTAINS fallback does NOT fire when FTS is available but BM25 returns zero results. Pins the safety property promised by the PR description: behavior is unchanged for users without the read-only-DB condition. If anyone later loosens the gate to `symbolMatches.length === 0` alone, this test fails. --------- Co-authored-by: Gergő Magyar --- gitnexus/src/core/augmentation/engine.ts | 32 ++++++- .../test/integration/augmentation.test.ts | 85 +++++++++++++++++++ 2 files changed, 113 insertions(+), 4 deletions(-) diff --git a/gitnexus/src/core/augmentation/engine.ts b/gitnexus/src/core/augmentation/engine.ts index 42087e4b0..81e41077e 100644 --- a/gitnexus/src/core/augmentation/engine.ts +++ b/gitnexus/src/core/augmentation/engine.ts @@ -86,6 +86,9 @@ async function findRepoForCwd(cwd: string): Promise<{ export async function augment(pattern: string, cwd?: string): Promise { if (!pattern || pattern.length < 3) return ''; + const patternFirstWord = pattern.trim().replace(/'/g, "''").split(/\s+/)[0]; + if (!patternFirstWord || patternFirstWord.length < 2) return ''; + const workDir = cwd || process.cwd(); try { @@ -104,9 +107,7 @@ export async function augment(pattern: string, cwd?: string): Promise { } // Step 1: BM25 search (fast, no embeddings) - const { results: bm25Results } = await searchFTSFromLbug(pattern, 10, repoId); - - if (bm25Results.length === 0) return ''; + const { results: bm25Results, ftsAvailable } = await searchFTSFromLbug(pattern, 10, repoId); // Step 2: Map BM25 file results to symbols const symbolMatches: Array<{ @@ -124,7 +125,7 @@ export async function augment(pattern: string, cwd?: string): Promise { repoId, ` MATCH (n) WHERE n.filePath = '${escaped}' - AND n.name CONTAINS '${pattern.replace(/'/g, "''").split(/\s+/)[0]}' + AND n.name CONTAINS '${patternFirstWord}' RETURN n.id AS id, n.name AS name, labels(n)[0] AS type, n.filePath AS filePath LIMIT 3 `, @@ -143,6 +144,29 @@ export async function augment(pattern: string, cwd?: string): Promise { } } + // When FTS indexes are unavailable (read-only DB, first run before indexes are built), + // fall back to a direct name CONTAINS query so enrichment still works. + if (symbolMatches.length === 0 && !ftsAvailable) { + const fallbackRows = await executeQuery( + repoId, + ` + MATCH (n) + WHERE n.name CONTAINS '${patternFirstWord}' + RETURN n.id AS id, n.name AS name, labels(n)[0] AS type, n.filePath AS filePath + LIMIT 5 + `, + ).catch(() => []); + for (const sym of fallbackRows) { + symbolMatches.push({ + nodeId: sym.id || sym[0], + name: sym.name || sym[1], + type: sym.type || sym[2], + filePath: sym.filePath || sym[3], + score: 1.0, + }); + } + } + if (symbolMatches.length === 0) return ''; // Step 3: Batch-fetch callers/callees/processes/cohesion for top matches diff --git a/gitnexus/test/integration/augmentation.test.ts b/gitnexus/test/integration/augmentation.test.ts index aed09d61f..60c4170fd 100644 --- a/gitnexus/test/integration/augmentation.test.ts +++ b/gitnexus/test/integration/augmentation.test.ts @@ -57,6 +57,7 @@ vi.mock('../../src/storage/repo-manager.js', () => ({ })); let augment: (pattern: string, cwd?: string) => Promise; +let augmentNoFts: (pattern: string, cwd?: string) => Promise; withTestLbugDB( 'augment', @@ -106,6 +107,28 @@ withTestLbugDB( const result = await augment('日本語テスト', handle.dbPath); expect(typeof result).toBe('string'); }); + + // ─── Negative-safety: fallback must stay gated on !ftsAvailable ─── + // + // When FTS is available but happens to return zero BM25 hits, the + // CONTAINS fallback must NOT fire — preserving the original early-return + // semantics. If anyone later loosens the gate to `symbolMatches.length + // === 0` alone, this test fails. + + it('does NOT fire CONTAINS fallback when FTS is available but BM25 returns empty', async () => { + const bm25 = await import('../../src/core/search/bm25-index.js'); + const spy = vi + .spyOn(bm25, 'searchFTSFromLbug') + .mockResolvedValue({ results: [], ftsAvailable: true }); + try { + // 'login' WOULD match a graph node via CONTAINS, but FTS is available + // and empty → fallback gate must hold → result must be ''. + const result = await augment('login', handle.dbPath); + expect(result).toBe(''); + } finally { + spy.mockRestore(); + } + }); }); }, { @@ -131,3 +154,65 @@ withTestLbugDB( }, }, ); + +// ─── FTS-unavailable suite: exercises the CONTAINS fallback branch ──────────── +// +// No ftsIndexes → searchFTSFromLbug returns ftsAvailable: false → fallback fires. +// Same seed data so 'login' still exists as a graph node. + +withTestLbugDB( + 'augment-no-fts', + (handle) => { + describe('augment() — FTS indexes unavailable (CONTAINS fallback)', () => { + it('falls back to CONTAINS query and returns enrichment when FTS is unavailable', async () => { + const result = await augmentNoFts('login', handle.dbPath); + + expect(result.length).toBeGreaterThan(0); + expect(result).toContain('[GitNexus]'); + }); + + it("returns empty string for whitespace-only pattern (CONTAINS '' guard)", async () => { + const result = await augmentNoFts(' ', handle.dbPath); + expect(result).toBe(''); + }); + + it('returns empty string when no nodes match the CONTAINS query', async () => { + const result = await augmentNoFts('nxyz_notfound', handle.dbPath); + expect(result).toBe(''); + }); + + it('returns empty string when fallback CONTAINS query throws', async () => { + const poolAdapter = await import('../../src/core/lbug/pool-adapter.js'); + const spy = vi + .spyOn(poolAdapter, 'executeQuery') + .mockRejectedValue(new Error('simulated DB error')); + try { + const result = await augmentNoFts('login', handle.dbPath); + expect(result).toBe(''); + } finally { + spy.mockRestore(); + } + }); + }); + }, + { + seed: AUGMENT_SEED_DATA, + // Intentionally no ftsIndexes — forces searchFTSFromLbug to return ftsAvailable: false + poolAdapter: true, + afterSetup: async (handle) => { + const { listRegisteredRepos } = await import('../../src/storage/repo-manager.js'); + (listRegisteredRepos as ReturnType).mockResolvedValue([ + { + name: handle.repoId, + path: handle.dbPath, + storagePath: handle.tmpHandle.dbPath, + indexedAt: new Date().toISOString(), + lastCommit: 'abc123', + }, + ]); + + const engine = await import('../../src/core/augmentation/engine.js'); + augmentNoFts = engine.augment; + }, + }, +);