Open
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR introduces a WebSocket-to-TCP bridge in the DAP subsystem so remote debugging can connect via ws:///wss:// to the runner’s DAP server (e.g., through Dev Tunnels using GitHub auth), while the internal DAP server continues to speak DAP-over-TCP.
Changes:
- Add
WebSocketDapBridgeto translate WebSocket text frames ↔ DAP TCP (Content-Length framed) messages. - Update
DapDebuggerstartup/shutdown to expose the tunnel port via the WebSocket bridge and move the internal DAP listener to an ephemeral local port. - Add L0 coverage for the bridge and for DapDebugger WebSocket connectivity (including “pre-upgraded” WebSocket streams).
Show a summary per file
| File | Description |
|---|---|
| src/Runner.Worker/Dap/WebSocketDapBridge.cs | New bridge implementation handling upgrade detection, handshake, and bidirectional message pumping. |
| src/Runner.Worker/Dap/DapDebugger.cs | Starts/stops the bridge and uses an internal ephemeral TCP port for the actual DAP server when the bridge is enabled. |
| src/Test/L0/Worker/WebSocketDapBridgeL0.cs | New L0 tests validating forwarding, rejection paths, message size limits, and disposal behavior. |
| src/Test/L0/Worker/DapDebuggerL0.cs | Extends L0 tests to validate DapDebugger can accept DAP initialize via WebSocket and pre-upgraded WebSocket streams. |
Copilot's findings
- Files reviewed: 4/4 changed files
- Comments generated: 7
TingluoHuang
reviewed
Apr 9, 2026
|
|
||
| namespace GitHub.Runner.Worker.Dap | ||
| { | ||
| internal sealed class WebSocketDapBridge : IAsyncDisposable |
Member
There was a problem hiding this comment.
do you want to wrap this as IRunnerService, so you don't have to pass in a tracer.
TingluoHuang
reviewed
Apr 9, 2026
TingluoHuang
reviewed
Apr 9, 2026
| _trace.Info($"WebSocket DAP bridge listening on {_listener.LocalEndpoint} -> 127.0.0.1:{_targetPort}"); | ||
| } | ||
|
|
||
| public async ValueTask DisposeAsync() |
Member
There was a problem hiding this comment.
do you want to explicit shutdown the WebSocketDapBridge from the caller?
ex: we can just make a method call public async Task Shutdown()
TingluoHuang
reviewed
Apr 9, 2026
TingluoHuang
reviewed
Apr 9, 2026
TingluoHuang
reviewed
Apr 9, 2026
TingluoHuang
reviewed
Apr 9, 2026
TingluoHuang
reviewed
Apr 9, 2026
Co-authored-by: Tingluo Huang <tingluohuang@github.com>
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.
This adds a bridge converting messages to/from TCP <-> WS so that we can connect to the DAP server through
wss://directly and using GitHub credentials out of the box with Dev Tunnels.https://github.com/github/c2c-actions/issues/9831