Improve prompt error handling and relative path support

[data-douser]

Fixes #143

Problems solved

• Cryptic MCP protocol errors on invalid file paths: Prompt handlers (develop_codeql_query, document_codeql_query, ql_lsp_iterative_development, etc.) now return inline error messages instead of throwing unhandled MCP protocol errors when given invalid or non-existent paths.
• Relative paths not resolved: File path parameters (e.g. queryPath, testPath, workspaceUri) are now resolved relative to the workspace root via resolvePromptFilePath, so workspace-relative paths work as expected.
• Errors surfaced too late: Validation now runs early via createSafePromptHandler, which wraps each prompt with Zod schema parsing and catches errors before template rendering. Invalid inputs fail fast with a user-facing message.
• Optional parameters that should be required: Key prompt parameters (e.g. language, queryPath) promoted from optional to required where appropriate, so missing values are caught at input time.

Changes

• server/src/prompts/workflow-prompts.ts — Added resolvePromptFilePath, createSafePromptHandler, and toPermissiveShape utilities; refactored all prompt registrations to use them.
• server/test/src/prompts/workflow-prompts.test.ts — Expanded unit tests for path resolution, error handling, and schema validation.
• extensions/vscode/test/suite/mcp-prompt-e2e.integration.test.ts — New E2E integration tests for prompt error handling with invalid paths.
• client/src/lib/integration-test-runner.js, client/src/lib/mcp-test-suite.js — New prompt fixture test runner and test suite utilities.
• client/integration-tests/invalid_query_path/ — Test fixture for invalid path scenario.

[github-actions]

Dependency Review

The following issues were found:

• ✅ 0 vulnerable package(s)
• ✅ 0 package(s) with incompatible licenses
• ✅ 0 package(s) with invalid SPDX license definitions
• ⚠️ 1 package(s) with unknown licenses.

See the Details below.

Snapshot Warnings

⚠️: No snapshots were found for the head SHA af3cca6.

Ensure that dependencies are being submitted on PR branches and consider enabling retry-on-snapshot-warnings. See the documentation for more information and troubleshooting advice.

License Issues

.github/workflows/copilot-setup-steps.yml

Package	Version	License	Issue Type
actions/cache	5.*.*	Null	Unknown License

OpenSSF Scorecard

Package	Version	Score	Details
actions/actions/cache	5.*.*	🟢 6.3	Details Check Score Reason Code-Review 🟢 10 all changesets reviewed Maintained 🟢 10 30 commit(s) and 1 issue activity found in the last 90 days -- score normalized to 10 Packaging ⚠️ -1 packaging workflow not detected Dangerous-Workflow 🟢 10 no dangerous workflow patterns detected Binary-Artifacts 🟢 10 no binaries found in the repo CII-Best-Practices ⚠️ 0 no effort to earn an OpenSSF best practices badge detected Token-Permissions ⚠️ 0 detected GitHub workflow tokens with excessive permissions Pinned-Dependencies 🟢 3 dependency not pinned by hash detected -- score normalized to 3 Fuzzing ⚠️ 0 project is not fuzzed License 🟢 10 license file detected Signed-Releases ⚠️ -1 no releases found Security-Policy 🟢 9 security policy file detected Branch-Protection ⚠️ 0 branch protection not enabled on development/release branches SAST 🟢 10 SAST tool is run on all commits

Scanned Files

• .github/workflows/copilot-setup-steps.yml

[Copilot]

Pull request overview

This PR aims to improve MCP workflow prompt UX by preventing raw MCP protocol errors for invalid prompt inputs (especially invalid enums and problematic file paths), and by adding coverage to ensure prompts return user-friendly inline warnings/errors.

Changes:

• Added prompt-side validation/error recovery (createSafePromptHandler) and permissive prompt-argument registration (toPermissiveShape) to avoid protocol-layer validation failures.
• Implemented workspace-relative file path resolution with warnings (resolvePromptFilePath) and surfaced these warnings in prompt output.
• Expanded automated coverage with new server unit tests and a new VS Code Extension Development Host E2E suite for prompt error handling.

Reviewed changes

