Add offline GitHub proxy for E2E tests#1199
Merged
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR strengthens cross-SDK E2E isolation by adding a shared local CONNECT/MITM proxy to the test harness, so CLI subprocesses can’t reach GitHub services directly while CAPI traffic continues to be replayed via the existing proxy.
Changes:
- Add a shared CONNECT/MITM proxy with an ephemeral CA and domain allow/block controls.
- Mock
github.com,api.github.com, andapi.mcp.github.com, while forwardingapi.githubcopilot.comto the replaying CAPI proxy. - Update Node/Python/Go/.NET E2E harness wrappers to parse startup metadata and inject proxy/CA + isolated GitHub CLI config/token env vars.
Show a summary per file
| File | Description |
|---|---|
| test/harness/util.ts | Removes SDK type dependency by inlining the shell tool-name shape used by the harness. |
| test/harness/server.ts | Boots CONNECT proxy + mocks, tracks blocked/unhandled network activity, and prints startup metadata for wrappers. |
| test/harness/replayingCapiProxy.ts | Adds stop hook for coordinated shutdown and improves header/tool-output normalization for new proxy scenarios. |
| test/harness/replayingCapiProxy.test.ts | Adds regression coverage for new GitHub CLI auth/proxy failure normalization. |
| test/harness/package.json | Adds certificate-generation deps (node-forge + types) for the MITM CA/certs. |
| test/harness/package-lock.json | Locks new deps required by the MITM proxy. |
| test/harness/mockHandlers.ts | Implements mock routing/forwarding for GitHub/GitHub API/MCP registry and forwards CAPI to the replay proxy. |
| test/harness/connectProxy.ts | Implements the CONNECT proxy, TLS MITM, domain allow/block, and socket lifecycle handling. |
| test/harness/connectProxy.test.ts | Adds unit coverage for CONNECT parsing, interception, blocking, and mock behavior. |
| test/harness/certUtils.ts | Generates an ephemeral CA and per-host certs for TLS interception. |
| python/e2e/testharness/proxy.py | Parses proxy startup metadata and provides env vars to route subprocess HTTPS via the CONNECT proxy. |
| python/e2e/testharness/context.py | Injects proxy env + isolates GH config dir and uses fake tokens in CI. |
| nodejs/test/e2e/harness/sdkTestContext.ts | Injects CONNECT proxy env into Node E2E subprocess environment and isolates GH config dir. |
| nodejs/test/e2e/harness/CapiProxy.ts | Parses harness startup metadata and exposes CONNECT proxy env vars to tests/subprocesses. |
| go/internal/e2e/testharness/proxy.go | Parses startup metadata and provides a CONNECT-proxy environment block for subprocesses. |
| go/internal/e2e/testharness/context.go | Injects CONNECT-proxy env + isolates GH config dir and uses fake tokens in CI. |
| dotnet/test/Harness/E2ETestContext.cs | Injects CONNECT proxy env + isolates GH config dir and uses fake tokens in CI. |
| dotnet/test/Harness/CapiProxy.cs | Parses startup metadata into strongly-typed fields via source-generated JSON context. |
Copilot's findings
Files not reviewed (1)
- test/harness/package-lock.json: Language not supported
- Files reviewed: 17/18 changed files
- Comments generated: 4
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Contributor
There was a problem hiding this comment.
Generated by SDK Consistency Review Agent for issue #1199 · ● 549.6K
This comment has been minimized.
This comment has been minimized.
Route SDK E2E test subprocess traffic through a shared CONNECT proxy so GitHub and MCP requests are handled locally while CAPI traffic continues to use replay snapshots. Wire the proxy metadata into the Node, Python, Go, and .NET harnesses and add coverage for the proxy and GitHub CLI auth normalization. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
a2b4303 to
c3c50a4
Compare
This comment has been minimized.
This comment has been minimized.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This comment has been minimized.
This comment has been minimized.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This comment has been minimized.
This comment has been minimized.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This comment has been minimized.
This comment has been minimized.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This comment has been minimized.
This comment has been minimized.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
Cross-SDK Consistency Review ✅This PR adds the offline CONNECT proxy to the E2E harness across all four SDK implementations. The changes are well-implemented and consistent across Node.js, Python, Go, and .NET. Summary of changes per SDKAll four harness wrappers implement the same behaviour:
Minor implementation note (no action needed)Python's startup parsing uses two separate regexes (one for the URL, one for the JSON metadata at end of line) instead of a single combined regex pattern used by the other three SDKs. This is functionally equivalent — Python correctly fails if metadata is absent, validates the JSON, and checks for empty
|
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.
E2E replay previously only covered CAPI traffic, so CLI subprocesses could still contact GitHub services directly during SDK tests. This adds a shared CONNECT proxy to the E2E harness so GitHub, GitHub API, and MCP registry traffic is intercepted locally while CAPI traffic continues to flow through the existing replay proxy.
The shared TypeScript harness now generates an ephemeral CA, starts a CONNECT/MITM proxy, mocks
github.com,api.github.com, andapi.mcp.github.com, and blocks unexpected outbound CONNECT targets. The Node, Python, Go, and .NET E2E wrappers parse the shared proxy metadata and inject proxy/CA/isolation environment variables into CLI subprocesses, including isolatedGH_CONFIG_DIRand fake CI auth tokens for tests that start additional CLI server processes.Generated by Copilot.