Skip to content

Commit a5e5460

Browse files
committed
Address latest PR review feedback
1 parent 6a682d6 commit a5e5460

File tree

8 files changed

+58
-62
lines changed

8 files changed

+58
-62
lines changed

package-lock.json

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

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

Lines changed: 12 additions & 9 deletions
Large diffs are not rendered by default.

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

Lines changed: 2 additions & 2 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: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -612,11 +612,19 @@ export function registerCLITool(server: McpServer, definition: CLIToolDefinition
612612
// when the same database is referenced through relative paths, symlinks,
613613
// or different casing.
614614
let dbLock: { ready: Promise<void>; release: () => void } | undefined;
615-
if (name === 'codeql_database_analyze' && positionalArgs.length > 0) {
616-
let lockKey = resolve(positionalArgs[0]);
617-
try { lockKey = realpathSync(lockKey); } catch { /* use resolved path if realpath fails */ }
618-
dbLock = acquireDatabaseLock(lockKey);
619-
await dbLock.ready;
615+
if (name === 'codeql_database_analyze') {
616+
// Use the resolved database path from params (set before positionalArgs
617+
// construction) rather than positionalArgs[0], which may include
618+
// _positional values prepended before the database path.
619+
const resolvedDb = typeof params.database === 'string'
620+
? resolveDatabasePath(params.database)
621+
: (positionalArgs.length > 0 ? positionalArgs[0] : undefined);
622+
if (resolvedDb) {
623+
let lockKey = resolve(resolvedDb);
624+
try { lockKey = realpathSync(lockKey); } catch { /* use resolved path if realpath fails */ }
625+
dbLock = acquireDatabaseLock(lockKey);
626+
await dbLock.ready;
627+
}
620628
}
621629

622630
try {

server/src/prompts/compare-overlapping-alerts.prompt.md

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,14 +19,20 @@ This workflow supports:
1919

2020
### Step 1: Discover available rules
2121

22-
Use #sarif_list_rules on each SARIF source to understand what rules and result counts are present:
22+
Use #sarif_list_rules on each SARIF source to understand what rules and result counts are present. When comparing within a single file, call it once:
23+
24+
```
25+
sarif_list_rules(sarifPath="{{sarifPathA}}")
26+
```
27+
28+
When comparing two different SARIF files, call it on both:
2329

2430
```
2531
sarif_list_rules(sarifPath="{{sarifPathA}}")
2632
sarif_list_rules(sarifPath="{{sarifPathB}}")
2733
```
2834

29-
If comparing within a single file, call it once. The response includes `ruleId`, `resultCount`, `kind`, `precision`, `severity`, and `tags` for each rule.
35+
The response includes `ruleId`, `resultCount`, `kind`, `precision`, `severity`, and `tags` for each rule.
3036

3137
### Step 2: Choose a comparison strategy
3238

server/src/resources/server-prompts.md

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -39,12 +39,12 @@ This resource provides a complete reference of the prompts exposed by the CodeQL
3939
### Documentation and Quality
4040

4141
- **`document_codeql_query`** — Generates standardized markdown documentation as a sibling `.md` file to a query. Requires `queryPath` and `language`.
42-
- **`run_query_and_summarize_false_positives`** — Runs a CodeQL query on a database and groups results into false-positive categories by root cause. Uses #query_results_cache_lookup, #sarif_list_rules, #sarif_extract_rule, #sarif_rule_to_markdown, and #read_database_source for structured analysis.
43-
- **`sarif_rank_false_positives`** / **`sarif_rank_true_positives`** — Analyze SARIF output to assess query precision by ranking results as likely true or false positives. Uses #sarif_list_rules, #sarif_extract_rule, #sarif_rule_to_markdown, #read_database_source, #sarif_compare_alerts, and #sarif_diff_runs for context gathering.
42+
- **`run_query_and_summarize_false_positives`** — Runs a CodeQL query on a database and groups results into false-positive categories by root cause. Uses `query_results_cache_lookup`, `sarif_list_rules`, `sarif_extract_rule`, `sarif_rule_to_markdown`, and `read_database_source` for structured analysis.
43+
- **`sarif_rank_false_positives`** / **`sarif_rank_true_positives`** — Analyze SARIF output to assess query precision by ranking results as likely true or false positives. Uses `sarif_list_rules`, `sarif_extract_rule`, `sarif_rule_to_markdown`, `read_database_source`, `sarif_compare_alerts`, and `sarif_diff_runs` for context gathering.
4444

4545
### Alert Analysis and Comparison
4646

47-
- **`compare_overlapping_alerts`** — Compares CodeQL SARIF alerts across any combination of SARIF files, analysis runs, CodeQL databases, or query packs. Classifies findings as redundant, complementary, false overlap, behavioral regression, or new coverage. Uses #sarif_list_rules, #sarif_extract_rule, #sarif_rule_to_markdown, #sarif_compare_alerts, #sarif_diff_runs, and #read_database_source tools. Requires `sarifPathA`; optionally accepts `sarifPathB` for cross-file comparison, `ruleIdA`/`ruleIdB` to narrow to specific rules, and `databasePath` for source code context.
47+
- **`compare_overlapping_alerts`** — Compares CodeQL SARIF alerts across any combination of SARIF files, analysis runs, CodeQL databases, or query packs. Classifies findings as redundant, complementary, false overlap, behavioral regression, or new coverage. Uses `sarif_list_rules`, `sarif_extract_rule`, `sarif_rule_to_markdown`, `sarif_compare_alerts`, `sarif_diff_runs`, and `read_database_source` tools. Requires `sarifPathA`; optionally accepts `sarifPathB` for cross-file comparison, `ruleIdA`/`ruleIdB` to narrow to specific rules, and `databasePath` for source code context.
4848

4949
### Workshop Creation
5050

server/test/src/lib/result-processor.test.ts

Lines changed: 19 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,11 @@
22
* Tests for result-processor — query result caching behavior.
33
*/
44

5-
import { existsSync, rmSync } from 'fs';
5+
import { existsSync, rmSync, writeFileSync } from 'fs';
6+
import { join } from 'path';
67
import { afterEach, beforeEach, describe, expect, it } from 'vitest';
78
import { computeQueryCacheKey } from '../../../src/lib/result-processor';
9+
import { readDatabaseMetadata } from '../../../src/lib/database-resolver';
810
import { createProjectTempDir } from '../../../src/utils/temp-dir';
911

1012
// We can't easily test processQueryRunResults end-to-end because it
@@ -66,12 +68,10 @@ describe('computeQueryCacheKey', () => {
6668
});
6769

6870
/**
69-
* Test the resolveDatabaseLanguage function.
70-
* This is an internal function, so we test it indirectly by importing it
71-
* via a re-export or by testing through the cache integration path.
72-
* For direct testing, we use the module internals.
71+
* Test language resolution via readDatabaseMetadata (the shared function
72+
* used by result-processor for resolving language from database metadata).
7373
*/
74-
describe('resolveDatabaseLanguage', () => {
74+
describe('database language resolution for caching', () => {
7575
let testDir: string;
7676

7777
beforeEach(() => {
@@ -84,29 +84,20 @@ describe('resolveDatabaseLanguage', () => {
8484
}
8585
});
8686

87-
// Since resolveDatabaseLanguage is not exported, we test the behavior
88-
// through importing the module and testing the function's effect on
89-
// caching. For now, validate the YAML parsing logic pattern.
90-
it('should extract language from codeql-database.yml content', () => {
91-
const yamlContent = `---
92-
sourceLocationPrefix: /src
93-
primaryLanguage: javascript
94-
creationMetadata:
95-
cliVersion: 2.25.1
96-
`;
97-
// Simulate the regex the function uses
98-
const match = yamlContent.match(/^primaryLanguage\s*:\s*(\S+)/m);
99-
expect(match).not.toBeNull();
100-
expect(match![1]).toBe('javascript');
87+
it('should resolve language from codeql-database.yml', () => {
88+
writeFileSync(join(testDir, 'codeql-database.yml'), 'primaryLanguage: javascript\n');
89+
const metadata = readDatabaseMetadata(testDir);
90+
expect(metadata.language).toBe('javascript');
10191
});
10292

103-
it('should handle YAML without primaryLanguage', () => {
104-
const yamlContent = `---
105-
sourceLocationPrefix: /src
106-
creationMetadata:
107-
cliVersion: 2.25.1
108-
`;
109-
const match = yamlContent.match(/^primaryLanguage\s*:\s*(\S+)/m);
110-
expect(match).toBeNull();
93+
it('should return undefined language when file is missing', () => {
94+
const metadata = readDatabaseMetadata(testDir);
95+
expect(metadata.language).toBeUndefined();
96+
});
97+
98+
it('should resolve language from codeql-database.yaml', () => {
99+
writeFileSync(join(testDir, 'codeql-database.yaml'), 'primaryLanguage: python\n');
100+
const metadata = readDatabaseMetadata(testDir);
101+
expect(metadata.language).toBe('python');
111102
});
112103
});

server/test/src/tools/cache-tools.test.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -508,9 +508,8 @@ describe('Cache Tools', () => {
508508

509509
it('should use latest entry resultCount for comparison', async () => {
510510
const store = sessionDataManager.getStore();
511-
// Cache entry without resultCount (simulates old cache entries)
512511
store.putCacheEntry({
513-
cacheKey: 'compare-no-count',
512+
cacheKey: 'compare-latest-count',
514513
queryName: 'UI5Clickjacking',
515514
queryPath: '/ui5.ql',
516515
databasePath: '/db1',

0 commit comments

Comments
 (0)