Skip to content

Commit e561fa4

Browse files
data-douserdata-douser
committed
Address PR review comments & fix integration tests
1 parent b20db0b commit e561fa4

File tree

11 files changed

+104
-57
lines changed

11 files changed

+104
-57
lines changed

‎client/src/lib/integration-test-runner.js‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -832,7 +832,16 @@ export class IntegrationTestRunner {
832832

833833
if (success) {
834834
this.logger.log(`✅ ${toolName}/${testCase} - Tool executed successfully`);
835-
this.logger.log(` Result: ${result.content?.[0]?.text || "No content"}`);
835+
// Truncate long results to avoid excessive CI log output
836+
const resultText = result.content?.[0]?.text || "No content";
837+
const MAX_LOG_LENGTH = 500;
838+
if (resultText.length > MAX_LOG_LENGTH) {
839+
this.logger.log(
840+
` Result: ${resultText.substring(0, MAX_LOG_LENGTH)}... (truncated, ${resultText.length} chars total)`
841+
);
842+
} else {
843+
this.logger.log(` Result: ${resultText}`);
844+
}
836845
} else {
837846
this.logger.log(`❌ ${toolName}/${testCase} - Tool execution failed`);
838847
const errorText = result.content?.[0]?.text || "Unknown error";

‎client/src/lib/monitoring-integration-test-runner.js‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -330,8 +330,7 @@ export class MonitoringIntegrationTestRunner {
330330
const params = {};
331331

332332
if (toolName === "codeql_lsp_diagnostics") {
333-
params.query = "from DataFlow::Configuration cfg select cfg";
334-
params.language = "javascript";
333+
params.ql_code = "from DataFlow::Configuration cfg select cfg";
335334
} else if (toolName === "codeql_query_format") {
336335
// Look for .ql files in the before directory
337336
const beforeDir = path.join(testCaseDir, "before");

‎docs/tools-reference.md‎

Lines changed: 24 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -181,44 +181,44 @@ Get code completions at a cursor position in a CodeQL file.
181181
182182
**Parameters:**
183183

184-
| Parameter | Type | Required | Description |
185-
| --------------- | ------ | -------- | --------------------------------------------- |
186-
| `file_path` | string | Yes | Absolute path to the `.ql`/`.qll` file |
187-
| `line` | number | Yes | 0-based line number |
188-
| `character` | number | Yes | 0-based character offset |
189-
| `file_content` | string | No | File content override (reads disk if omitted) |
190-
| `search_path` | string | No | Search path for CodeQL libraries |
191-
| `workspace_uri` | string | No | Workspace URI for context |
184+
| Parameter | Type | Required | Description |
185+
| --------------- | ------ | -------- | ------------------------------------------------------------------------------ |
186+
| `file_path` | string | Yes | Path to the `.ql`/`.qll` file (relative paths resolved against user workspace) |
187+
| `line` | number | Yes | 0-based line number |
188+
| `character` | number | Yes | 0-based character offset |
189+
| `file_content` | string | No | File content override (reads disk if omitted) |
190+
| `search_path` | string | No | Search path for CodeQL libraries |
191+
| `workspace_uri` | string | No | Workspace URI for context |
192192

193193
### codeql_lsp_definition
194194

195195
Go to the definition of a CodeQL symbol at a given position.
196196

197197
**Parameters:**
198198

199-
| Parameter | Type | Required | Description |
200-
| --------------- | ------ | -------- | --------------------------------------------- |
201-
| `file_path` | string | Yes | Absolute path to the `.ql`/`.qll` file |
202-
| `line` | number | Yes | 0-based line number |
203-
| `character` | number | Yes | 0-based character offset |
204-
| `file_content` | string | No | File content override (reads disk if omitted) |
205-
| `search_path` | string | No | Search path for CodeQL libraries |
206-
| `workspace_uri` | string | No | Workspace URI for context |
199+
| Parameter | Type | Required | Description |
200+
| --------------- | ------ | -------- | ------------------------------------------------------------------------------ |
201+
| `file_path` | string | Yes | Path to the `.ql`/`.qll` file (relative paths resolved against user workspace) |
202+
| `line` | number | Yes | 0-based line number |
203+
| `character` | number | Yes | 0-based character offset |
204+
| `file_content` | string | No | File content override (reads disk if omitted) |
205+
| `search_path` | string | No | Search path for CodeQL libraries |
206+
| `workspace_uri` | string | No | Workspace URI for context |
207207

208208
### codeql_lsp_references
209209

210210
Find all references to a CodeQL symbol at a given position.
211211

212212
**Parameters:**
213213

214-
| Parameter | Type | Required | Description |
215-
| --------------- | ------ | -------- | --------------------------------------------- |
216-
| `file_path` | string | Yes | Absolute path to the `.ql`/`.qll` file |
217-
| `line` | number | Yes | 0-based line number |
218-
| `character` | number | Yes | 0-based character offset |
219-
| `file_content` | string | No | File content override (reads disk if omitted) |
220-
| `search_path` | string | No | Search path for CodeQL libraries |
221-
| `workspace_uri` | string | No | Workspace URI for context |
214+
| Parameter | Type | Required | Description |
215+
| --------------- | ------ | -------- | ------------------------------------------------------------------------------ |
216+
| `file_path` | string | Yes | Path to the `.ql`/`.qll` file (relative paths resolved against user workspace) |
217+
| `line` | number | Yes | 0-based line number |
218+
| `character` | number | Yes | 0-based character offset |
219+
| `file_content` | string | No | File content override (reads disk if omitted) |
220+
| `search_path` | string | No | Search path for CodeQL libraries |
221+
| `workspace_uri` | string | No | Workspace URI for context |
222222

223223
## Testing Tools
224224

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

Lines changed: 23 additions & 10 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.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/language-server.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -267,13 +267,13 @@ export class CodeQLLanguageServer extends EventEmitter {
267267
this.pendingResponses.set(id, { resolve, reject });
268268
this.sendMessage(message);
269269

270-
// Add timeout
270+
// Add timeout — CI environments may need longer for initial JVM warm-up
271271
setTimeout(() => {
272272
if (this.pendingResponses.has(id)) {
273273
this.pendingResponses.delete(id);
274274
reject(new Error(`LSP request timeout for method: ${method}`));
275275
}
276-
}, 10000); // 10 second timeout
276+
}, 30_000); // 30 second timeout
277277
});
278278
}
279279

@@ -385,7 +385,7 @@ export class CodeQLLanguageServer extends EventEmitter {
385385
this.removeAllListeners('diagnostics');
386386
reject(new Error('Timeout waiting for diagnostics'));
387387
}
388-
}, 5000);
388+
}, 30_000); // 30s — CI environments need longer for JVM warm-up
389389

390390
// Listen for diagnostics
391391
const diagnosticsHandler = (params: PublishDiagnosticsParams) => {

‎server/src/lib/query-server.ts‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -114,7 +114,15 @@ export class CodeQLQueryServer extends EventEmitter {
114114

115115
return new Promise((resolve, reject) => {
116116
this.pendingRequests.set(id, { reject, resolve });
117-
this.sendRaw(message);
117+
118+
try {
119+
this.sendRaw(message);
120+
} catch (error) {
121+
// Clean up immediately — sendRaw() failed so no response will arrive.
122+
this.pendingRequests.delete(id);
123+
reject(error instanceof Error ? error : new Error(String(error)));
124+
return;
125+
}
118126

119127
const timer = setTimeout(() => {
120128
if (this.pendingRequests.has(id)) {

‎server/src/tools/lsp/lsp-diagnostics.ts‎

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ import { LanguageServerConfig } from '../../lib/server-config';
1313
import { getServerManager } from '../../lib/server-manager';
1414
import { logger } from '../../utils/logger';
1515
import { getProjectTmpDir } from '../../utils/temp-dir';
16-
import { join, resolve } from 'path';
16+
import { join, isAbsolute, resolve } from 'path';
1717
import { pathToFileURL } from 'url';
1818

1919
export interface LspDiagnosticsParams {
@@ -108,7 +108,17 @@ async function getLanguageServer(
108108
const manager = getServerManager();
109109
const languageServer = await manager.getLanguageServer(config);
110110

111-
const effectiveUri = workspaceUri ?? pathToFileURL(resolve(pkgRoot, 'ql')).href;
111+
// Normalize workspace URI: resolve relative / bare directory paths to
112+
// file:// URIs against getUserWorkspaceDir() (respects CODEQL_MCP_WORKSPACE).
113+
let effectiveUri = workspaceUri;
114+
if (effectiveUri && !effectiveUri.startsWith('file://')) {
115+
const { getUserWorkspaceDir } = await import('../../utils/package-paths');
116+
const absWorkspace = isAbsolute(effectiveUri)
117+
? effectiveUri
118+
: resolve(getUserWorkspaceDir(), effectiveUri);
119+
effectiveUri = pathToFileURL(absWorkspace).href;
120+
}
121+
effectiveUri = effectiveUri ?? pathToFileURL(resolve(pkgRoot, 'ql')).href;
112122
await languageServer.initialize(effectiveUri);
113123

114124
return languageServer;

‎server/src/tools/lsp/lsp-handlers.ts‎

Lines changed: 16 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -6,17 +6,19 @@
66
* opens the requested document, sends the LSP request, and returns the result.
77
*/
88

9-
import { LanguageServerConfig } from '../../lib/server-config';
10-
import { getServerManager } from '../../lib/server-manager';
9+
import { readFile } from 'fs/promises';
10+
import { isAbsolute, resolve } from 'path';
11+
import { pathToFileURL } from 'url';
12+
1113
import {
1214
CompletionItem,
1315
LSPLocation,
1416
TextDocumentPositionParams,
1517
} from '../../lib/language-server';
18+
import { LanguageServerConfig } from '../../lib/server-config';
19+
import { getServerManager } from '../../lib/server-manager';
1620
import { logger } from '../../utils/logger';
17-
import { readFile } from 'fs/promises';
18-
import { pathToFileURL } from 'url';
19-
import { isAbsolute, resolve } from 'path';
21+
import { getUserWorkspaceDir } from '../../utils/package-paths';
2022

2123
/**
2224
* Common parameters for LSP tool invocations.
@@ -51,12 +53,14 @@ async function getInitializedServer(params: LSPToolParams) {
5153
const manager = getServerManager();
5254
const server = await manager.getLanguageServer(config);
5355

54-
// Resolve workspace URI: convert relative paths to absolute file:// URIs
56+
// Resolve workspace URI: convert relative paths to absolute file:// URIs.
57+
// Relative paths resolve against getUserWorkspaceDir() so that
58+
// CODEQL_MCP_WORKSPACE is respected and behaviour is consistent across tools.
5559
let effectiveUri = params.workspaceUri;
5660
if (effectiveUri && !effectiveUri.startsWith('file://')) {
5761
const absWorkspace = isAbsolute(effectiveUri)
5862
? effectiveUri
59-
: resolve(process.cwd(), effectiveUri);
63+
: resolve(getUserWorkspaceDir(), effectiveUri);
6064
effectiveUri = pathToFileURL(absWorkspace).href;
6165
}
6266
effectiveUri = effectiveUri ?? pathToFileURL(resolve(pkgRoot, 'ql')).href;
@@ -71,8 +75,11 @@ async function getInitializedServer(params: LSPToolParams) {
7175
function prepareDocumentPosition(
7276
params: LSPToolParams,
7377
): { absPath: string; docUri: string } {
74-
// Resolve relative paths against CWD (supports integration test fixtures)
75-
const absPath = isAbsolute(params.filePath) ? params.filePath : resolve(process.cwd(), params.filePath);
78+
// Resolve relative paths against getUserWorkspaceDir() so that
79+
// CODEQL_MCP_WORKSPACE is respected and behaviour is consistent across tools.
80+
const absPath = isAbsolute(params.filePath)
81+
? params.filePath
82+
: resolve(getUserWorkspaceDir(), params.filePath);
7683
const docUri = pathToFileURL(absPath).href;
7784

7885
return { absPath, docUri };

‎server/src/tools/lsp/lsp-tools.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ import { logger } from '../../utils/logger';
2424
const lspParamsSchema = {
2525
character: z.number().int().min(0).describe('0-based character offset within the line'),
2626
file_content: z.string().optional().describe('Optional file content override (reads from disk if omitted)'),
27-
file_path: z.string().describe('Absolute path to the CodeQL (.ql/.qll) file'),
27+
file_path: z.string().describe('Path to the CodeQL (.ql/.qll) file. Relative paths are resolved against the user workspace directory (see CODEQL_MCP_WORKSPACE).'),
2828
line: z.number().int().min(0).describe('0-based line number in the document'),
2929
search_path: z.string().optional().describe('Optional search path for CodeQL libraries'),
3030
workspace_uri: z.string().optional().describe('Optional workspace URI for context (defaults to ./ql directory)'),

0 commit comments

Comments
 (0)