Skip to content

Commit c822c89

Browse files
committed
WIP mcp tool improvements
1 parent 7fedace commit c822c89

File tree

15 files changed

+446
-180
lines changed

15 files changed

+446
-180
lines changed
Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
{
2+
"sessions": [
3+
{
4+
"expectedContentPatterns": [
5+
"sarif_list_rules",
6+
"sarif_compare_alerts"
7+
]
8+
}
9+
]
10+
}
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
{
2+
"sessions": [],
3+
"parameters": {
4+
"sarifPathA": "nonexistent/path/to/results.sarif"
5+
}
6+
}

extensions/vscode/vitest.config.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ export default defineConfig({
66
globals: true,
77
environment: 'node',
88
include: ['test/**/*.{test,spec}.{js,ts}'],
9-
exclude: ['test/suite/**'],
9+
exclude: ['test/suite/**', 'dist/**', 'node_modules/**'],
1010
watch: false,
1111
testTimeout: 10000,
1212
alias: {

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

Lines changed: 79 additions & 82 deletions
Large diffs are not rendered by default.

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: 5 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -5,13 +5,13 @@
55
import { McpServer } from '@modelcontextprotocol/sdk/server/mcp.js';
66
import { z } from 'zod';
77
import { CLIExecutionResult, executeCodeQLCommand, executeQLTCommand } from './cli-executor';
8-
import { resolveDatabasePath } from './database-resolver';
8+
import { readDatabaseMetadata, resolveDatabasePath } from './database-resolver';
99
import { logger } from '../utils/logger';
1010
import { getOrCreateLogDirectory } from './log-directory-manager';
1111
import { resolveQueryPath } from './query-resolver';
1212
import { cacheDatabaseAnalyzeResults, processQueryRunResults } from './result-processor';
1313
import { getUserWorkspaceDir, packageRootDir } from '../utils/package-paths';
14-
import { existsSync, mkdirSync, readFileSync, realpathSync, rmSync, writeFileSync } from 'fs';
14+
import { existsSync, mkdirSync, realpathSync, rmSync, writeFileSync } from 'fs';
1515
import { delimiter, dirname, isAbsolute, join, resolve } from 'path';
1616
import * as yaml from 'js-yaml';
1717
import { createProjectTempDir } from '../utils/temp-dir';
@@ -430,15 +430,9 @@ export function registerCLITool(server: McpServer, definition: CLIToolDefinition
430430
// Auto-resolve --source-location-prefix from codeql-database.yml
431431
// (only when not explicitly provided)
432432
if (!options['source-location-prefix']) {
433-
const dbYmlPath = join(dbPath, 'codeql-database.yml');
434-
try {
435-
const dbYml = readFileSync(dbYmlPath, 'utf8');
436-
const dbMeta = yaml.load(dbYml) as Record<string, unknown> | undefined;
437-
if (dbMeta?.sourceLocationPrefix && typeof dbMeta.sourceLocationPrefix === 'string') {
438-
options['source-location-prefix'] = dbMeta.sourceLocationPrefix;
439-
}
440-
} catch {
441-
// codeql-database.yml missing or unparseable — skip prefix resolution
433+
const dbMeta = readDatabaseMetadata(dbPath);
434+
if (dbMeta.sourceLocationPrefix) {
435+
options['source-location-prefix'] = dbMeta.sourceLocationPrefix;
442436
}
443437
}
444438
}

server/src/lib/database-resolver.ts

Lines changed: 71 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
*/
77

88
import { basename, join } from 'path';
9-
import { existsSync, readdirSync, statSync } from 'fs';
9+
import { existsSync, readdirSync, readFileSync, statSync } from 'fs';
1010
import { logger } from '../utils/logger';
1111

1212
/**
@@ -70,3 +70,73 @@ export function resolveDatabasePath(dbPath: string): string {
7070
databasePathCache.set(dbPath, dbPath);
7171
return dbPath;
7272
}
73+
74+
// ---------------------------------------------------------------------------
75+
// Database metadata
76+
// ---------------------------------------------------------------------------
77+
78+
/**
79+
* Metadata parsed from `codeql-database.yml`.
80+
*/
81+
export interface DatabaseMetadata {
82+
cliVersion?: string;
83+
creationTime?: string;
84+
language?: string;
85+
sourceLocationPrefix?: string;
86+
}
87+
88+
/**
89+
* Read and parse `codeql-database.yml` from a CodeQL database directory.
90+
*
91+
* Uses simple line-based parsing (no YAML dependency) to extract common
92+
* metadata fields. Returns an empty object if the file is missing or
93+
* unparseable. Handles both `.yml` and `.yaml` extensions.
94+
*
95+
* This is the single authoritative function for reading database metadata.
96+
* All code that needs `primaryLanguage`, `sourceLocationPrefix`, etc.
97+
* should use this function instead of ad-hoc YAML parsing.
98+
*/
99+
export function readDatabaseMetadata(databasePath: string): DatabaseMetadata {
100+
for (const filename of ['codeql-database.yml', 'codeql-database.yaml']) {
101+
try {
102+
const content = readFileSync(join(databasePath, filename), 'utf8');
103+
return parseDatabaseYmlContent(content);
104+
} catch { /* file not found, try next */ }
105+
}
106+
return {};
107+
}
108+
109+
/**
110+
* Parse the text content of a `codeql-database.yml` file.
111+
*
112+
* Exported for testing — callers should prefer `readDatabaseMetadata()`.
113+
*/
114+
export function parseDatabaseYmlContent(content: string): DatabaseMetadata {
115+
const metadata: DatabaseMetadata = {};
116+
117+
for (const line of content.split('\n')) {
118+
const trimmed = line.trim();
119+
const colonIdx = trimmed.indexOf(':');
120+
if (colonIdx === -1) continue;
121+
122+
const key = trimmed.substring(0, colonIdx).trim();
123+
const value = trimmed.substring(colonIdx + 1).trim().replace(/^["']|["']$/g, '');
124+
125+
switch (key) {
126+
case 'primaryLanguage':
127+
metadata.language = value;
128+
break;
129+
case 'sourceLocationPrefix':
130+
metadata.sourceLocationPrefix = value;
131+
break;
132+
case 'cliVersion':
133+
metadata.cliVersion = value;
134+
break;
135+
case 'creationTime':
136+
metadata.creationTime = value;
137+
break;
138+
}
139+
}
140+
141+
return metadata;
142+
}

server/src/lib/result-processor.ts

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -10,9 +10,10 @@ import { basename, dirname } from 'path';
1010
import { mkdirSync, readFileSync } from 'fs';
1111
import { createHash } from 'crypto';
1212
import { CLIExecutionResult, executeCodeQLCommand, getActualCodeqlVersion } from './cli-executor';
13+
import { readDatabaseMetadata } from './database-resolver';
1314
import { evaluateQueryResults, extractQueryMetadata, QueryEvaluationResult } from './query-results-evaluator';
1415
import { resolveQueryPath } from './query-resolver';
15-
import { decomposeSarifByRule } from './sarif-utils';
16+
import { decomposeSarifByRule, getRuleDisplayName } from './sarif-utils';
1617
import { sessionDataManager } from './session-data-manager';
1718

1819
/**
@@ -218,7 +219,7 @@ export async function processQueryRunResults(
218219
const resultContent = readFileSync(outputFilePath, 'utf8');
219220
const codeqlVersion = getActualCodeqlVersion();
220221
const dbPath = (params.database as string) || '';
221-
const lang = (queryLanguage as string) || 'unknown';
222+
const lang = (queryLanguage as string) || (dbPath ? (readDatabaseMetadata(dbPath).language ?? 'unknown') : 'unknown');
222223
const extPreds: Record<string, string> = {};
223224
if (params.sourceFiles) extPreds.sourceFiles = params.sourceFiles as string;
224225
if (params.sourceFunction) extPreds.sourceFunction = params.sourceFunction as string;
@@ -237,14 +238,17 @@ export async function processQueryRunResults(
237238
// Compute result count from content for SARIF formats
238239
let resultCount: number | null = null;
239240
let ruleId: string | null = null;
241+
let sarifQueryName: string | null = null;
240242
if (outputFormat.includes('sarif')) {
241243
try {
242244
const sarif = JSON.parse(resultContent);
243245
resultCount = (sarif?.runs?.[0]?.results as unknown[] | undefined)?.length ?? 0;
244-
// Extract ruleId from SARIF — single-query runs have exactly one rule
246+
// Extract ruleId and query name from SARIF — single-query runs have exactly one rule
245247
const rules = sarif?.runs?.[0]?.tool?.driver?.rules;
246248
if (Array.isArray(rules) && rules.length > 0) {
247-
ruleId = (rules[0] as { id?: string }).id ?? null;
249+
const rule = rules[0] as import('../types/sarif').SarifRule;
250+
ruleId = rule.id ?? null;
251+
sarifQueryName = getRuleDisplayName(rule);
248252
}
249253
} catch { /* non-SARIF content — leave count/ruleId null */ }
250254
}
@@ -258,7 +262,7 @@ export async function processQueryRunResults(
258262
interpretedPath: outputFilePath,
259263
language: lang,
260264
outputFormat,
261-
queryName: (queryName as string) || basename(queryPath, '.ql'),
265+
queryName: sarifQueryName || (queryName as string) || basename(queryPath, '.ql'),
262266
queryPath,
263267
resultContent,
264268
resultCount,
@@ -371,8 +375,8 @@ export function cacheDatabaseAnalyzeResults(
371375
resultCount = runs?.[0]?.results?.length ?? 0;
372376
} catch { /* non-SARIF content */ }
373377

374-
// Resolve language from database metadata if possible
375-
const language = (params.language as string) || 'unknown';
378+
// Resolve language from database metadata
379+
const language = (params.language as string) || (readDatabaseMetadata(dbPath).language ?? 'unknown');
376380

377381
const cacheKey = computeQueryCacheKey({
378382
codeqlVersion,
@@ -406,6 +410,8 @@ export function cacheDatabaseAnalyzeResults(
406410
for (const [ruleId, ruleSarif] of decomposed) {
407411
const ruleResults = ruleSarif.runs[0]?.results ?? [];
408412
const ruleContent = JSON.stringify(ruleSarif, null, 2);
413+
const ruleDef = ruleSarif.runs[0]?.tool.driver.rules?.[0];
414+
const ruleDisplayName = ruleDef ? getRuleDisplayName(ruleDef) : ruleId;
409415
const ruleCacheKey = computeQueryCacheKey({
410416
codeqlVersion,
411417
databasePath: dbPath,
@@ -420,7 +426,7 @@ export function cacheDatabaseAnalyzeResults(
420426
interpretedPath: outputPath,
421427
language,
422428
outputFormat: format,
423-
queryName: ruleId,
429+
queryName: ruleDisplayName,
424430
queryPath: queries || 'database-analyze',
425431
resultContent: ruleContent,
426432
resultCount: ruleResults.length,

server/src/lib/sarif-utils.ts

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,21 @@ export interface SarifDiffResult {
8585
unchangedRules: SarifRuleSummary[];
8686
}
8787

88+
// ---------------------------------------------------------------------------
89+
// SARIF rule helpers
90+
// ---------------------------------------------------------------------------
91+
92+
/**
93+
* Get the human-readable display name for a SARIF rule.
94+
*
95+
* Priority: `shortDescription.text` → `name` → `id` (fallback).
96+
* This is the single authoritative function for deriving a rule's
97+
* display name from its SARIF definition.
98+
*/
99+
export function getRuleDisplayName(rule: SarifRule): string {
100+
return rule.shortDescription?.text ?? rule.name ?? rule.id;
101+
}
102+
88103
// ---------------------------------------------------------------------------
89104
// Location extraction helpers
90105
// ---------------------------------------------------------------------------
@@ -445,7 +460,7 @@ export function sarifRuleToMarkdown(sarif: SarifDocument, ruleId: string): strin
445460
lines.push(`## ${ruleId}`);
446461
lines.push('');
447462
if (rule) {
448-
const name = rule.shortDescription?.text ?? rule.name ?? ruleId;
463+
const name = getRuleDisplayName(rule);
449464
lines.push(`**Name**: ${name}`);
450465

451466
const props = rule.properties as Record<string, unknown> | undefined;
@@ -549,7 +564,7 @@ export function listSarifRules(sarif: SarifDocument): SarifRuleSummary[] {
549564
const props = rule.properties as Record<string, unknown> | undefined;
550565
summaries.push({
551566
kind: props?.kind as string | undefined,
552-
name: rule.shortDescription?.text ?? rule.name,
567+
name: getRuleDisplayName(rule),
553568
precision: props?.precision as string | undefined,
554569
resultCount: countByRule.get(rule.id) ?? 0,
555570
ruleId: rule.id,

server/src/tools/cache-tools.ts

Lines changed: 7 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -227,29 +227,17 @@ function registerQueryResultsCacheCompareTool(server: McpServer): void {
227227
}
228228

229229
const comparison = Array.from(byDatabase.entries()).map(([db, dbEntries]) => {
230-
// For each entry, prefer the stored resultCount; fall back to
231-
// parsing the cached SARIF content to extract the result count.
232-
let totalResultCount = 0;
233-
for (const e of dbEntries) {
234-
if (e.resultCount != null) {
235-
totalResultCount += e.resultCount;
236-
} else if (e.outputFormat.includes('sarif')) {
237-
// Attempt to derive result count from cached SARIF content
238-
try {
239-
const content = store.getCacheContent(e.cacheKey);
240-
if (content) {
241-
const sarif = JSON.parse(content);
242-
const count = (sarif?.runs?.[0]?.results as unknown[] | undefined)?.length ?? 0;
243-
totalResultCount += count;
244-
}
245-
} catch { /* malformed SARIF or missing content */ }
246-
}
247-
}
230+
// Use the latest entry's result count (entries are sorted by createdAt DESC).
231+
// When multiple cache entries exist for the same ruleId on the same database
232+
// (e.g. one from query_run and another from database_analyze decomposition),
233+
// the latest entry is the most representative.
234+
const latestEntry = dbEntries[0];
235+
const resultCount = latestEntry.resultCount ?? 0;
248236
return {
249237
database: db,
250238
languages: [...new Set(dbEntries.map(e => e.language))],
251239
formats: [...new Set(dbEntries.map(e => e.outputFormat))],
252-
totalResultCount,
240+
resultCount,
253241
cachedRuns: dbEntries.length,
254242
latestCachedAt: dbEntries[0].createdAt,
255243
};

0 commit comments

Comments
 (0)