Skip to content

Avoid timeouts in client integration test fixtures#74

Merged
data-douser merged 1 commit intomainfrom
dd/integration-test-improvements/1
Feb 23, 2026
Merged

Avoid timeouts in client integration test fixtures#74
data-douser merged 1 commit intomainfrom
dd/integration-test-improvements/1

Conversation

@data-douser
Copy link
Copy Markdown
Collaborator

This pull request refactors how timeouts are handled when calling MCP tools, especially CodeQL tools, across the integration test runner, monitoring integration test runner, and MCP client. The main improvement is the consistent application of a generous 5-minute timeout for all CodeQL-related tool invocations to prevent intermittent request timeouts in CI environments. The refactor also centralizes and simplifies the tool invocation logic, reducing duplication and improving maintainability.

Timeout and Tool Invocation Refactoring

  • Introduced a callTool helper method in both IntegrationTestRunner and MonitoringIntegrationTestRunner that automatically applies a 5-minute timeout for all CodeQL tools (those with names starting with codeql_), and a 1-minute timeout for others. This replaces scattered, tool-specific timeout logic throughout the codebase. [1] [2]
  • Updated all internal calls within IntegrationTestRunner to use the new callTool method, removing duplicated timeout logic and simplifying tool invocation throughout the class. [1] [2] [3] [4] [5] [6] [7]

MCP Client Improvements

  • Refactored the callTool method in CodeQLMCPClient to use the same logic: any tool with a name starting with codeql_ gets a 5-minute timeout and has resetTimeoutOnProgress enabled, ensuring consistent behavior across the codebase.

Test Suite Consistency

  • Updated the MCPTestSuite to explicitly use a 5-minute timeout and progress reset for its tool invocation, bringing it in line with the new standard.

Overall, these changes make the codebase more robust against CI flakiness due to timeouts and easier to maintain by consolidating timeout logic in one place.

Replace brittle longRunningTools whitelists with a prefix-based check
(toolName.startsWith("codeql_")) so that every codeql_* tool gets a
5-minute timeout with progress-based resets. This fixes intermittent
-32001 RequestTimeout failures in CI (e.g. codeql_pack_install) caused
by cold JVM starts, network pack downloads, and Windows runner overhead.

- ql-mcp-client.js: callTool() now checks prefix instead of whitelist
- integration-test-runner.js: add callTool() helper with timeout logic;
  migrate all direct this.client.callTool() calls to use it
- monitoring-integration-test-runner.js: add timeout options to callTool()
- mcp-test-suite.js: add explicit timeout to connectivity test
@data-douser data-douser self-assigned this Feb 23, 2026
Copilot AI review requested due to automatic review settings February 23, 2026 11:30
@data-douser data-douser added the bug Something isn't working label Feb 23, 2026
@github-actions
Copy link
Copy Markdown
Contributor

Dependency Review

✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.

Scanned Files

None

@data-douser data-douser marked this pull request as ready for review February 23, 2026 11:31
@data-douser data-douser requested review from a team and enyil as code owners February 23, 2026 11:31
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR refactors MCP tool invocation in the client integration testing harness to apply consistent request timeouts, aiming to reduce CI flakiness from intermittent MCP request timeouts—especially for CodeQL-related tools.

Changes:

  • Updated CodeQLMCPClient.callTool to treat all codeql_* tools as long-running (5-minute timeout + resetTimeoutOnProgress), with 1-minute defaults for others.
  • Added callTool helpers to both IntegrationTestRunner and MonitoringIntegrationTestRunner and migrated internal tool calls to use them.
  • Updated MCPTestSuite’s tool call to explicitly use the 5-minute timeout + progress reset.

Reviewed changes

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

File Description
client/src/ql-mcp-client.js Standardizes timeouts based on codeql_* prefix and enables progress-based timeout reset for CodeQL tools.
client/src/lib/integration-test-runner.js Introduces a centralized callTool helper and replaces scattered timeout logic with consistent defaults.
client/src/lib/monitoring-integration-test-runner.js Adds a callTool helper mirroring the standardized timeout behavior.
client/src/lib/mcp-test-suite.js Aligns basic tool invocation with the new 5-minute CodeQL timeout standard.

Comment thread client/src/lib/integration-test-runner.js
Comment thread client/src/lib/monitoring-integration-test-runner.js
Comment thread client/src/ql-mcp-client.js
@data-douser data-douser merged commit bdfa53d into main Feb 23, 2026
16 checks passed
@data-douser data-douser deleted the dd/integration-test-improvements/1 branch February 23, 2026 11:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants