Merged
Conversation
…itHubMCPRealBackend go-sdk v1.4.0 changed session handling so that requests without an Mcp-Session-Id header create new stateful (not stateless) sessions. These uninitialized sessions reject tools/list with an error because the SDK requires initialize to complete before accepting other methods. Fix by capturing Mcp-Session-Id from the initialize response and sending it on all subsequent requests (notifications/initialized, tools/list, tools/call) to reuse the already-initialized SDK session. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ackends Integration tests were failing because: - echo Docker image doesn't exist, causing docker pull attempts to hang - Gateway blocks HTTP server startup until all backends connect (even in routed mode) - TestPlaywrightWithLocalDockerfile had port conflict (13101 shared with TestPipeBasedLaunch) - TestSafeinputsHTTPBackend used time.Sleep(2s) which was insufficient Fixes: - TestOutputConfigWithAuthHeaders: replace echo container with createMinimalMockMCPBackend - TestOutputConfigUnifiedMode: replace echo containers with createMinimalMockMCPBackend - TestBinaryInvocation_ConfigStdin: replace echo container with createMinimalMockMCPBackend - TestSafeinputsHTTPBackend: add --listen flag, replace sleep with waitForServer - TestPlaywrightWithLocalDockerfile: change port 13101→13109, add --unified mode so health check only passes after Docker backend connects (eliminates timing race), update sendMCPRequest URLs from /mcp/mock-playwright to /mcp (unified endpoint) - TestBinaryInvocation_PipeInputOutput: use mock HTTP backend instead of echo container - TestPipeBasedLaunch: use MOCK_BACKEND_URL env var to pass mock HTTP backend URL to shell script - start_gateway_with_pipe.sh: use HTTP backend when MOCK_BACKEND_URL is set, add --max-time 2 to curl health checks to prevent indefinite hangs Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Updates integration tests and helpers to make MCP Gateway startup and session handling more reliable (notably around StreamableHTTP session reuse) and to reduce Docker dependency during automated runs.
Changes:
- Add/propagate
Mcp-Session-Idin the real GitHub MCP integration test to reuse stateful sessions across requests. - Introduce an in-process mock MCP HTTP backend and switch several integration tests/scripts from Docker-backed stdio servers to HTTP backends.
- Reduce startup flakiness by adding explicit
--listenin a test and replacing fixed sleeps with readiness checks; adjust Playwright integration test to use unified endpoint.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| test/integration/start_gateway_with_pipe.sh | Supports MOCK_BACKEND_URL to use HTTP backend; adds curl timeout in readiness loop |
| test/integration/safeinputs_http_test.go | Adds explicit --listen and waits for /health instead of sleeping |
| test/integration/playwright_test.go | Switches to unified mode and /mcp endpoint; adjusts test port |
| test/integration/pipe_launch_test.go | Starts shared in-process mock backend and passes it to the shell script via env |
| test/integration/github_test.go | Captures and reuses Mcp-Session-Id; adds extra logging |
| test/integration/binary_test.go | Adds in-process mock backend helper and uses it to avoid Docker in multiple tests |
| test/integration/auth_config_test.go | Uses in-process mock backends instead of Docker containers |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
168
to
172
| cmd := exec.CommandContext(ctx, binaryPath, | ||
| "--config-stdin", | ||
| "--listen", "127.0.0.1:3001", | ||
| "--routed", | ||
| ) |
There was a problem hiding this comment.
The test hard-codes port 3001 for the gateway listen address, but doesn’t free it (e.g., via killProcessOnPort) or pick an ephemeral port. This can make the test flaky if 3001 is already in use on a dev machine/CI runner. Consider selecting a free port at runtime (net.Listen("127.0.0.1:0") → close → reuse) or calling killProcessOnPort(t, "3001") before starting the gateway, and use that same port consistently in both configJSON and --listen.
Collaborator
Author
There was a problem hiding this comment.
@copilot open a new pull request to apply changes based on this feedback
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
Contributor
|
Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details. Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.