Skip to content

Commit 2ca9dd1

Browse files
mars167mars167
committed
fix: address Copilot PR #22 review comments
- Extract shared helpers (isCLIError, buildRepoMapAttachment, resolveWikiDir, inferLangFromFile, filterWorkspaceRowsByLang) into sharedHelpers.ts to eliminate duplication between queryHandlers.ts and queryFilesHandlers.ts - Add path traversal protection in resolveWikiDir: reject paths that escape repoRoot - Fix wildcard SQL prefilter: convert glob patterns to SQL LIKE patterns (globToSqlLike) instead of treating raw glob as substring - Return explicit error for workspace mode in query-files since queryManifestWorkspace queries by symbol, not file name - Remove unused import (resolveLangs) from queryFilesHandlers.ts - Convert Commander.js string options to numbers in queryFilesCommand.ts (limit, maxCandidates, repoMapFiles, repoMapSymbols) - Improve tests: add proper assertions for wildcard, language filtering, empty pattern (via Zod schema), and invalid mode (via Zod schema) - Remove unused variable in empty pattern test
1 parent cb0878d commit 2ca9dd1

6 files changed

Lines changed: 181 additions & 176 deletions

File tree

‎package-lock.json‎

Lines changed: 2 additions & 8 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎src/cli/commands/queryFilesCommand.ts‎

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,5 +15,16 @@ export const queryFilesCommand = new Command('query-files')
1515
.option('--repo-map-symbols <n>', 'Max repo map symbols per file', '5')
1616
.option('--wiki <dir>', 'Wiki directory (default: docs/wiki or wiki)', '')
1717
.action(async (pattern, options) => {
18-
await executeHandler('query-files', { pattern, ...options });
18+
const limit = parseInt(options.limit, 10);
19+
const maxCandidates = parseInt(options.maxCandidates, 10);
20+
const repoMapFiles = parseInt(options.repoMapFiles, 10);
21+
const repoMapSymbols = parseInt(options.repoMapSymbols, 10);
22+
await executeHandler('query-files', {
23+
pattern,
24+
...options,
25+
limit,
26+
maxCandidates,
27+
repoMapFiles,
28+
repoMapSymbols,
29+
});
1930
});

‎src/cli/handlers/queryFilesHandlers.ts‎

Lines changed: 41 additions & 80 deletions
Original file line numberDiff line numberDiff line change
@@ -1,85 +1,55 @@
11
import path from 'path';
2-
import fs from 'fs-extra';
32
import { inferWorkspaceRoot, resolveGitRoot } from '../../core/git';
43
import { defaultDbDir, openTablesByLang, type IndexLang } from '../../core/lancedb';
5-
import { queryManifestWorkspace } from '../../core/workspace';
64
import { inferSymbolSearchMode, type SymbolSearchMode } from '../../core/symbolSearch';
75
import { createLogger } from '../../core/log';
8-
import { resolveLangs } from '../../core/indexCheck';
9-
import { generateRepoMap, type FileRank } from '../../core/repoMap';
106
import type { CLIResult, CLIError } from '../types';
117
import { success, error } from '../types';
128
import { resolveRepoContext, validateIndex, resolveLanguages, type RepoContext } from '../helpers';
139
import type { SearchFilesInput } from '../schemas/queryFilesSchemas';
14-
15-
function isCLIError(value: unknown): value is CLIError {
16-
return typeof value === 'object' && value !== null && 'ok' in value && (value as any).ok === false;
17-
}
18-
19-
async function buildRepoMapAttachment(
20-
repoRoot: string,
21-
options: { wiki: string; repoMapFiles: number; repoMapSymbols: number }
22-
): Promise<{ enabled: boolean; wikiDir: string; files: FileRank[] } | { enabled: boolean; skippedReason: string }> {
23-
try {
24-
const wikiDir = resolveWikiDir(repoRoot, options.wiki);
25-
const files = await generateRepoMap({
26-
repoRoot,
27-
maxFiles: options.repoMapFiles,
28-
maxSymbolsPerFile: options.repoMapSymbols,
29-
wikiDir,
30-
});
31-
return { enabled: true, wikiDir, files };
32-
} catch (e: any) {
33-
return { enabled: false, skippedReason: String(e?.message ?? e) };
34-
}
35-
}
36-
37-
function resolveWikiDir(repoRoot: string, wikiOpt: string): string {
38-
const w = String(wikiOpt ?? '').trim();
39-
if (w) return path.resolve(repoRoot, w);
40-
const candidates = [path.join(repoRoot, 'docs', 'wiki'), path.join(repoRoot, 'wiki')];
41-
for (const c of candidates) {
42-
if (fs.existsSync(c)) return c;
43-
}
44-
return '';
45-
}
46-
47-
function inferLangFromFile(file: string): IndexLang {
48-
const f = String(file);
49-
if (f.endsWith('.md') || f.endsWith('.mdx')) return 'markdown';
50-
if (f.endsWith('.yml') || f.endsWith('.yaml')) return 'yaml';
51-
if (f.endsWith('.java')) return 'java';
52-
if (f.endsWith('.c') || f.endsWith('.h')) return 'c';
53-
if (f.endsWith('.go')) return 'go';
54-
if (f.endsWith('.py')) return 'python';
55-
if (f.endsWith('.rs')) return 'rust';
56-
return 'ts';
57-
}
58-
59-
function filterWorkspaceRowsByLang(rows: any[], langSel: string): any[] {
60-
const sel = String(langSel ?? 'auto');
61-
if (sel === 'auto' || sel === 'all') return rows;
62-
const target = sel as IndexLang;
63-
return rows.filter(r => inferLangFromFile(String((r as any).file ?? '')) === target);
64-
}
10+
import {
11+
isCLIError,
12+
buildRepoMapAttachment,
13+
filterWorkspaceRowsByLang,
14+
} from './sharedHelpers';
6515

