Skip to content

Commit d927751

Browse files
committed
Address PR #234 review feedback
1 parent f19ee41 commit d927751

File tree

5 files changed

+57
-11
lines changed

5 files changed

+57
-11
lines changed

client/integration-tests/primitives/tools/codeql_query_compile/compile_query/test-config.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
{
22
"toolName": "codeql_query_compile",
33
"arguments": {
4-
"query": "server/ql/javascript/examples/src/ExampleQuery1/ExampleQuery1.ql"
4+
"query": "test_query.ql"
55
},
66
"assertions": {
77
"responseContains": ["DIL file:", ".dil"]

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

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -193922,9 +193922,13 @@ function registerCLITool(server, definition) {
193922193922
}
193923193923
}
193924193924
let compileLogDir;
193925-
if (name === "codeql_query_compile" && options["dump-dil"] !== false) {
193926-
compileLogDir = getOrCreateLogDirectory(customLogDir);
193927-
logger.info(`Using log directory for ${name}: ${compileLogDir}`);
193925+
if (name === "codeql_query_compile") {
193926+
const pendingArgs = Array.isArray(options.additionalArgs) ? options.additionalArgs : [];
193927+
const effectiveDumpDilDisabled = options["dump-dil"] === false || pendingArgs.includes("--no-dump-dil");
193928+
if (!effectiveDumpDilDisabled) {
193929+
compileLogDir = getOrCreateLogDirectory(customLogDir);
193930+
logger.info(`Using log directory for ${name}: ${compileLogDir}`);
193931+
}
193928193932
}
193929193933
const rawAdditionalArgs = Array.isArray(options.additionalArgs) ? options.additionalArgs : [];
193930193934
delete options.additionalArgs;

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 & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -556,11 +556,20 @@ export function registerCLITool(server: McpServer, definition: CLIToolDefinition
556556
}
557557
}
558558

559-
// Set up log directory for compile runs to persist DIL output
559+
// Set up log directory for compile runs to persist DIL output.
560+
// Compute an effective "dump-dil enabled" flag that accounts for
561+
// both `dump-dil: false` and `--no-dump-dil` in `additionalArgs`.
560562
let compileLogDir: string | undefined;
561-
if (name === 'codeql_query_compile' && options['dump-dil'] !== false) {
562-
compileLogDir = getOrCreateLogDirectory(customLogDir as string | undefined);
563-
logger.info(`Using log directory for ${name}: ${compileLogDir}`);
563+
if (name === 'codeql_query_compile') {
564+
const pendingArgs = Array.isArray(options.additionalArgs)
565+
? options.additionalArgs as string[]
566+
: [];
567+
const effectiveDumpDilDisabled = options['dump-dil'] === false
568+
|| pendingArgs.includes('--no-dump-dil');
569+
if (!effectiveDumpDilDisabled) {
570+
compileLogDir = getOrCreateLogDirectory(customLogDir as string | undefined);
571+
logger.info(`Using log directory for ${name}: ${compileLogDir}`);
572+
}
564573
}
565574

566575
// Extract additionalArgs from options so they are passed as raw CLI

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

Lines changed: 34 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44

55
import { describe, it, expect, vi, beforeEach } from 'vitest';
66
import { existsSync, mkdirSync, readFileSync, rmSync, writeFileSync } from 'fs';
7-
import { join } from 'path';
7+
import { dirname, join } from 'path';
88
import { McpServer } from '@modelcontextprotocol/sdk/server/mcp';
99
import { z } from 'zod';
1010
import {
@@ -705,6 +705,10 @@ describe('registerCLITool handler behavior', () => {
705705
expect(existsSync(dilFilePath)).toBe(true);
706706
const fileContent = readFileSync(dilFilePath, 'utf8');
707707
expect(fileContent).toBe(dilContent);
708+
709+
// Clean up generated .dil file and its containing log directory
710+
const dilDir = dirname(dilFilePath);
711+
rmSync(dilDir, { recursive: true, force: true });
708712
});
709713

710714
it('should not save DIL file when dump-dil is explicitly false for codeql_query_compile', async () => {
@@ -736,6 +740,35 @@ describe('registerCLITool handler behavior', () => {
736740
expect(result.content[0].text).not.toContain('DIL file:');
737741
});
738742

743+
it('should not save DIL file when --no-dump-dil is in additionalArgs for codeql_query_compile', async () => {
744+
const definition: CLIToolDefinition = {
745+
name: 'codeql_query_compile',
746+
description: 'Compile query',
747+
command: 'codeql',
748+
subcommand: 'query compile',
749+
inputSchema: {
750+
query: z.string(),
751+
'dump-dil': z.boolean().optional(),
752+
additionalArgs: z.array(z.string()).optional()
753+
}
754+
};
755+
756+
registerCLITool(mockServer, definition);
757+
758+
const handler = (mockServer.registerTool as ReturnType<typeof vi.fn>).mock.calls[0][2];
759+
760+
executeCodeQLCommand.mockResolvedValueOnce({
761+
stdout: 'Compilation successful',
762+
stderr: '',
763+
success: true
764+
});
765+
766+
const result = await handler({ query: '/path/to/MyQuery.ql', additionalArgs: ['--no-dump-dil'] });
767+
768+
// Response should NOT contain DIL file path since --no-dump-dil disables DIL
769+
expect(result.content[0].text).not.toContain('DIL file:');
770+
});
771+
739772
it('should pass format to CLI for codeql_bqrs_interpret', async () => {
740773
const definition: CLIToolDefinition = {
741774
name: 'codeql_bqrs_interpret',

0 commit comments

Comments
 (0)