Skip to content

Commit 7854829

Browse files
Copilotdata-douser
andauthored
Address PR review feedback and fix failing integration tests
- Fix databaseLocks memory leak: clean up map entries on release - Fix bqrs_interpret source-archive: prefer src.zip over src/ - Fix cacheDatabaseAnalyzeResults: use resolved database path - Fix annotation category search: add COLLATE NOCASE - Fix resultCount: use ?? 0 instead of ?? null (two locations) - Fix VS Code tests: wrap mock mutations in try/finally - Fix codeql_bqrs_info integration tests: files → file in test runner, monitoring runner, and test-config.json Agent-Logs-Url: https://github.com/advanced-security/codeql-development-mcp-server/sessions/52936c89-89e6-4b1d-8c0e-12dcb9ad2388 Co-authored-by: data-douser <70299490+data-douser@users.noreply.github.com>
1 parent ddd4de4 commit 7854829

9 files changed

Lines changed: 92 additions & 65 deletions

File tree

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
{
22
"toolName": "codeql_bqrs_info",
33
"arguments": {
4-
"files": ["client/integration-tests/primitives/tools/codeql_bqrs_info/json_format/before/results.bqrs"],
4+
"file": "client/integration-tests/primitives/tools/codeql_bqrs_info/json_format/before/results.bqrs",
55
"format": "json"
66
}
77
}

