Skip to content

Commit 8bd3dff

Browse files
data-douserCopilot
data-douser
and
Copilot
authored
Fixes for codeql_query_compile MCP tool and dump-dil argument handling (#243)
* Fixes for codeql_query_compile dump-dil Applies fixes and improves tests for codeql_query_compile MCP tool related to --dump-dil versus --no-dump-dil arg handling. * Address PR #243 review feedback * Update server/test/src/lib/cli-tool-registry.test.ts Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: Nathan Randall <70299490+data-douser@users.noreply.github.com> --------- Signed-off-by: Nathan Randall <70299490+data-douser@users.noreply.github.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
1 parent a3a140b commit 8bd3dff

File tree

4 files changed

+319
-24
lines changed

4 files changed

+319
-24
lines changed

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

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -193967,13 +193967,24 @@ function registerCLITool(server, definition) {
193967193967
}
193968193968
}
193969193969
let effectiveDumpDilEnabled = false;
193970+
let rawAdditionalArgs = Array.isArray(options.additionalArgs) ? options.additionalArgs : [];
193971+
delete options.additionalArgs;
193970193972
if (name === "codeql_query_compile") {
193971-
const pendingArgs = Array.isArray(options.additionalArgs) ? options.additionalArgs : [];
193972-
const effectiveDumpDilDisabled = options["dump-dil"] === false || pendingArgs.includes("--no-dump-dil");
193973-
effectiveDumpDilEnabled = !effectiveDumpDilDisabled;
193973+
const lastDumpDilFlag = [...rawAdditionalArgs].reverse().find(
193974+
(arg) => arg === "--dump-dil" || arg === "--no-dump-dil"
193975+
);
193976+
if (lastDumpDilFlag === "--dump-dil") {
193977+
options["dump-dil"] = true;
193978+
} else if (lastDumpDilFlag === "--no-dump-dil") {
193979+
options["dump-dil"] = false;
193980+
}
193981+
if (lastDumpDilFlag !== void 0) {
193982+
rawAdditionalArgs = rawAdditionalArgs.filter(
193983+
(arg) => arg !== "--dump-dil" && arg !== "--no-dump-dil"
193984+
);
193985+
}
193986+
effectiveDumpDilEnabled = options["dump-dil"] !== false;
193974193987
}
193975-
const rawAdditionalArgs = Array.isArray(options.additionalArgs) ? options.additionalArgs : [];
193976-
delete options.additionalArgs;
193977193988
const managedFlagNames = /* @__PURE__ */ new Set([
193978193989
"evaluator-log",
193979193990
"logdir",

‎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: 20 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -561,23 +561,34 @@ export function registerCLITool(server: McpServer, definition: CLIToolDefinition
561561
// `additionalArgs`. The log directory is created lazily post-success
562562
// to avoid leaving empty directories behind on compilation failures.
563563
let effectiveDumpDilEnabled = false;
564-
if (name === 'codeql_query_compile') {
565-
const pendingArgs = Array.isArray(options.additionalArgs)
566-
? options.additionalArgs as string[]
567-
: [];
568-
const effectiveDumpDilDisabled = options['dump-dil'] === false
569-
|| pendingArgs.includes('--no-dump-dil');
570-
effectiveDumpDilEnabled = !effectiveDumpDilDisabled;
571-
}
572564

573565
// Extract additionalArgs from options so they are passed as raw CLI
574566
// arguments instead of being transformed into --additionalArgs=value
575567
// by buildCodeQLArgs.
576-
const rawAdditionalArgs = Array.isArray(options.additionalArgs)
568+
let rawAdditionalArgs = Array.isArray(options.additionalArgs)
577569
? options.additionalArgs as string[]
578570
: [];
579571
delete options.additionalArgs;
580572

573+
if (name === 'codeql_query_compile') {
574+
// Last --dump-dil / --no-dump-dil in additionalArgs overrides the named param
575+
const lastDumpDilFlag = [...rawAdditionalArgs].reverse().find(
576+
(arg) => arg === '--dump-dil' || arg === '--no-dump-dil',
577+
);
578+
if (lastDumpDilFlag === '--dump-dil') {
579+
options['dump-dil'] = true;
580+
} else if (lastDumpDilFlag === '--no-dump-dil') {
581+
options['dump-dil'] = false;
582+
}
583+
if (lastDumpDilFlag !== undefined) {
584+
rawAdditionalArgs = rawAdditionalArgs.filter(
585+
(arg) => arg !== '--dump-dil' && arg !== '--no-dump-dil',
586+
);
587+
}
588+
589+
effectiveDumpDilEnabled = options['dump-dil'] !== false;
590+
}
591+
581592
// For tools with post-execution processing (query run, test run,
582593
// database analyze), certain CLI flags are set internally and their
583594
// values are read back after execution (e.g. --evaluator-log for log

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

Lines changed: 281 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -625,10 +625,11 @@ describe('registerCLITool handler behavior', () => {
625625
const options = call[1] as Record<string, unknown>;
626626
const positionalArgs = call[2] as string[];
627627

628-
// --dump-dil should not be in options when --no-dump-dil is in additionalArgs
629-
expect(options['dump-dil']).toBeUndefined();
630-
// --no-dump-dil should be passed through in the positional/additional args
631-
expect(positionalArgs).toContain('--no-dump-dil');
628+
// --no-dump-dil should be normalized into options['dump-dil'] = false
629+
expect(options['dump-dil']).toBe(false);
630+
// --no-dump-dil should be filtered from raw CLI args (not duplicated)
631+
expect(positionalArgs).not.toContain('--no-dump-dil');
632+
expect(positionalArgs).not.toContain('--dump-dil');
632633
});
633634

634635
it('should not inject --dump-dil when --dump-dil is already in additionalArgs for codeql_query_compile', async () => {
@@ -660,10 +661,11 @@ describe('registerCLITool handler behavior', () => {
660661
const options = call[1] as Record<string, unknown>;
661662
const positionalArgs = call[2] as string[];
662663

663-
// --dump-dil should not be duplicated in options when already in additionalArgs
664-
expect(options['dump-dil']).toBeUndefined();
665-
// The explicit --dump-dil from additionalArgs should be passed through
666-
expect(positionalArgs).toContain('--dump-dil');
664+
// --dump-dil should be normalized into options['dump-dil'] = true
665+
expect(options['dump-dil']).toBe(true);
666+
// --dump-dil should be filtered from raw CLI args (not duplicated)
667+
expect(positionalArgs).not.toContain('--dump-dil');
668+
expect(positionalArgs).not.toContain('--no-dump-dil');
667669
});
668670

669671
it('should save DIL output to a .dil file for codeql_query_compile', async () => {
@@ -831,6 +833,277 @@ describe('registerCLITool handler behavior', () => {
831833
expect(result.content[0].text).not.toContain('DIL file:');
832834
});
833835

836+
it('should let --dump-dil in additionalArgs override dump-dil: false for codeql_query_compile', async () => {
837+
const definition: CLIToolDefinition = {
838+
name: 'codeql_query_compile',
839+
description: 'Compile query',
840+
command: 'codeql',
841+
subcommand: 'query compile',
842+
inputSchema: {
843+
query: z.string(),
844+
'dump-dil': z.boolean().optional(),
845+
additionalArgs: z.array(z.string()).optional()
846+
}
847+
};
848+
849+
registerCLITool(mockServer, definition);
850+
851+
const handler = (mockServer.registerTool as ReturnType<typeof vi.fn>).mock.calls[0][2];
852+
853+
const dilContent = 'DIL:\n predicate#abc123\n SCAN table';
854+
executeCodeQLCommand.mockResolvedValueOnce({
855+
stdout: dilContent,
856+
stderr: '',
857+
success: true
858+
});
859+
860+
const codeqlQueryLogDir = createTestTempDir('codeql-query-log-dir');
861+
const originalCodeqlQueryLogDir = process.env.CODEQL_QUERY_LOG_DIR;
862+
process.env.CODEQL_QUERY_LOG_DIR = codeqlQueryLogDir;
863+
864+
try {
865+
const result = await handler({
866+
query: '/path/to/MyQuery.ql',
867+
'dump-dil': false,
868+
additionalArgs: ['--dump-dil']
869+
});
870+
871+
const call = executeCodeQLCommand.mock.calls[0];
872+
const options = call[1] as Record<string, unknown>;
873+
const positionalArgs = call[2] as string[];
874+
875+
// additionalArgs --dump-dil should override the named dump-dil: false
876+
expect(options['dump-dil']).toBe(true);
877+
878+
// --dump-dil should be filtered from raw CLI args (not duplicated)
879+
expect(positionalArgs).not.toContain('--dump-dil');
880+
expect(positionalArgs).not.toContain('--no-dump-dil');
881+
882+
// DIL output should be saved since dump-dil is effectively enabled
883+
expect(result.content[0].text).toContain('DIL file:');
884+
expect(result.content[0].text).toContain('MyQuery.dil');
885+
} finally {
886+
if (originalCodeqlQueryLogDir === undefined) {
887+
delete process.env.CODEQL_QUERY_LOG_DIR;
888+
} else {
889+
process.env.CODEQL_QUERY_LOG_DIR = originalCodeqlQueryLogDir;
890+
}
891+
892+
rmSync(codeqlQueryLogDir, { recursive: true, force: true });
893+
}
894+
});
895+
896+
it('should let --no-dump-dil in additionalArgs override dump-dil: true for codeql_query_compile', async () => {
897+
const definition: CLIToolDefinition = {
898+
name: 'codeql_query_compile',
899+
description: 'Compile query',
900+
command: 'codeql',
901+
subcommand: 'query compile',
902+
inputSchema: {
903+
query: z.string(),
904+
'dump-dil': z.boolean().optional(),
905+
additionalArgs: z.array(z.string()).optional()
906+
}
907+
};
908+
909+
registerCLITool(mockServer, definition);
910+
911+
const handler = (mockServer.registerTool as ReturnType<typeof vi.fn>).mock.calls[0][2];
912+
913+
executeCodeQLCommand.mockResolvedValueOnce({
914+
stdout: 'Compilation successful',
915+
stderr: '',
916+
success: true
917+
});
918+
919+
const result = await handler({
920+
query: '/path/to/MyQuery.ql',
921+
'dump-dil': true,
922+
additionalArgs: ['--no-dump-dil']
923+
});
924+
925+
const call = executeCodeQLCommand.mock.calls[0];
926+
const options = call[1] as Record<string, unknown>;
927+
const positionalArgs = call[2] as string[];
928+
929+
// additionalArgs --no-dump-dil should override the named dump-dil: true
930+
expect(options['dump-dil']).toBe(false);
931+
932+
// --no-dump-dil should be filtered from raw CLI args (not duplicated)
933+
expect(positionalArgs).not.toContain('--dump-dil');
934+
expect(positionalArgs).not.toContain('--no-dump-dil');
935+
936+
// DIL output should NOT be saved since dump-dil is effectively disabled
937+
expect(result.content[0].text).not.toContain('DIL file:');
938+
});
939+
940+
it('should filter --dump-dil from additionalArgs when passed without named param for codeql_query_compile', async () => {
941+
const definition: CLIToolDefinition = {
942+
name: 'codeql_query_compile',
943+
description: 'Compile query',
944+
command: 'codeql',
945+
subcommand: 'query compile',
946+
inputSchema: {
947+
query: z.string(),
948+
'dump-dil': z.boolean().optional(),
949+
additionalArgs: z.array(z.string()).optional()
950+
}
951+
};
952+
953+
registerCLITool(mockServer, definition);
954+
955+
const handler = (mockServer.registerTool as ReturnType<typeof vi.fn>).mock.calls[0][2];
956+
957+
executeCodeQLCommand.mockResolvedValueOnce({
958+
stdout: '',
959+
stderr: '',
960+
success: true
961+
});
962+
963+
await handler({ query: '/path/to/query.ql', additionalArgs: ['--dump-dil'] });
964+
965+
const call = executeCodeQLCommand.mock.calls[0];
966+
const options = call[1] as Record<string, unknown>;
967+
const positionalArgs = call[2] as string[];
968+
969+
// --dump-dil should be normalized into the options object
970+
expect(options['dump-dil']).toBe(true);
971+
972+
// --dump-dil should be filtered from raw CLI args to avoid duplication
973+
expect(positionalArgs).not.toContain('--dump-dil');
974+
});
975+
976+
it('should filter --no-dump-dil from additionalArgs when passed without named param for codeql_query_compile', async () => {
977+
const definition: CLIToolDefinition = {
978+
name: 'codeql_query_compile',
979+
description: 'Compile query',
980+
command: 'codeql',
981+
subcommand: 'query compile',
982+
inputSchema: {
983+
query: z.string(),
984+
'dump-dil': z.boolean().optional(),
985+
additionalArgs: z.array(z.string()).optional()
986+
}
987+
};
988+
989+
registerCLITool(mockServer, definition);
990+
991+
const handler = (mockServer.registerTool as ReturnType<typeof vi.fn>).mock.calls[0][2];
992+
993+
executeCodeQLCommand.mockResolvedValueOnce({
994+
stdout: 'Compilation successful',
995+
stderr: '',
996+
success: true
997+
});
998+
999+
await handler({ query: '/path/to/query.ql', additionalArgs: ['--no-dump-dil'] });
1000+
1001+
const call = executeCodeQLCommand.mock.calls[0];
1002+
const options = call[1] as Record<string, unknown>;
1003+
const positionalArgs = call[2] as string[];
1004+
1005+
// --no-dump-dil should be normalized into the options object
1006+
expect(options['dump-dil']).toBe(false);
1007+
1008+
// --no-dump-dil should be filtered from raw CLI args to avoid duplication
1009+
expect(positionalArgs).not.toContain('--no-dump-dil');
1010+
});
1011+
1012+
it('should use last --dump-dil/--no-dump-dil flag in additionalArgs for codeql_query_compile', async () => {
1013+
const definition: CLIToolDefinition = {
1014+
name: 'codeql_query_compile',
1015+
description: 'Compile query',
1016+
command: 'codeql',
1017+
subcommand: 'query compile',
1018+
inputSchema: {
1019+
query: z.string(),
1020+
'dump-dil': z.boolean().optional(),
1021+
additionalArgs: z.array(z.string()).optional()
1022+
}
1023+
};
1024+
1025+
registerCLITool(mockServer, definition);
1026+
1027+
const handler = (mockServer.registerTool as ReturnType<typeof vi.fn>).mock.calls[0][2];
1028+
1029+
executeCodeQLCommand.mockResolvedValueOnce({
1030+
stdout: 'Compilation successful',
1031+
stderr: '',
1032+
success: true
1033+
});
1034+
1035+
await handler({
1036+
query: '/path/to/query.ql',
1037+
additionalArgs: ['--dump-dil', '--no-dump-dil']
1038+
});
1039+
1040+
const call = executeCodeQLCommand.mock.calls[0];
1041+
const options = call[1] as Record<string, unknown>;
1042+
const positionalArgs = call[2] as string[];
1043+
1044+
// Last flag wins: --no-dump-dil is last → dump-dil should be false
1045+
expect(options['dump-dil']).toBe(false);
1046+
1047+
// Both flags should be filtered from raw CLI args
1048+
expect(positionalArgs).not.toContain('--dump-dil');
1049+
expect(positionalArgs).not.toContain('--no-dump-dil');
1050+
});
1051+
1052+
it('should preserve non-dump-dil additionalArgs while filtering dump-dil flags for codeql_query_compile', async () => {
1053+
const definition: CLIToolDefinition = {
1054+
name: 'codeql_query_compile',
1055+
description: 'Compile query',
1056+
command: 'codeql',
1057+
subcommand: 'query compile',
1058+
inputSchema: {
1059+
query: z.string(),
1060+
'dump-dil': z.boolean().optional(),
1061+
additionalArgs: z.array(z.string()).optional()
1062+
}
1063+
};
1064+
1065+
registerCLITool(mockServer, definition);
1066+
1067+
const handler = (mockServer.registerTool as ReturnType<typeof vi.fn>).mock.calls[0][2];
1068+
const originalQueryLogDir = process.env.CODEQL_QUERY_LOG_DIR;
1069+
const queryLogDir = createTestTempDir('codeql-query-compile-dump-dil-logs');
1070+
1071+
process.env.CODEQL_QUERY_LOG_DIR = queryLogDir;
1072+
1073+
try {
1074+
executeCodeQLCommand.mockResolvedValueOnce({
1075+
stdout: 'DIL output here',
1076+
stderr: '',
1077+
success: true
1078+
});
1079+
1080+
await handler({
1081+
query: '/path/to/query.ql',
1082+
additionalArgs: ['--threads=4', '--dump-dil', '--warnings=show']
1083+
});
1084+
1085+
const call = executeCodeQLCommand.mock.calls[0];
1086+
const options = call[1] as Record<string, unknown>;
1087+
const positionalArgs = call[2] as string[];
1088+
1089+
// dump-dil should be normalized into options
1090+
expect(options['dump-dil']).toBe(true);
1091+
1092+
// --dump-dil should be filtered, but other args preserved
1093+
expect(positionalArgs).not.toContain('--dump-dil');
1094+
expect(positionalArgs).toContain('--threads=4');
1095+
expect(positionalArgs).toContain('--warnings=show');
1096+
} finally {
1097+
if (originalQueryLogDir === undefined) {
1098+
delete process.env.CODEQL_QUERY_LOG_DIR;
1099+
} else {
1100+
process.env.CODEQL_QUERY_LOG_DIR = originalQueryLogDir;
1101+
}
1102+
1103+
rmSync(queryLogDir, { force: true, recursive: true });
1104+
}
1105+
});
1106+
8341107
it('should pass format to CLI for codeql_bqrs_interpret', async () => {
8351108
const definition: CLIToolDefinition = {
8361109
name: 'codeql_bqrs_interpret',

0 commit comments

Comments
 (0)