6616
function escapeQuotes(s: string): string {
6717
return s.replace(/'/g, "''");
6818
}
6919

20+
/**
21+
* Convert a glob pattern to a SQL LIKE pattern.
22+
* Escapes SQL LIKE wildcards (% and _), then converts glob * to % and ? to _.
23+
*/
24+
function globToSqlLike(pattern: string): string {
25+
let like = pattern.replace(/\\/g, '\\\\').replace(/([%_])/g, '\\$1');
26+
like = like.replace(/\*/g, '%').replace(/\?/g, '_');
27+
return like;
28+
}
29+
7030
function buildFileWhere(pattern: string, mode: SymbolSearchMode, caseInsensitive: boolean): string | null {
71-
const safe = escapeQuotes(pattern);
72-
if (!safe) return null;
31+
if (!pattern) return null;
7332
const likeOp = caseInsensitive ? 'ILIKE' : 'LIKE';
7433

7534
if (mode === 'prefix') {
35+
const safe = escapeQuotes(pattern);
36+
if (!safe) return null;
7637
return `file ${likeOp} '${safe}%'`;
7738
}
7839

79-
if (mode === 'substring' || mode === 'wildcard') {
40+
if (mode === 'substring') {
41+
const safe = escapeQuotes(pattern);
42+
if (!safe) return null;
8043
return `file ${likeOp} '%${safe}%'`;
8144
}
8245

46+
if (mode === 'wildcard') {
47+
const likePattern = globToSqlLike(pattern);
48+
const safeLike = escapeQuotes(likePattern);
49+
if (!safeLike) return null;
50+
return `file ${likeOp} '${safeLike}'`;
51+
}
52+
8353
// For regex and fuzzy, we'll handle them in memory after fetching
8454
return null;
8555
}
@@ -190,36 +160,28 @@ export async function handleSearchFiles(input: SearchFilesInput): Promise<CLIRes
190160
const repoRoot = await resolveGitRoot(path.resolve(input.path));
191161
const mode = inferSymbolSearchMode(input.pattern, input.mode);
192162

163+
// Workspace mode is not supported for query-files because
164+
// queryManifestWorkspace queries by symbol, not by file name.
193165
if (inferWorkspaceRoot(repoRoot)) {
194-
const res = await queryManifestWorkspace({
195-
manifestRepoRoot: repoRoot,
196-
keyword: input.pattern,
197-
limit: input.maxCandidates,
198-
});
199-
const filteredByLang = filterWorkspaceRowsByLang(res.rows, input.lang);
200-
const rows = filterAndRankFileRows(
201-
filteredByLang,
202-
input.pattern,
203-
mode,
204-
input.caseInsensitive,
205-
input.limit
206-
);
166+
const durationMs = Date.now() - startedAt;
207167
log.info('query_files', {
208-
ok: true,
168+
ok: false,
209169
repoRoot,
210170
workspace: true,
211171
mode,
212172
case_insensitive: input.caseInsensitive,
213173
limit: input.limit,
214174
max_candidates: input.maxCandidates,
215-
candidates: res.rows.length,
216-
rows: rows.length,
217-
duration_ms: Date.now() - startedAt,
175+
candidates: 0,
176+
rows: 0,
177+
duration_ms: durationMs,
178+
error: 'workspace_mode_not_supported_for_query_files',
179+
});
180+
return error('workspace_mode_not_supported_for_query_files', {
181+
message:
182+
'query-files does not currently support workspace manifests. ' +
183+
'Please run this command from a non-workspace repository root or disable workspace mode.',
218184
});
219-
const repoMap = input.withRepoMap
220-
? { enabled: false, skippedReason: 'workspace_mode_not_supported' }
221-
: undefined;
222-
return success({ ...res, rows, ...(repoMap ? { repo_map: repoMap } : {}) });
223185
}
224186

225187
const ctxOrError = await resolveRepoContext(input.path);
@@ -256,7 +218,6 @@ export async function handleSearchFiles(input: SearchFilesInput): Promise<CLIRes
256218
const t = byLang[lang as IndexLang];
257219
if (!t) continue;
258220

259-
// Fetch candidates based on mode
260221
// For regex/fuzzy, we fetch all and filter in memory
261222
const shouldFetchAll = mode === 'regex' || mode === 'fuzzy';
262223
const rows = shouldFetchAll

‎src/cli/handlers/queryHandlers.ts‎

Lines changed: 5 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -1,66 +1,17 @@
11
import path from 'path';
2-
import fs from 'fs-extra';
32
import { inferWorkspaceRoot, resolveGitRoot } from '../../core/git';
43
import { defaultDbDir, openTablesByLang, type IndexLang } from '../../core/lancedb';
54
import { queryManifestWorkspace } from '../../core/workspace';
65
import { buildCoarseWhere, filterAndRankSymbolRows, inferSymbolSearchMode, pickCoarseToken, type SymbolSearchMode } from '../../core/symbolSearch';
76
import { createLogger } from '../../core/log';
8-
import { checkIndex, resolveLangs } from '../../core/indexCheck';
9-
import { generateRepoMap, type FileRank } from '../../core/repoMap';
107
import type { CLIResult, CLIError } from '../types';
118
import { success, error } from '../types';
129
import { resolveRepoContext, validateIndex, resolveLanguages, type RepoContext } from '../helpers';
13-
14-
function isCLIError(value: unknown): value is CLIError {
15-
return typeof value === 'object' && value !== null && 'ok' in value && (value as any).ok === false;
16-
}
17-
18-
async function buildRepoMapAttachment(
19-
repoRoot: string,
20-
options: { wiki: string; repoMapFiles: number; repoMapSymbols: number }
21-
): Promise<{ enabled: boolean; wikiDir: string; files: FileRank[] } | { enabled: boolean; skippedReason: string }> {
22-
try {
23-
const wikiDir = resolveWikiDir(repoRoot, options.wiki);
24-
const files = await generateRepoMap({
25-
repoRoot,
26-
maxFiles: options.repoMapFiles,
27-
maxSymbolsPerFile: options.repoMapSymbols,
28-
wikiDir,
29-
});
30-
return { enabled: true, wikiDir, files };
31-
} catch (e: any) {
32-
return { enabled: false, skippedReason: String(e?.message ?? e) };
33-
}
34-
}
35-
36-
function resolveWikiDir(repoRoot: string, wikiOpt: string): string {
37-
const w = String(wikiOpt ?? '').trim();
38-
if (w) return path.resolve(repoRoot, w);
39-
const candidates = [path.join(repoRoot, 'docs', 'wiki'), path.join(repoRoot, 'wiki')];
40-
for (const c of candidates) {
41-
if (fs.existsSync(c)) return c;
42-
}
43-
return '';
44-
}
45-
46-
function inferLangFromFile(file: string): IndexLang {
47-
const f = String(file);
48-
if (f.endsWith('.md') || f.endsWith('.mdx')) return 'markdown';
49-
if (f.endsWith('.yml') || f.endsWith('.yaml')) return 'yaml';
50-
if (f.endsWith('.java')) return 'java';
51-
if (f.endsWith('.c') || f.endsWith('.h')) return 'c';
52-
if (f.endsWith('.go')) return 'go';
53-
if (f.endsWith('.py')) return 'python';
54-
if (f.endsWith('.rs')) return 'rust';
55-
return 'ts';
56-
}
57-
58-
function filterWorkspaceRowsByLang(rows: any[], langSel: string): any[] {
59-
const sel = String(langSel ?? 'auto');
60-
if (sel === 'auto' || sel === 'all') return rows;
61-
const target = sel as IndexLang;
62-
return rows.filter(r => inferLangFromFile(String((r as any).file ?? '')) === target);
63-
}
10+
import {
11+
isCLIError,
12+
buildRepoMapAttachment,
13+
filterWorkspaceRowsByLang,
14+
} from './sharedHelpers';
6415

6516
export async function handleSearchSymbols(input: {
6617
keyword: string;

‎src/cli/handlers/sharedHelpers.ts‎

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
import path from 'path';
2+
import fs from 'fs-extra';
3+
import type { IndexLang } from '../../core/lancedb';
4+
import { generateRepoMap, type FileRank } from '../../core/repoMap';
5+
import type { CLIError } from '../types';
6+
7+
export function isCLIError(value: unknown): value is CLIError {
8+
return typeof value === 'object' && value !== null && 'ok' in value && (value as any).ok === false;
9+
}
10+
11+
export async function buildRepoMapAttachment(
12+
repoRoot: string,
13+
options: { wiki: string; repoMapFiles: number; repoMapSymbols: number }
14+
): Promise<{ enabled: boolean; wikiDir: string; files: FileRank[] } | { enabled: boolean; skippedReason: string }> {
15+
try {
16+
const wikiDir = resolveWikiDir(repoRoot, options.wiki);
17+
const files = await generateRepoMap({
18+
repoRoot,
19+
maxFiles: options.repoMapFiles,
20+
maxSymbolsPerFile: options.repoMapSymbols,
21+
wikiDir,
22+
});
23+
return { enabled: true, wikiDir, files };
24+
} catch (e: any) {
25+
return { enabled: false, skippedReason: String(e?.message ?? e) };
26+
}
27+
}
28+
29+
/**
30+
* Resolve wiki directory, ensuring the resolved path stays within repoRoot
31+
* to prevent path traversal attacks.
32+
*/
33+
export function resolveWikiDir(repoRoot: string, wikiOpt: string): string {
34+
const w = String(wikiOpt ?? '').trim();
35+
if (w) {
36+
const resolved = path.resolve(repoRoot, w);
37+
// Prevent path traversal outside repoRoot
38+
if (!resolved.startsWith(repoRoot + path.sep) && resolved !== repoRoot) {
39+
return '';
40+
}
41+
return resolved;
42+
}
43+
const candidates = [path.join(repoRoot, 'docs', 'wiki'), path.join(repoRoot, 'wiki')];
44+
for (const c of candidates) {
45+
if (fs.existsSync(c)) return c;
46+
}
47+
return '';
48+
}
49+
50+
export function inferLangFromFile(file: string): IndexLang {
51+
const f = String(file);
52+
if (f.endsWith('.md') || f.endsWith('.mdx')) return 'markdown';
53+
if (f.endsWith('.yml') || f.endsWith('.yaml')) return 'yaml';
54+
if (f.endsWith('.java')) return 'java';
55+
if (f.endsWith('.c') || f.endsWith('.h')) return 'c';
56+
if (f.endsWith('.go')) return 'go';
57+
if (f.endsWith('.py')) return 'python';
58+
if (f.endsWith('.rs')) return 'rust';
59+
return 'ts';
60+
}
61+
62+
export function filterWorkspaceRowsByLang(rows: any[], langSel: string): any[] {
63+
const sel = String(langSel ?? 'auto');
64+
if (sel === 'auto' || sel === 'all') return rows;
65+
const target = sel as IndexLang;
66+
return rows.filter(r => inferLangFromFile(String((r as any).file ?? '')) === target);
67+
}

0 commit comments

Comments
 (0)