Copilot reviewed 8 out of 11 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
server/src/prompts/workflow-prompts.ts	Adds permissive schema registration + safe handler wrapper; introduces file-path resolution/warnings; adjusts prompt schemas/requiredness.
server/test/src/prompts/workflow-prompts.test.ts	Updates schema expectations and adds unit tests for permissive shapes, validation formatting, safe handler behavior, and path resolution.
extensions/vscode/test/suite/mcp-prompt-e2e.integration.test.ts	New end-to-end prompt error-handling tests running against a real spawned MCP server.
extensions/vscode/esbuild.config.js	Registers the new E2E test entrypoint for bundling.
client/src/lib/mcp-test-suite.js	Adds a client-side prompt invalid-path test case to the basic MCP test suite runner.
client/integration-tests/primitives/prompts/explain_codeql_query/invalid_query_path/README.md	Documents the new invalid-path integration test scenario.
client/integration-tests/primitives/prompts/explain_codeql_query/invalid_query_path/before/monitoring-state.json	Adds before fixture for the integration test.
client/integration-tests/primitives/prompts/explain_codeql_query/invalid_query_path/after/monitoring-state.json	Adds after fixture for the integration test.
package-lock.json	Lockfile updates from dependency install/re-lock.
server/dist/codeql-development-mcp-server.js	Rebuilt bundle reflecting the prompt changes.

[Copilot]

Pull request overview

Copilot reviewed 8 out of 11 changed files in this pull request and generated 3 comments.

[data-douser]

@copilot open a new pull request to apply changes based on the comments in this thread

[Copilot]

@data-douser I've opened a new pull request, #155, to work on those changes. Once the pull request is ready, I'll request review from you.

[Copilot]

Pull request overview

Copilot reviewed 9 out of 12 changed files in this pull request and generated 3 comments.

Comments suppressed due to low confidence (2)

server/src/prompts/workflow-prompts.ts:65

• The doc comment says resolvedPath is “always set (to the best-effort absolute path)”, but for the empty/whitespace path case the function returns resolvedPath as the raw (empty) input. Either adjust the docs to reflect that empty input returns the original value, or return an absolute fallback (e.g. workspace root) so the invariant holds.

/**
 * Result of resolving a user-supplied file path in a prompt parameter.
 *
 * `resolvedPath` is always set (to the best-effort absolute path).
 * `warning` is set only when the path is problematic — the caller should
 * embed it in the prompt response so the user sees a clear message.
 */
export interface PromptFilePathResult {
  resolvedPath: string;
  warning?: string;
}

/**
 * Resolve a user-supplied file path for a prompt parameter.
 *
 * Relative paths are resolved against `workspaceRoot` (which defaults to
 * `getUserWorkspaceDir()`).  The function never throws — it returns a
 * `warning` string when the path is empty, contains traversal sequences, or
 * does not exist on disk.
 *
 * @param filePath      - The raw path value from the prompt parameter.
 * @param workspaceRoot - Directory to resolve relative paths against.
 * @returns An object with the resolved absolute path and an optional warning.
 */
