Skip to content

Commit 7fd8da3

Browse files
Copilotdata-douser
andauthored
Fix bugs 1-5 and implement design improvements 2, 3, 5
Bug 1: Improve bqrs_interpret -t parameter docs, add database param Bug 2: Fix query_results_cache_compare to derive resultCount from SARIF Bug 3: Include category field in annotation_search FTS matching Bug 4: Add findingId as primary lookup for audit_add_notes Bug 5: Change bqrs_info files param to single file string Design 2: Serialize concurrent database_analyze on same database Design 3: Already implemented (cacheKey/queryName in cache_lookup) Design 5: Auto-enable annotation tools in VSIX with new setting Agent-Logs-Url: https://github.com/advanced-security/codeql-development-mcp-server/sessions/0d52c27d-21be-49ea-bf90-c8fb6f50a3da Co-authored-by: data-douser <70299490+data-douser@users.noreply.github.com>
1 parent ae1eaf7 commit 7fd8da3

16 files changed

Lines changed: 440 additions & 45 deletions

extensions/vscode/package.json

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,11 @@
9090
"default": true,
9191
"markdownDescription": "Copy CodeQL databases from the `GitHub.vscode-codeql` extension storage into a managed directory, removing query-server lock files so the MCP server CLI can operate without contention. Disable to use databases in-place (may fail when the CodeQL query server is running)."
9292
},
93+
"codeql-mcp.enableAnnotationTools": {
94+
"type": "boolean",
95+
"default": true,
96+
"markdownDescription": "Enable annotation, audit, and query results caching tools. When enabled, the MCP server registers `annotation_*`, `audit_*`, and `query_results_cache_*` tools. Disable to reduce the tool surface if these capabilities are not needed."
97+
},
9398
"codeql-mcp.serverArgs": {
9499
"type": "array",
95100
"items": {

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

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -147,7 +147,17 @@ export class EnvironmentBuilder extends DisposableObject {
147147
queryDirs.push(...userQueryDirs);
148148
env.CODEQL_QUERY_RUN_RESULTS_DIRS = queryDirs.join(delimiter);
149149

150-
// User-configured additional environment variables
150+
// Annotation, audit, and cache tools — enabled by default (Design 5).
151+
// The setting controls ENABLE_ANNOTATION_TOOLS and defaults
152+
// MONITORING_STORAGE_LOCATION to the scratch directory so tools work
153+
// out-of-the-box without manual env var configuration.
154+
const enableAnnotations = config.get<boolean>('enableAnnotationTools', true);
155+
env.ENABLE_ANNOTATION_TOOLS = enableAnnotations ? 'true' : 'false';
156+
if (enableAnnotations && env.CODEQL_MCP_SCRATCH_DIR) {
157+
env.MONITORING_STORAGE_LOCATION = env.CODEQL_MCP_SCRATCH_DIR;
158+
}
159+
160+
// User-configured additional environment variables (overrides above defaults)
151161
const additionalEnv = config.get<Record<string, string>>('additionalEnv', {});
152162
for (const [key, value] of Object.entries(additionalEnv)) {
153163
env[key] = value;

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

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -272,4 +272,70 @@ describe('EnvironmentBuilder', () => {
272272
it('should be disposable', () => {
273273
expect(() => builder.dispose()).not.toThrow();
274274
});
275+
276+
it('should set ENABLE_ANNOTATION_TOOLS=true by default', async () => {
277+
const env = await builder.build();
278+
expect(env.ENABLE_ANNOTATION_TOOLS).toBe('true');
279+
});
280+
281+
it('should set ENABLE_ANNOTATION_TOOLS=false when setting is disabled', async () => {
282+
const vscode = await import('vscode');
283+
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;
296+
297+
builder.invalidate();
298+
const env = await builder.build();
299+
expect(env.ENABLE_ANNOTATION_TOOLS).toBe('false');
300+
301+
vscode.workspace.getConfiguration = originalGetConfig;
302+
});
303+
304+
it('should set MONITORING_STORAGE_LOCATION to scratch dir when annotations enabled with workspace', async () => {
305+
const vscode = await import('vscode');
306+
const origFolders = vscode.workspace.workspaceFolders;
307+
(vscode.workspace.workspaceFolders as any) = [
308+
{ uri: { fsPath: '/mock/workspace' }, name: 'ws', index: 0 },
309+
];
310+
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;
316+
});
317+
318+
it('should allow additionalEnv to override ENABLE_ANNOTATION_TOOLS', async () => {
319+
const vscode = await import('vscode');
320+
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;
333+
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;
340+
});
275341
});

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

Lines changed: 98 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -183584,8 +183584,9 @@ 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)");
183587+
conditions.push("(id IN (SELECT rowid FROM annotations_fts WHERE annotations_fts MATCH $search) OR category = $search_cat)");
183588183588
params.$search = filter.search;
183589+
params.$search_cat = filter.search;
183589183590
}
183590183591
let sql = "SELECT * FROM annotations";
183591183592
if (conditions.length > 0) {
@@ -184496,6 +184497,14 @@ Interpreted output saved to: ${outputFilePath}`;
184496184497
queryPath
184497184498
});
184498184499
const store = sessionDataManager.getStore();
184500+
let resultCount = null;
184501+
if (outputFormat.includes("sarif")) {
184502+
try {
184503+
const sarif = JSON.parse(resultContent);
184504+
resultCount = sarif?.runs?.[0]?.results?.length ?? null;
184505+
} catch {
184506+
}
184507+
}
184499184508
store.putCacheEntry({
184500184509
bqrsPath,
184501184510
cacheKey: cacheKey2,
@@ -184507,7 +184516,8 @@ Interpreted output saved to: ${outputFilePath}`;
184507184516
outputFormat,
184508184517
queryName: queryName || basename4(queryPath, ".ql"),
184509184518
queryPath,
184510-
resultContent
184519+
resultContent,
184520+
resultCount
184511184521
});
184512184522
enhancedOutput += `
184513184523
Results cached with key: ${cacheKey2}`;
@@ -187168,6 +187178,21 @@ var safeDump = renamed("safeDump", "dump");
187168187178

187169187179
// src/lib/cli-tool-registry.ts
187170187180
init_temp_dir();
187181+
var databaseLocks = /* @__PURE__ */ new Map();
187182+
function acquireDatabaseLock(dbPath) {
187183+
const key = dbPath;
187184+
const previous = databaseLocks.get(key) ?? Promise.resolve();
187185+
let release;
187186+
const gate = new Promise((resolve15) => {
187187+
release = resolve15;
187188+
});
187189+
databaseLocks.set(key, gate);
187190+
return {
187191+
ready: previous.then(() => {
187192+
}),
187193+
release
187194+
};
187195+
}
187171187196
var defaultCLIResultProcessor = (result, _params) => {
187172187197
if (!result.success) {
187173187198
return `Command failed (exit code ${result.exitCode || "unknown"}):
@@ -187407,6 +187432,13 @@ function registerCLITool(server, definition) {
187407187432
}
187408187433
break;
187409187434
}
187435+
case "codeql_bqrs_interpret":
187436+
if (options.database) {
187437+
const dbPath = resolveDatabasePath(options.database);
187438+
options["source-archive"] = join10(dbPath, "src");
187439+
delete options.database;
187440+
}
187441+
break;
187410187442
case "codeql_query_compile":
187411187443
case "codeql_resolve_metadata":
187412187444
if (query) {
@@ -187501,7 +187533,16 @@ function registerCLITool(server, definition) {
187501187533
if (name === "codeql_test_run") {
187502187534
options["keep-databases"] = true;
187503187535
}
187504-
result = await executeCodeQLCommand(subcommand, options, [...positionalArgs, ...userAdditionalArgs], cwd);
187536+
let dbLock;
187537+
if (name === "codeql_database_analyze" && positionalArgs.length > 0) {
187538+
dbLock = acquireDatabaseLock(positionalArgs[0]);
187539+
await dbLock.ready;
187540+
}
187541+
try {
187542+
result = await executeCodeQLCommand(subcommand, options, [...positionalArgs, ...userAdditionalArgs], cwd);
187543+
} finally {
187544+
dbLock?.release();
187545+
}
187505187546
} else if (command === "qlt") {
187506187547
result = await executeQLTCommand(subcommand, options, [...positionalArgs, ...userAdditionalArgs]);
187507187548
} else {
@@ -187654,7 +187695,7 @@ var codeqlBqrsInfoTool = {
187654187695
command: "codeql",
187655187696
subcommand: "bqrs info",
187656187697
inputSchema: {
187657-
files: external_exports.array(external_exports.string()).describe("BQRS file(s) to examine"),
187698+
file: external_exports.string().describe("BQRS file to examine"),
187658187699
format: external_exports.enum(["text", "json"]).optional().describe("Output format: text (default) or json. Use json for machine-readable output and pagination offset computation."),
187659187700
"paginate-rows": external_exports.number().optional().describe("Compute byte offsets for pagination at intervals of this many rows. Use with --format=json. Offsets can be passed to codeql_bqrs_decode --start-at."),
187660187701
"paginate-result-set": external_exports.string().optional().describe("Compute pagination offsets only for this result set name"),
@@ -187679,7 +187720,8 @@ var codeqlBqrsInterpretTool = {
187679187720
file: external_exports.string().describe("The BQRS file to interpret"),
187680187721
format: external_exports.enum(["csv", "sarif-latest", "sarifv2.1.0", "graphtext", "dgml", "dot"]).describe("Output format: csv (comma-separated), sarif-latest/sarifv2.1.0 (SARIF), graphtext/dgml/dot (graph formats, only for @kind graph queries)"),
187681187722
output: createCodeQLSchemas.output(),
187682-
t: external_exports.array(external_exports.string()).describe('Query metadata key=value pairs. At least "kind" and "id" must be specified (e.g., ["kind=graph", "id=js/print-ast"])'),
187723+
database: external_exports.string().optional().describe("Path to the CodeQL database, used to resolve source archive context for SARIF interpretation (provides file contents and snippets)"),
187724+
t: external_exports.array(external_exports.string()).describe('Query metadata key=value pairs in KEY=VALUE format. At least "kind" and "id" must be specified. Example: ["kind=problem", "id=js/sql-injection"]. Common keys: kind (problem|path-problem|graph|metric|diagnostic), id (query identifier like js/xss)'),
187683187725
"max-paths": external_exports.number().optional().describe("Maximum number of paths to produce for each alert with paths (default: 4)"),
187684187726
"sarif-add-file-contents": external_exports.boolean().optional().describe("[SARIF only] Include full file contents for all files referenced in results"),
187685187727
"sarif-add-snippets": external_exports.boolean().optional().describe("[SARIF only] Include code snippets for each location with context"),
@@ -193667,16 +193709,35 @@ function registerAuditListFindingsTool(server) {
193667193709
function registerAuditAddNotesTool(server) {
193668193710
server.tool(
193669193711
"audit_add_notes",
193670-
"Append notes to an existing audit finding. The notes are appended to the annotation content.",
193712+
"Append notes to an existing audit finding. Identify the finding by findingId (preferred) or by owner+repo+sourceLocation+line.",
193671193713
{
193672-
owner: external_exports.string().describe("Repository owner."),
193673-
repo: external_exports.string().describe("Repository name."),
193674-
sourceLocation: external_exports.string().describe("File path of the finding."),
193675-
line: external_exports.number().int().min(1).describe("Line number of the finding (integer >= 1)."),
193714+
findingId: external_exports.number().int().positive().optional().describe("Annotation ID of the finding (returned by audit_store_findings and audit_list_findings). Preferred lookup method."),
193715+
owner: external_exports.string().optional().describe("Repository owner (required when findingId is not provided)."),
193716+
repo: external_exports.string().optional().describe("Repository name (required when findingId is not provided)."),
193717+
sourceLocation: external_exports.string().optional().describe("File path of the finding (required when findingId is not provided)."),
193718+
line: external_exports.number().int().min(1).optional().describe("Line number of the finding (required when findingId is not provided)."),
193676193719
notes: external_exports.string().describe("Notes to append.")
193677193720
},
193678-
async ({ owner, repo, sourceLocation, line, notes }) => {
193721+
async ({ findingId, owner, repo, sourceLocation, line, notes }) => {
193679193722
const store = sessionDataManager.getStore();
193723+
if (findingId) {
193724+
const annotation2 = store.getAnnotation(findingId);
193725+
if (!annotation2 || annotation2.category !== AUDIT_CATEGORY) {
193726+
return {
193727+
content: [{ type: "text", text: `No audit finding found with id ${findingId}.` }]
193728+
};
193729+
}
193730+
const updatedContent2 = (annotation2.content || "") + "\n" + notes;
193731+
store.updateAnnotation(annotation2.id, { content: updatedContent2.trim() });
193732+
return {
193733+
content: [{ type: "text", text: `Updated notes for finding id ${findingId}.` }]
193734+
};
193735+
}
193736+
if (!owner || !repo || !sourceLocation || !line) {
193737+
return {
193738+
content: [{ type: "text", text: "Either findingId or all of owner+repo+sourceLocation+line must be provided." }]
193739+
};
193740+
}
193680193741
const entityKey = `${repoKey(owner, repo)}:${sourceLocation}:L${line}`;
193681193742
const existing = store.listAnnotations({
193682193743
category: AUDIT_CATEGORY,
@@ -193879,14 +193940,32 @@ function registerQueryResultsCacheCompareTool(server) {
193879193940
if (!byDatabase.has(key)) byDatabase.set(key, []);
193880193941
byDatabase.get(key).push(entry);
193881193942
}
193882-
const comparison = Array.from(byDatabase.entries()).map(([db, dbEntries]) => ({
193883-
database: db,
193884-
languages: [...new Set(dbEntries.map((e) => e.language))],
193885-
formats: [...new Set(dbEntries.map((e) => e.outputFormat))],
193886-
totalResultCount: dbEntries.reduce((sum, e) => sum + (e.resultCount ?? 0), 0),
193887-
cachedRuns: dbEntries.length,
193888-
latestCachedAt: dbEntries[0].createdAt
193889-
}));
193943+
const comparison = Array.from(byDatabase.entries()).map(([db, dbEntries]) => {
193944+
let totalResultCount = 0;
193945+
for (const e of dbEntries) {
193946+
if (e.resultCount != null) {
193947+
totalResultCount += e.resultCount;
193948+
} else {
193949+
try {
193950+
const content = store.getCacheContent(e.cacheKey);
193951+
if (content) {
193952+
const sarif = JSON.parse(content);
193953+
const count = sarif?.runs?.[0]?.results?.length ?? 0;
193954+
totalResultCount += count;
193955+
}
193956+
} catch {
193957+
}
193958+
}
193959+
}
193960+
return {
193961+
database: db,
193962+
languages: [...new Set(dbEntries.map((e) => e.language))],
193963+
formats: [...new Set(dbEntries.map((e) => e.outputFormat))],
193964+
totalResultCount,
193965+
cachedRuns: dbEntries.length,
193966+
latestCachedAt: dbEntries[0].createdAt
193967+
};
193968+
});
193890193969
return {
193891193970
content: [{
193892193971
type: "text",

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: 48 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,32 @@ import { createProjectTempDir } from '../utils/temp-dir';
1818

1919
export type { CLIExecutionResult } from './cli-executor';
2020

21+
/**
22+
* Per-database mutex map — serializes concurrent CLI operations that
23+
* acquire an exclusive lock on a CodeQL database (e.g. database analyze).
24+
*/
25+
const databaseLocks = new Map<string, Promise<unknown>>();
26+
27+
/**
28+
* Acquire a serialized slot for the given database path.
29+
* Returns a release function that must be called when the operation completes.
30+
*/
31+
function acquireDatabaseLock(dbPath: string): { ready: Promise<void>; release: () => void } {
32+
const key = dbPath;
33+
const previous = databaseLocks.get(key) ?? Promise.resolve();
34+
35+
let release!: () => void;
36+
const gate = new Promise<void>(resolve => {
37+
release = resolve;
38+
});
39+
databaseLocks.set(key, gate);
40+
41+
return {
42+
ready: previous.then(() => {}),
43+
release,
44+
};
45+
}
46+
2147
export interface CLIToolDefinition {
2248
name: string;
2349
description: string;
@@ -352,6 +378,15 @@ export function registerCLITool(server: McpServer, definition: CLIToolDefinition
352378
break;
353379
}
354380

381+
case 'codeql_bqrs_interpret':
382+
// Map 'database' to '--source-archive' for codeql bqrs interpret
383+
if (options.database) {
384+
const dbPath = resolveDatabasePath(options.database as string);
385+
options['source-archive'] = join(dbPath, 'src');
386+
delete options.database;
387+
}
388+
break;
389+
355390
case 'codeql_query_compile':
356391
case 'codeql_resolve_metadata':
357392
// Handle query parameter as positional argument for query compilation and metadata tools
@@ -519,7 +554,19 @@ export function registerCLITool(server: McpServer, definition: CLIToolDefinition
519554
options['keep-databases'] = true;
520555
}
521556

522-
result = await executeCodeQLCommand(subcommand, options, [...positionalArgs, ...userAdditionalArgs], cwd);
557+
// Serialize concurrent database_analyze calls to the same database
558+
// to prevent "cache directory is already locked" errors from the CLI.
559+
let dbLock: { ready: Promise<void>; release: () => void } | undefined;
560+
if (name === 'codeql_database_analyze' && positionalArgs.length > 0) {
561+
dbLock = acquireDatabaseLock(positionalArgs[0]);
562+
await dbLock.ready;
563+
}
564+
565+
try {
566+
result = await executeCodeQLCommand(subcommand, options, [...positionalArgs, ...userAdditionalArgs], cwd);
567+
} finally {
568+
dbLock?.release();
569+
}
523570
} else if (command === 'qlt') {
524571
result = await executeQLTCommand(subcommand, options, [...positionalArgs, ...userAdditionalArgs]);
525572
} else {

server/src/lib/result-processor.ts

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -232,6 +232,16 @@ export async function processQueryRunResults(
232232
});
233233

234234
const store = sessionDataManager.getStore();
235+
236+
// Compute result count from content for SARIF formats
237+
let resultCount: number | null = null;
238+
if (outputFormat.includes('sarif')) {
239+
try {
240+
const sarif = JSON.parse(resultContent);
241+
resultCount = (sarif?.runs?.[0]?.results as unknown[] | undefined)?.length ?? null;
242+
} catch { /* non-SARIF content — leave count null */ }
243+
}
244+
235245
store.putCacheEntry({
236246
bqrsPath,
237247
cacheKey,
@@ -244,6 +254,7 @@ export async function processQueryRunResults(
244254
queryName: (queryName as string) || basename(queryPath, '.ql'),
245255
queryPath,
246256
resultContent,
257+
resultCount,
247258
});
248259
enhancedOutput += `\nResults cached with key: ${cacheKey}`;
249260
logger.info(`Cached query results with key: ${cacheKey}`);

0 commit comments

Comments
 (0)