fix(analyze): address review — rename --no-git to --skip-git, fix stale cache
Addresses all review items from @magyargergo and Copilot: 1. **Rename --no-git to --skip-git**: Commander.js treats --no-X flags as negation of --X (stores as options.git = false, not options.noGit). --skip-git maps correctly to options.skipGit. 2. **Fix false " Already up to date\ on non-git folders**: When currentCommit is empty string, skip the cache check — we cannot detect changes without git, so always rebuild. 3. **Replace isGitRepo() with hasGitDir()**: Use filesystem check (statSync on .git) instead of shelling out to git CLI. Consistent, faster, and works when git is not installed. 4. **Fix misleading warning**: Message now only fires when .git directory is actually absent (not when git CLI fails). 5. **Add CLI integration tests**: Verify Commander maps --skip-git correctly and that non-git folders are rejected without the flag.
This commit is contained in:
+16
-13
@@ -15,7 +15,7 @@ import { initLbug, loadGraphToLbug, getLbugStats, executeQuery, executeWithReuse
|
||||
// versions whose ABI is not yet supported by the native binary (#89).
|
||||
// disposeEmbedder intentionally not called — ONNX Runtime segfaults on cleanup (see #38)
|
||||
import { getStoragePaths, saveMeta, loadMeta, addToGitignore, registerRepo, getGlobalRegistryPath, cleanupOldKuzuFiles } from '../storage/repo-manager.js';
|
||||
import { getCurrentCommit, isGitRepo, getGitRoot, hasGitDir } from '../storage/git.js';
|
||||
import { getCurrentCommit, getGitRoot, hasGitDir } from '../storage/git.js';
|
||||
import { generateAIContextFiles } from './ai-context.js';
|
||||
import { generateSkillFiles, type GeneratedSkillInfo } from './skill-gen.js';
|
||||
import fs from 'fs/promises';
|
||||
@@ -49,7 +49,7 @@ export interface AnalyzeOptions {
|
||||
skills?: boolean;
|
||||
verbose?: boolean;
|
||||
/** Index the folder even when no .git directory is present. */
|
||||
noGit?: boolean;
|
||||
skipGit?: boolean;
|
||||
}
|
||||
|
||||
/** Threshold: auto-skip embeddings for repos with more nodes than this */
|
||||
@@ -89,26 +89,26 @@ export const analyzeCommand = async (
|
||||
} else {
|
||||
const gitRoot = getGitRoot(process.cwd());
|
||||
if (!gitRoot) {
|
||||
if (!options?.noGit) {
|
||||
console.log(' Not inside a git repository.\n Tip: pass --no-git to index any folder without a .git directory.\n');
|
||||
if (!options?.skipGit) {
|
||||
console.log(' Not inside a git repository.\n Tip: pass --skip-git to index any folder without a .git directory.\n');
|
||||
process.exitCode = 1;
|
||||
return;
|
||||
}
|
||||
// --no-git: fall back to cwd as the root
|
||||
// --skip-git: fall back to cwd as the root
|
||||
repoPath = path.resolve(process.cwd());
|
||||
} else {
|
||||
repoPath = gitRoot;
|
||||
}
|
||||
}
|
||||
|
||||
const repoHasGit = isGitRepo(repoPath);
|
||||
if (!repoHasGit && !options?.noGit) {
|
||||
console.log(' Not a git repository.\n Tip: pass --no-git to index any folder without a .git directory.\n');
|
||||
const repoHasGit = hasGitDir(repoPath);
|
||||
if (!repoHasGit && !options?.skipGit) {
|
||||
console.log(' Not a git repository.\n Tip: pass --skip-git to index any folder without a .git directory.\n');
|
||||
process.exitCode = 1;
|
||||
return;
|
||||
}
|
||||
if (!repoHasGit) {
|
||||
console.log(' Warning: no .git directory found — commit-tracking and incremental updates disabled.\n');
|
||||
console.log(' Warning: no .git directory found \u2014 commit-tracking and incremental updates disabled.\n');
|
||||
}
|
||||
|
||||
const { storagePath, lbugPath } = getStoragePaths(repoPath);
|
||||
@@ -124,8 +124,11 @@ export const analyzeCommand = async (
|
||||
const existingMeta = await loadMeta(storagePath);
|
||||
|
||||
if (existingMeta && !options?.force && !options?.skills && existingMeta.lastCommit === currentCommit) {
|
||||
console.log(' Already up to date\n');
|
||||
return;
|
||||
// Non-git folders have currentCommit = '' — always rebuild since we can't detect changes
|
||||
if (currentCommit !== '') {
|
||||
console.log(' Already up to date\n');
|
||||
return;
|
||||
}
|
||||
}
|
||||
|
||||
if (process.env.GITNEXUS_NO_GITIGNORE) {
|
||||
@@ -329,8 +332,8 @@ export const analyzeCommand = async (
|
||||
await saveMeta(storagePath, meta);
|
||||
await registerRepo(repoPath, meta);
|
||||
// Only attempt to update .gitignore when a .git directory is present.
|
||||
// Use hasGitDir (filesystem check) rather than isGitRepo (shells out to git)
|
||||
// so we skip correctly for --no-git folders even if git CLI is available.
|
||||
// Use hasGitDir (filesystem check) rather than git CLI subprocess
|
||||
// so we skip correctly for --skip-git folders even if git CLI is available.
|
||||
if (hasGitDir(repoPath)) {
|
||||
await addToGitignore(repoPath);
|
||||
}
|
||||
|
||||
@@ -28,7 +28,7 @@ program
|
||||
.option('-f, --force', 'Force full re-index even if up to date')
|
||||
.option('--embeddings', 'Enable embedding generation for semantic search (off by default)')
|
||||
.option('--skills', 'Generate repo-specific skill files from detected communities')
|
||||
.option('--no-git', 'Index a folder without requiring a .git directory')
|
||||
.option('--skip-git', 'Index a folder without requiring a .git directory')
|
||||
.option('-v, --verbose', 'Enable verbose ingestion warnings (default: false)')
|
||||
.addHelpText('after', '\nEnvironment variables:\n GITNEXUS_NO_GITIGNORE=1 Skip .gitignore parsing (still reads .gitnexusignore)')
|
||||
.action(createLazyAction(() => import('./analyze.js'), 'analyzeCommand'));
|
||||
|
||||
@@ -0,0 +1,38 @@
|
||||
import { describe, it, expect } from 'vitest';
|
||||
import { execSync } from 'child_process';
|
||||
import path from 'path';
|
||||
import os from 'os';
|
||||
import fs from 'fs';
|
||||
|
||||
describe('--skip-git CLI flag', () => {
|
||||
it('Commander maps --skip-git to options.skipGit (not --no-git inversion)', () => {
|
||||
// Verify the CLI defines --skip-git, not --no-git
|
||||
const helpOutput = execSync('node dist/cli/index.js analyze --help', {
|
||||
cwd: path.resolve(__dirname, '../..'),
|
||||
encoding: 'utf8',
|
||||
timeout: 10000,
|
||||
});
|
||||
|
||||
expect(helpOutput).toContain('--skip-git');
|
||||
expect(helpOutput).not.toContain('--no-git');
|
||||
});
|
||||
|
||||
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;');
|
||||
|
||||
try {
|
||||
execSync(`node dist/cli/index.js analyze "${tmpDir}"`, {
|
||||
cwd: path.resolve(__dirname, '../..'),
|
||||
encoding: 'utf8',
|
||||
timeout: 10000,
|
||||
});
|
||||
// Should not reach here
|
||||
expect.unreachable('Should have exited with non-zero');
|
||||
} catch (err: any) {
|
||||
expect(err.stdout || err.stderr || '').toContain('--skip-git');
|
||||
} finally {
|
||||
fs.rmSync(tmpDir, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user