export function resolvePromptFilePath(
  filePath: string,
  workspaceRoot?: string,
): PromptFilePathResult {
  if (!filePath || filePath.trim() === '') {
    return {
      resolvedPath: filePath ?? '',
      warning: '⚠ **File path is empty.** Please provide a valid file path.',
    };

server/src/prompts/workflow-prompts.ts:538

• Not all workflow prompts appear to be using the new toPermissiveShape(...) + createSafePromptHandler(...) pattern yet. In particular, check_for_duplicated_code and find_overlapping_queries are still registered with their strict schema shapes and unwrapped handlers, so they can still surface raw MCP protocol validation errors (and don’t get the new path-resolution/warning behavior). Consider updating those registrations too so prompt behavior is consistent across the whole set.

  server.prompt(
    'test_driven_development',
    'Test-driven development workflow for CodeQL queries using MCP tools',
    toPermissiveShape(testDrivenDevelopmentSchema.shape),
    createSafePromptHandler(
      'test_driven_development',

[Copilot]

Pull request overview

Copilot reviewed 9 out of 12 changed files in this pull request and generated 6 comments.

[Copilot]

@data-douser I've opened a new pull request, #162, to work on those changes. Once the pull request is ready, I'll request review from you.

[Copilot]

Pull request overview

Copilot reviewed 9 out of 11 changed files in this pull request and generated 4 comments.

Comments suppressed due to low confidence (3)

server/test/src/prompts/workflow-prompts.test.ts:1234

• Same issue as above: this test expects an inline validation error when required fields are missing, but in production the SDK will reject missing required args before the handler runs. Update the test strategy so it matches the actual invocation path (schema validation before handler).

    it('ql_tdd_advanced handler (createSafePromptHandler) should return inline validation error when required parameters are missing', async () => {
      const handler = getRegisteredHandler(mockServer, 'ql_tdd_advanced');
      const result: PromptResult = await handler({});
      expect(result.messages[0].content.text).toContain('Invalid input');
    });

server/test/src/prompts/workflow-prompts.test.ts:1262

• These tests expect inline validation errors for missing required sarifPath, but sarifPath is required in the registered schema, so real MCP clients will be rejected by SDK validation before the handler runs. Consider replacing these with schema requiredness assertions and rely on E2E coverage for protocol-layer behavior.

    it('sarif_rank handlers should return inline error with no parameters', async () => {
      for (const name of ['sarif_rank_false_positives', 'sarif_rank_true_positives']) {
        const handler = getRegisteredHandler(mockServer, name);
        const result: PromptResult = await handler({});
        expect(result.messages[0].content.text).toContain('Invalid input');

server/test/src/prompts/workflow-prompts.test.ts:1279

• This test expects an inline validation error when queryPath is missing, but queryPath is required in the registered schema, so real MCP clients will get a protocol-level validation error before the handler executes. Adjust the test to align with real behavior (schema requiredness vs handler behavior).

    it('run_query_and_summarize_false_positives handler should return inline error with no parameters', async () => {
      const handler = getRegisteredHandler(mockServer, 'run_query_and_summarize_false_positives');
      const result: PromptResult = await handler({});
      expect(result.messages[0].content.text).toContain('Invalid input');
    });

[Copilot]

Pull request overview

Copilot reviewed 15 out of 17 changed files in this pull request and generated 3 comments.

[Copilot]

Pull request overview

Copilot reviewed 15 out of 17 changed files in this pull request and generated 2 comments.

[Copilot]

Pull request overview

Copilot reviewed 15 out of 17 changed files in this pull request and generated 3 comments.

Comments suppressed due to low confidence (1)

client/src/lib/integration-test-runner.js:1325

• runWorkflowIntegrationTests() always returns { passed: true } when it completes, even if one or more workflow test cases failed (since runSingleWorkflowIntegrationTest catches errors and only logs). This makes the workflowTestsPassed signal unreliable and can cause runIntegrationTests() to report success despite workflow failures. Consider having workflow test execution return booleans and aggregate pass/fail like the new prompt fixture runner does.

      // Run tests for each workflow
      let totalWorkflowTests = 0;
      for (const workflowName of workflowDirs) {
        const testCount = await this.runWorkflowTests(workflowName, workflowTestsDir);
        totalWorkflowTests += testCount;
      }

      this.logger.log(`Total workflow integration tests executed: ${totalWorkflowTests}`);
      return { executed: totalWorkflowTests, passed: true };
    } catch (error) {

[Copilot]

Pull request overview

Copilot reviewed 15 out of 17 changed files in this pull request and generated 4 comments.

[Copilot]

In the empty/whitespace-only path case, resolvedPath is set to the original filePath value (which can be just whitespace). This can leak a meaningless value into downstream context; consider returning resolvedPath: '' (or filePath.trim()) to keep the result consistent with other “blocked/invalid” outcomes.

Suggested change

	resolvedPath: filePath ?? '',
	resolvedPath: '',

[Copilot]

toolTestsPassed is currently computed as totalIntegrationTests > 0, which only checks that some tests executed, not that they passed. This makes the aggregated pass/fail result from runIntegrationTests() unreliable; consider tracking failures from runSingleIntegrationTest() / runToolIntegrationTests() and using that to compute toolTestsPassed.

[Copilot]

runWorkflowIntegrationTests() always returns { passed: true } when it completes, even if individual workflow tests were marked failed via logTest. This prevents callers from reliably using the returned passed flag; consider tracking whether any workflow test failed (or remove the passed field and rely solely on executed).

[Copilot]

suppressedError is assigned in the unhandledRejection handler and reset on each attempt, but it’s never read. This is likely to trip no-unused-vars/lint rules; either remove the variable or include it in the retry error reporting so it serves a purpose.