client/src/lib/integration-test-runner.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -931,7 +931,7 @@ export class IntegrationTestRunner {
931931
// Use static BQRS file
932932
const bqrsFile = path.join(staticPath, "src", "ExampleQuery1", "ExampleQuery1.test.bqrs");
933933
if (fs.existsSync(bqrsFile)) {
934-
params.files = [bqrsFile];
934+
params.file = bqrsFile;
935935
} else {
936936
throw new Error(`Static BQRS file not found: ${bqrsFile}`);
937937
}

client/src/lib/monitoring-integration-test-runner.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -361,7 +361,7 @@ export class MonitoringIntegrationTestRunner {
361361
if (fs.existsSync(beforeDir)) {
362362
const bqrsFiles = fs.readdirSync(beforeDir).filter((f) => f.endsWith(".bqrs"));
363363
if (bqrsFiles.length > 0) {
364-
params.bqrs = path.join(beforeDir, bqrsFiles[0]);
364+
params.file = path.join(beforeDir, bqrsFiles[0]);
365365
} else {
366366
throw new Error(`No .bqrs files found in ${beforeDir} for ${toolName}`);
367367
}

extensions/vscode/test/bridge/environment-builder.test.ts

Lines changed: 52 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -281,61 +281,70 @@ describe('EnvironmentBuilder', () => {
281281
it('should set ENABLE_ANNOTATION_TOOLS=false when setting is disabled', async () => {
282282
const vscode = await import('vscode');
283283
const originalGetConfig = vscode.workspace.getConfiguration;
284-
vscode.workspace.getConfiguration = () => ({
285-
get: (_key: string, defaultVal?: any) => {
286-
if (_key === 'enableAnnotationTools') return false;
287-
if (_key === 'additionalDatabaseDirs') return [];
288-
if (_key === 'additionalQueryRunResultsDirs') return [];
289-
if (_key === 'additionalMrvaRunResultsDirs') return [];
290-
return defaultVal;
291-
},
292-
has: () => false,
293-
inspect: () => undefined as any,
294-
update: () => Promise.resolve(),
295-
}) as any;
296284

297-
builder.invalidate();
298-
const env = await builder.build();
299-
expect(env.ENABLE_ANNOTATION_TOOLS).toBe('false');
300-
301-
vscode.workspace.getConfiguration = originalGetConfig;
285+
try {
286+
vscode.workspace.getConfiguration = () => ({
287+
get: (_key: string, defaultVal?: any) => {
288+
if (_key === 'enableAnnotationTools') return false;
289+
if (_key === 'additionalDatabaseDirs') return [];
290+
if (_key === 'additionalQueryRunResultsDirs') return [];
291+
if (_key === 'additionalMrvaRunResultsDirs') return [];
292+
return defaultVal;
293+
},
294+
has: () => false,
295+
inspect: () => undefined as any,
296+
update: () => Promise.resolve(),
297+
}) as any;
298+
299+
builder.invalidate();
300+
const env = await builder.build();
301+
expect(env.ENABLE_ANNOTATION_TOOLS).toBe('false');
302+
} finally {
303+
vscode.workspace.getConfiguration = originalGetConfig;
304+
}
302305
});
303306

304307
it('should set MONITORING_STORAGE_LOCATION to scratch dir when annotations enabled with workspace', async () => {
305308
const vscode = await import('vscode');
306309
const origFolders = vscode.workspace.workspaceFolders;
307-
(vscode.workspace.workspaceFolders as any) = [
308-
{ uri: { fsPath: '/mock/workspace' }, name: 'ws', index: 0 },
309-
];
310310

311-
builder.invalidate();
312-
const env = await builder.build();
313-
expect(env.MONITORING_STORAGE_LOCATION).toBe('/mock/workspace/.codeql/ql-mcp');
314-
315-
(vscode.workspace.workspaceFolders as any) = origFolders;
311+
try {
312+
(vscode.workspace.workspaceFolders as any) = [
313+
{ uri: { fsPath: '/mock/workspace' }, name: 'ws', index: 0 },
314+
];
315+
316+
builder.invalidate();
317+
const env = await builder.build();
318+
expect(env.MONITORING_STORAGE_LOCATION).toBe('/mock/workspace/.codeql/ql-mcp');
319+
} finally {
320+
(vscode.workspace.workspaceFolders as any) = origFolders;
321+
}
316322
});
317323

318324
it('should allow additionalEnv to override ENABLE_ANNOTATION_TOOLS', async () => {
319325
const vscode = await import('vscode');
320326
const originalGetConfig = vscode.workspace.getConfiguration;
321-
vscode.workspace.getConfiguration = () => ({
322-
get: (_key: string, defaultVal?: any) => {
323-
if (_key === 'additionalEnv') return { ENABLE_ANNOTATION_TOOLS: 'false' };
324-
if (_key === 'additionalDatabaseDirs') return [];
325-
if (_key === 'additionalQueryRunResultsDirs') return [];
326-
if (_key === 'additionalMrvaRunResultsDirs') return [];
327-
return defaultVal;
328-
},
329-
has: () => false,
330-
inspect: () => undefined as any,
331-
update: () => Promise.resolve(),
332-
}) as any;
333327

334-
builder.invalidate();
335-
const env = await builder.build();
336-
// additionalEnv comes after the default, so it should override
337-
expect(env.ENABLE_ANNOTATION_TOOLS).toBe('false');
338-
339-
vscode.workspace.getConfiguration = originalGetConfig;
328+
try {
329+
vscode.workspace.getConfiguration = () => ({
330+
get: (_key: string, defaultVal?: any) => {
331+
if (_key === 'additionalEnv') return { ENABLE_ANNOTATION_TOOLS: 'false' };
332+
if (_key === 'additionalDatabaseDirs') return [];
333+
if (_key === 'additionalQueryRunResultsDirs') return [];
334+
if (_key === 'additionalMrvaRunResultsDirs') return [];
335+
return defaultVal;
336+
},
337+
has: () => false,
338+
inspect: () => undefined as any,
339+
update: () => Promise.resolve(),
340+
}) as any;
341+
342+
builder.invalidate();
343+
const env = await builder.build();
344+
// additionalEnv comes after the default, so it should override
345+
expect(env.ENABLE_ANNOTATION_TOOLS).toBe('false');
346+
} finally {
347+
vscode.workspace.getConfiguration = originalGetConfig;
348+
}
340349
});
341350
});

server/dist/codeql-development-mcp-server.js

Lines changed: 16 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -183584,7 +183584,7 @@ var SqliteStore = class _SqliteStore {
183584183584
params.$entity_key_prefix = filter.entityKeyPrefix + "%";
183585183585
}
183586183586
if (filter?.search) {
183587-
conditions.push("(id IN (SELECT rowid FROM annotations_fts WHERE annotations_fts MATCH $search) OR category = $search_cat)");
183587+
conditions.push("(id IN (SELECT rowid FROM annotations_fts WHERE annotations_fts MATCH $search) OR category = $search_cat COLLATE NOCASE)");
183588183588
params.$search = filter.search;
183589183589
params.$search_cat = filter.search;
183590183590
}
@@ -184501,7 +184501,7 @@ Interpreted output saved to: ${outputFilePath}`;
184501184501
if (outputFormat.includes("sarif")) {
184502184502
try {
184503184503
const sarif = JSON.parse(resultContent);
184504-
resultCount = sarif?.runs?.[0]?.results?.length ?? null;
184504+
resultCount = sarif?.runs?.[0]?.results?.length ?? 0;
184505184505
} catch {
184506184506
}
184507184507
}
@@ -184605,7 +184605,7 @@ function cacheDatabaseAnalyzeResults(params, logger2) {
184605184605
let resultCount = null;
184606184606
try {
184607184607
const sarif = JSON.parse(resultContent);
184608-
resultCount = sarif?.runs?.[0]?.results?.length ?? null;
184608+
resultCount = sarif?.runs?.[0]?.results?.length ?? 0;
184609184609
} catch {
184610184610
}
184611184611
const cacheKey2 = computeQueryCacheKey({
@@ -187230,15 +187230,20 @@ var databaseLocks = /* @__PURE__ */ new Map();
187230187230
function acquireDatabaseLock(dbPath) {
187231187231
const key = dbPath;
187232187232
const previous = databaseLocks.get(key) ?? Promise.resolve();
187233-
let release;
187233+
let resolveGate;
187234187234
const gate = new Promise((resolve15) => {
187235-
release = resolve15;
187235+
resolveGate = resolve15;
187236187236
});
187237187237
databaseLocks.set(key, gate);
187238187238
return {
187239187239
ready: previous.then(() => {
187240187240
}),
187241-
release
187241+
release: () => {
187242+
resolveGate();
187243+
if (databaseLocks.get(key) === gate) {
187244+
databaseLocks.delete(key);
187245+
}
187246+
}
187242187247
};
187243187248
}
187244187249
var defaultCLIResultProcessor = (result, _params) => {
@@ -187483,7 +187488,9 @@ function registerCLITool(server, definition) {
187483187488
case "codeql_bqrs_interpret":
187484187489
if (options.database) {
187485187490
const dbPath = resolveDatabasePath(options.database);
187486-
options["source-archive"] = join10(dbPath, "src");
187491+
const srcZipPath = join10(dbPath, "src.zip");
187492+
const srcDirPath = join10(dbPath, "src");
187493+
options["source-archive"] = existsSync6(srcZipPath) ? srcZipPath : srcDirPath;
187487187494
delete options.database;
187488187495
}
187489187496
break;
@@ -187621,7 +187628,8 @@ function registerCLITool(server, definition) {
187621187628
}
187622187629
}
187623187630
if (name === "codeql_database_analyze" && result.success && options.output) {
187624-
cacheDatabaseAnalyzeResults({ ...params, output: options.output, format: options.format }, logger);
187631+
const resolvedDb = typeof params.database === "string" ? resolveDatabasePath(params.database) : params.database;
187632+
cacheDatabaseAnalyzeResults({ ...params, database: resolvedDb, output: options.output, format: options.format }, logger);
187625187633
}
187626187634
const processedResult = resultProcessor(result, params);
187627187635
return {

server/dist/codeql-development-mcp-server.js.map

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

server/src/lib/cli-tool-registry.ts

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -32,15 +32,20 @@ function acquireDatabaseLock(dbPath: string): { ready: Promise<void>; release: (
3232
const key = dbPath;
3333
const previous = databaseLocks.get(key) ?? Promise.resolve();
3434

35-
let release!: () => void;
35+
let resolveGate!: () => void;
3636
const gate = new Promise<void>(resolve => {
37-
release = resolve;
37+
resolveGate = resolve;
3838
});
3939
databaseLocks.set(key, gate);
4040

4141
return {
4242
ready: previous.then(() => {}),
43-
release,
43+
release: () => {
44+
resolveGate();
45+
if (databaseLocks.get(key) === gate) {
46+
databaseLocks.delete(key);
47+
}
48+
},
4449
};
4550
}
4651

@@ -382,7 +387,9 @@ export function registerCLITool(server: McpServer, definition: CLIToolDefinition
382387
// Map 'database' to '--source-archive' for codeql bqrs interpret
383388
if (options.database) {
384389
const dbPath = resolveDatabasePath(options.database as string);
385-
options['source-archive'] = join(dbPath, 'src');
390+
const srcZipPath = join(dbPath, 'src.zip');
391+
const srcDirPath = join(dbPath, 'src');
392+
options['source-archive'] = existsSync(srcZipPath) ? srcZipPath : srcDirPath;
386393
delete options.database;
387394
}
388395
break;
@@ -607,7 +614,10 @@ export function registerCLITool(server: McpServer, definition: CLIToolDefinition
607614

608615
// Post-execution: cache database_analyze results in query results cache
609616
if (name === 'codeql_database_analyze' && result.success && options.output) {
610-
cacheDatabaseAnalyzeResults({ ...params, output: options.output, format: options.format }, logger);
617+
const resolvedDb = typeof params.database === 'string'
618+
? resolveDatabasePath(params.database)
619+
: params.database;
620+
cacheDatabaseAnalyzeResults({ ...params, database: resolvedDb, output: options.output, format: options.format }, logger);
611621
}
612622

613623
// Process the result

server/src/lib/result-processor.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -238,7 +238,7 @@ export async function processQueryRunResults(
238238
if (outputFormat.includes('sarif')) {
239239
try {
240240
const sarif = JSON.parse(resultContent);
241-
resultCount = (sarif?.runs?.[0]?.results as unknown[] | undefined)?.length ?? null;
241+
resultCount = (sarif?.runs?.[0]?.results as unknown[] | undefined)?.length ?? 0;
242242
} catch { /* non-SARIF content — leave count null */ }
243243
}
244244

@@ -358,7 +358,7 @@ export function cacheDatabaseAnalyzeResults(
358358
let resultCount: number | null = null;
359359
try {
360360
const sarif = JSON.parse(resultContent);
361-
resultCount = (sarif?.runs?.[0]?.results as unknown[] | undefined)?.length ?? null;
361+
resultCount = (sarif?.runs?.[0]?.results as unknown[] | undefined)?.length ?? 0;
362362
} catch { /* non-SARIF content */ }
363363

364364
const cacheKey = computeQueryCacheKey({

server/src/lib/sqlite-store.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -459,7 +459,7 @@ export class SqliteStore {
459459
// across content, label, and metadata fields. Also match the category
460460
// column directly (not in FTS) so searches like "performance" find
461461
// annotations whose category is "performance".
462-
conditions.push('(id IN (SELECT rowid FROM annotations_fts WHERE annotations_fts MATCH $search) OR category = $search_cat)');
462+
conditions.push('(id IN (SELECT rowid FROM annotations_fts WHERE annotations_fts MATCH $search) OR category = $search_cat COLLATE NOCASE)');
463463
params.$search = filter.search;
464464
params.$search_cat = filter.search;
465465
}

0 commit comments

Comments
 (0)