Fix compile error in mcpGatewayToolBrokerChannel test#299072
Fix compile error in mcpGatewayToolBrokerChannel test#299072connor4312 merged 15 commits intomainfrom
Conversation
Instead of immediately returning empty results for Unknown-state servers and refreshing in the background, wait up to 5 seconds for the server to become live on the first list call. Subsequent calls find the already- resolved promise in _startupGrace and return immediately, so only the first message pays the startup cost. - _waitForStartup: races _ensureServerReady against the configurable grace period (default 5 000 ms); result is cached per server - _shouldUseCachedData: awaits the grace period for Unknown servers, falls back to background-refresh for Outdated servers - _listTools: refactored to Promise.all to parallelise per-server waits - Tests: updated 'starts server when cache state is unknown' to assert tools are returned after the grace period; added new test for the timeout path using createNeverStartingServer
Per review feedback: Outdated servers should use the same per-server startup grace period as Unknown servers. A prior fast startup does not guarantee a fast restart, so both states now race startup against the 5s timeout before returning results.
There was a problem hiding this comment.
Pull request overview
This PR fixes a compile error introduced in PR #298040 ("MCP Gateway: avoid blocking list calls on startup"). The main change is correcting the Promise type parameter in createNeverStartingServer from the overly-broad { state: McpConnectionState.Kind } to the correct McpConnectionState union type. The PR also includes the underlying feature changes from #298040: a startup grace period mechanism in McpGatewayToolBrokerChannel that races server startup against a configurable timeout, allowing cached data to be returned immediately without blocking list calls on server startup.
Changes:
- Added a
_startupGraceper-server promise cache and_waitForStartup/_shouldUseCachedDatamethods toMcpGatewayToolBrokerChannel, replacing the direct_ensureServerReadyblocking calls in_listTools,_listResources, and_listResourceTemplates - Fixed compile error in test:
new Promise<McpConnectionState>(() => { })instead ofnew Promise<{ state: McpConnectionState.Kind }>(() => { }) - Added new test cases covering the grace period behavior for
Unknown,Outdated, and never-starting servers
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
src/vs/workbench/contrib/mcp/common/mcpGatewayToolBrokerChannel.ts |
Adds startup grace period mechanism: _startupGrace map, _waitForStartup, and _shouldUseCachedData; updates _listTools, _listResources, _listResourceTemplates to use it |
src/vs/workbench/contrib/mcp/test/common/mcpGatewayToolBrokerChannel.test.ts |
Fixes compile error in createNeverStartingServer; adds createNeverStartingServer helper and three new tests for startup/grace period behavior |
You can also share your feedback on Copilot code review. Take the survey.
Fixes compile error from #298040 — the Promise type parameter in createNeverStartingServer used { state: McpConnectionState.Kind } (wide enum type) instead of McpConnectionState.