Skip to content
Merged
Show file tree
Hide file tree
Changes from 20 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
a85c51d
Initial plan
Copilot Mar 15, 2026
7a9b15a
Add path resolution and error handling for prompt file path parameters
Copilot Mar 15, 2026
20b083b
Address code review: add advisory comment on existsSync, improve test…
Copilot Mar 15, 2026
53d6722
plan: add extension integration test for prompt error handling
Copilot Mar 15, 2026
988bbd9
Add extension integration test for MCP prompt error handling with inv…
Copilot Mar 15, 2026
bb3f3ed
Initial plan
Copilot Mar 15, 2026
905e4de
fix: return inline errors for invalid prompt inputs
data-douser Mar 22, 2026
40c12a4
fix: promote key prompt parameters from optional to required
data-douser Mar 22, 2026
178c4e5
Addres PR review feedback
data-douser Mar 23, 2026
cfc2586
Sync server/dist/codeql-development-mcp-server.js.map
data-douser Mar 23, 2026
4ae4021
[UPDATE PRIMITIVE] Fix path traversal check, add prompt fixture runne…
Copilot Mar 23, 2026
6c8a229
Apply suggestions from code review
data-douser Mar 23, 2026
e07ae1b
Enforce tidy and rebuild server dist
data-douser Mar 23, 2026
24f76cc
address PR review comments for prompt path handling
data-douser Mar 23, 2026
4df0b32
Merge branch 'main' into dd/improve-error-handling-relative-path-support
data-douser Mar 23, 2026
8228a64
[WIP] Improve prompt error handling and relative path support (#157)
Copilot Mar 23, 2026
197e9f8
More fixes for PR review feedback
data-douser Mar 23, 2026
dc90a69
Merge branch 'main' into dd/improve-error-handling-relative-path-support
data-douser Mar 23, 2026
0db4c6d
Merge branch 'main' into dd/improve-error-handling-relative-path-support
data-douser Mar 23, 2026
0c0a715
[UPDATE PRIMITIVE] Fix markdown injection and platform-dependent path…
Copilot Mar 23, 2026
520a3cc
Add 'actions/cache@v5' for VS Code integration test download
data-douser Mar 23, 2026
080790c
Address latest feedback for PR #153
data-douser Mar 23, 2026
0b346ce
Add retries to download-vscode.js script
data-douser Mar 23, 2026
cb7387b
fix: resilient VS Code download for integration tests
data-douser Mar 23, 2026
7f7356c
Address PR #153 review comments
data-douser Mar 23, 2026
ef7517c
Apply suggestions from code review
data-douser Mar 23, 2026
af3cca6
fix: address PR #153 review comments for path resolution
data-douser Mar 23, 2026
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
# Integration Test: explain_codeql_query / invalid_query_path

## Purpose

Verify that the `explain_codeql_query` prompt returns a helpful error message
when the user provides a `queryPath` that does not exist on disk.

## Expected Behavior

- The prompt handler resolves the relative path against the workspace root.
- When the resolved path does not exist, the prompt returns a warning in the
response messages rather than throwing a raw MCP protocol error.
- The warning message should mention the file was not found so the user can
correct the path.
Comment thread
data-douser marked this conversation as resolved.

## Parameters

| Parameter | Value | Notes |
| -------------- | ------------------------------ | ------------------ |
| `databasePath` | `nonexistent/path/to/database` | Does **not** exist |
| `queryPath` | `nonexistent/path/to/query.ql` | Does **not** exist |
| `language` | `javascript` | Valid language |
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
{
"sessions": [
{
"expectedContentPatterns": [
"does not exist"
]
}
]
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
{
"sessions": [],
"parameters": {
"databasePath": "nonexistent/path/to/database",
"queryPath": "nonexistent/path/to/query.ql",
"language": "javascript"
}
}
178 changes: 176 additions & 2 deletions client/src/lib/integration-test-runner.js
Original file line number Diff line number Diff line change
Expand Up @@ -227,9 +227,34 @@ export class IntegrationTestRunner {
this.logger.log(`Completed ${totalIntegrationTests} tool-specific integration tests`);

// Also run workflow integration tests
await this.runWorkflowIntegrationTests(baseDir);
const workflowIntegrationSucceeded = await this.runWorkflowIntegrationTests(baseDir);
if (!workflowIntegrationSucceeded) {
this.logger.logTest(
"Workflow integration tests",
false,
new Error("Workflow integration tests did not complete successfully")
);
}

// Also run prompt integration tests
const promptIntegrationSucceeded = await this.runPromptIntegrationTests(baseDir);
if (!promptIntegrationSucceeded) {
this.logger.logTest(
"Prompt integration tests",
false,
new Error("Prompt integration tests did not complete successfully")
);
}

if (totalIntegrationTests === 0) {
this.logger.log(
"No integration tests were executed across tool, workflow, or prompt suites.",
Comment thread
data-douser marked this conversation as resolved.
Outdated
"ERROR"
);
return false;
}

return totalIntegrationTests > 0;
return workflowIntegrationSucceeded && promptIntegrationSucceeded;
Comment thread
data-douser marked this conversation as resolved.
Outdated
} catch (error) {
this.logger.log(`Error running integration tests: ${error.message}`, "ERROR");
return false;
Expand Down Expand Up @@ -1095,6 +1120,155 @@ export class IntegrationTestRunner {
return params;
}

/**
* Run prompt-level integration tests.
* Discovers test fixtures under `integration-tests/primitives/prompts/`
* and calls `client.getPrompt()` for each, validating the response.
*/
async runPromptIntegrationTests(baseDir) {
try {
this.logger.log("Discovering and running prompt integration tests...");

const promptTestsDir = path.join(baseDir, "..", "integration-tests", "primitives", "prompts");

if (!fs.existsSync(promptTestsDir)) {
this.logger.log("No prompt integration tests directory found", "INFO");
return true;
Comment thread
data-douser marked this conversation as resolved.
Outdated
}

// Get list of available prompts from the server
const response = await this.client.listPrompts();
const prompts = response.prompts || [];
const promptNames = prompts.map((p) => p.name);

this.logger.log(`Found ${promptNames.length} prompts available on server`);

// Discover prompt test directories (each subdirectory = one prompt name)
const promptDirs = fs
.readdirSync(promptTestsDir, { withFileTypes: true })
.filter((dirent) => dirent.isDirectory())
.map((dirent) => dirent.name);

this.logger.log(
`Found ${promptDirs.length} prompt test directories: ${promptDirs.join(", ")}`
);

let totalPromptTests = 0;
let allPromptTestsPassed = true;
for (const promptName of promptDirs) {
if (!promptNames.includes(promptName)) {
this.logger.log(`Skipping ${promptName} - prompt not found on server`, "WARN");
continue;
}

const promptDir = path.join(promptTestsDir, promptName);
const testCases = fs
.readdirSync(promptDir, { withFileTypes: true })
.filter((dirent) => dirent.isDirectory())
.map((dirent) => dirent.name);

this.logger.log(`Running ${testCases.length} test case(s) for prompt ${promptName}`);

for (const testCase of testCases) {
const passed = await this.runSinglePromptIntegrationTest(promptName, testCase, promptDir);
totalPromptTests++;
if (!passed) {
allPromptTestsPassed = false;
}
}
}

this.logger.log(`Total prompt integration tests executed: ${totalPromptTests}`);
return totalPromptTests > 0 ? allPromptTestsPassed : true;
} catch (error) {
Comment thread
data-douser marked this conversation as resolved.
this.logger.log(`Error running prompt integration tests: ${error.message}`, "ERROR");
return false;
}
}

/**
* Run a single prompt integration test.
*
* Reads parameters from `before/monitoring-state.json`, calls `getPrompt()`,
* and validates that the response contains messages (no protocol-level error).
*/
async runSinglePromptIntegrationTest(promptName, testCase, promptDir) {
const testName = `prompt:${promptName}/${testCase}`;
this.logger.log(`\nRunning prompt integration test: ${testName}`);

try {
const testCaseDir = path.join(promptDir, testCase);
const beforeDir = path.join(testCaseDir, "before");
const afterDir = path.join(testCaseDir, "after");

// Validate test structure
if (!fs.existsSync(beforeDir)) {
this.logger.logTest(testName, false, "Missing before directory");
return false;
}

if (!fs.existsSync(afterDir)) {
this.logger.logTest(testName, false, "Missing after directory");
return false;
}

// Load parameters from before/monitoring-state.json
const monitoringStatePath = path.join(beforeDir, "monitoring-state.json");
if (!fs.existsSync(monitoringStatePath)) {
this.logger.logTest(testName, false, "Missing before/monitoring-state.json");
return false;
}

const monitoringState = JSON.parse(fs.readFileSync(monitoringStatePath, "utf8"));
const params = monitoringState.parameters || {};
resolvePathPlaceholders(params, this.logger);

// Call the prompt
this.logger.log(`Calling prompt ${promptName} with params: ${JSON.stringify(params)}`);

const result = await this.client.getPrompt({
name: promptName,
arguments: params
});

// Validate that the response contains messages (no raw protocol error)
const hasMessages = result.messages && result.messages.length > 0;
if (!hasMessages) {
this.logger.logTest(testName, false, "Expected messages in prompt response");
return false;
}

// If the after/monitoring-state.json has expected content checks, validate
const afterMonitoringPath = path.join(afterDir, "monitoring-state.json");
if (fs.existsSync(afterMonitoringPath)) {
const afterState = JSON.parse(fs.readFileSync(afterMonitoringPath, "utf8"));

// Support both top-level expectedContentPatterns and sessions[].expectedContentPatterns
const sessions = afterState.sessions || [];
const topLevelPatterns = afterState.expectedContentPatterns || [];
const sessionPatterns =
sessions.length > 0 ? sessions[0].expectedContentPatterns || [] : [];
const expectedPatterns = topLevelPatterns.length > 0 ? topLevelPatterns : sessionPatterns;

if (expectedPatterns.length > 0) {
const text = result.messages[0]?.content?.text || "";
for (const pattern of expectedPatterns) {
if (!text.includes(pattern)) {
this.logger.logTest(testName, false, `Expected response to contain "${pattern}"`);
return false;
}
}
}
Comment thread
data-douser marked this conversation as resolved.
}

this.logger.logTest(testName, true, `Prompt returned ${result.messages.length} message(s)`);
return true;
} catch (error) {
this.logger.logTest(testName, false, `Error: ${error.message}`);
return false;
}
}

/**
* Run workflow-level integration tests
* These tests validate complete workflows rather than individual tools
Expand Down
54 changes: 54 additions & 0 deletions client/src/lib/mcp-test-suite.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ export class MCPTestSuite {
await this.testReadResource();
await this.testListPrompts();
await this.testGetPrompt();
await this.testGetPromptWithInvalidPath();
}

/**
Expand Down Expand Up @@ -132,6 +133,59 @@ export class MCPTestSuite {
}
}

/**
* Test that prompts handle invalid file paths gracefully.
*
* The explain_codeql_query prompt requires a `queryPath` parameter.
* When given a nonexistent path it should return a user-friendly error
* message inside the prompt response rather than throwing a raw MCP
* protocol error.
*/
async testGetPromptWithInvalidPath() {
try {
this.logger.log("Testing prompt error handling with invalid file path...");

const result = await this.client.getPrompt({
name: "explain_codeql_query",
arguments: {
databasePath: "nonexistent/path/to/database",
queryPath: "nonexistent/path/to/query.ql",
language: "javascript"
Comment thread
data-douser marked this conversation as resolved.
}
});

// The prompt should return messages (not throw) even for an invalid path
const hasMessages = result.messages && result.messages.length > 0;
if (!hasMessages) {
this.logger.logTest(
"Get Prompt with Invalid Path (explain_codeql_query)",
false,
"Expected messages in response"
);
return false;
}

// The response should contain a human-readable error about the invalid path
const text = result.messages[0]?.content?.text || "";
const mentionsPathError =
text.includes("does not exist") ||
text.includes("not found") ||
text.includes("could not be found") ||
text.includes("File not found") ||
text.includes("⚠");

this.logger.log(`Prompt response mentions path error: ${mentionsPathError}`);
this.logger.logTest("Get Prompt with Invalid Path (explain_codeql_query)", mentionsPathError);

return mentionsPathError;
} catch (error) {
// If the server throws instead of returning a message this test fails,
// which is the exact behaviour we want to fix.
this.logger.logTest("Get Prompt with Invalid Path (explain_codeql_query)", false, error);
return false;
}
}

/**
* Test listing prompts
*/
Expand Down
1 change: 1 addition & 0 deletions extensions/vscode/esbuild.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ const testSuiteConfig = {
'test/suite/bridge.integration.test.ts',
'test/suite/copydb-e2e.integration.test.ts',
'test/suite/extension.integration.test.ts',
'test/suite/mcp-prompt-e2e.integration.test.ts',
'test/suite/mcp-resource-e2e.integration.test.ts',
'test/suite/mcp-server.integration.test.ts',
'test/suite/mcp-tool-e2e.integration.test.ts',
Expand Down
Loading
Loading