-
Notifications
You must be signed in to change notification settings - Fork 675
Docker mcp gateway #603
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Docker mcp gateway #603
Conversation
Small update for CoplayDev#542
Reviewer's guide (collapsed on small PRs)Reviewer's GuideAdjusts WebSocket client host handling for bind-only addresses and restructures the Docker server image entrypoint to default to a stdio-based MCP gateway with configurable transport via container arguments. Sequence diagram for WebSocket client handling bind-only host addressessequenceDiagram
actor Developer
participant UnityClient
participant WebSocketTransportClient
participant WebSocketServer
Developer->>UnityClient: Configure MCP base URL (e.g. http://0.0.0.0:8080)
UnityClient->>WebSocketTransportClient: Connect(baseUrl)
WebSocketTransportClient->>WebSocketTransportClient: BuildWebSocketUri(baseUrl)
WebSocketTransportClient->>WebSocketTransportClient: Parse baseUrl as httpUri
WebSocketTransportClient->>WebSocketTransportClient: host = httpUri.Host
alt host is bind-only (0.0.0.0 or ::)
WebSocketTransportClient->>WebSocketTransportClient: Log warning via McpLog.Warn
WebSocketTransportClient->>WebSocketTransportClient: host = localhost
end
WebSocketTransportClient->>WebSocketTransportClient: Build ws or wss Uri using host
WebSocketTransportClient->>WebSocketServer: Open WebSocket connection
WebSocketServer-->>WebSocketTransportClient: Connection established
WebSocketTransportClient-->>UnityClient: Ready for MCP communication
Updated class diagram for WebSocketTransportClient host resolutionclassDiagram
class WebSocketTransportClient {
+Connect(string baseUrl)
-BuildWebSocketUri(string baseUrl) Uri
}
class McpLog {
+Warn(string message)
}
WebSocketTransportClient ..> McpLog : uses
class BuildWebSocketUriLogic {
-string baseUrl
-Uri httpUri
-string host
+ResolveHost()
+CreateWebSocketUri()
}
WebSocketTransportClient ..> BuildWebSocketUriLogic : delegates logic
class BindOnlyAddressHandling {
+bool IsBindOnly(string host)
+string ToClientHost(string host)
}
BuildWebSocketUriLogic ..> BindOnlyAddressHandling : checks 0_0_0_0_or_double_colon
BindOnlyAddressHandling ..> McpLog : logs_warn_on_bind_only
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Caution Review failedThe pull request is closed. 📝 WalkthroughWalkthroughTwo files are modified: WebSocketTransportClient.cs now treats both 0.0.0.0 and :: (IPv6 loopback) as bind-only addresses, substituting localhost for client connections and emitting a warning; Server/Dockerfile switches from CMD to ENTRYPOINT for running mcp-for-unity via uv with added documentation. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey - I've found 1 issue, and left some high level feedback:
- Instead of comparing bind-only addresses by string (
"0.0.0.0"and"::"), consider parsinghttpUri.Hostas anIPAddressand checkingIPAddress.Any/IPv6Anyto be more robust (e.g., handling variants like::0). - The new warning log in
BuildWebSocketUrimay fire frequently for valid setups using bind-only hosts; consider lowering this to a debug/info level or rate-limiting it so it doesn’t become noisy in normal operation.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Instead of comparing bind-only addresses by string (`"0.0.0.0"` and `"::"`), consider parsing `httpUri.Host` as an `IPAddress` and checking `IPAddress.Any`/`IPv6Any` to be more robust (e.g., handling variants like `::0`).
- The new warning log in `BuildWebSocketUri` may fire frequently for valid setups using bind-only hosts; consider lowering this to a debug/info level or rate-limiting it so it doesn’t become noisy in normal operation.
## Individual Comments
### Comment 1
<location> `Server/Dockerfile:31` </location>
<code_context>
ENV PYTHONPATH=/app/Server/src
-CMD ["uv", "run", "python", "src/main.py", "--transport", "http", "--http-host", "0.0.0.0", "--http-port", "8080"]
+# ENTRYPOINT allows override via docker run arguments
+# Default: stdio transport (Docker MCP Gateway compatible)
+# For HTTP: docker run -p 8080:8080 <image> --transport http --http-host 0.0.0.0 --http-port 8080
</code_context>
<issue_to_address>
**issue:** The comment about ENTRYPOINT being overridable via `docker run` is a bit misleading.
With an ENTRYPOINT like `[
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| ENV PYTHONPATH=/app/Server/src | ||
|
|
||
| CMD ["uv", "run", "python", "src/main.py", "--transport", "http", "--http-host", "0.0.0.0", "--http-port", "8080"] | ||
| # ENTRYPOINT allows override via docker run arguments |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
issue: The comment about ENTRYPOINT being overridable via docker run is a bit misleading.
With an ENTRYPOINT like `[
* Log a message with implicit URI changes Small update for CoplayDev#542 * Update docker container to default to stdio Replaces CoplayDev#541
* feat: Add CLI for Unity MCP server - Add click-based CLI with 15+ command groups - Commands: gameobject, component, scene, asset, script, editor, prefab, material, lighting, ui, audio, animation, code - HTTP transport to communicate with Unity via MCP server - Output formats: text, json, table - Configuration via environment variables or CLI options - Comprehensive usage guide and unit tests * Update based on AI feedback * Fixes main.py error * Update for further error fix * Update based on AI * Update script.py * Update with better coverage and Tool Readme * Log a message with implicit URI changes Small update for #542 * Minor fixes (#602) * Log a message with implicit URI changes Small update for #542 * Log a message with implicit URI changes Small update for #542 * Add helper scripts to update forks * fix: improve HTTP Local URL validation UX and styling specificity - Rename CSS class from generic "error" to "http-local-url-error" for better specificity - Rename "invalid-url" class to "http-local-invalid-url" for clarity - Disable httpServerCommandField when URL is invalid or transport not HTTP Local - Clear field value and tooltip when showing validation errors - Ensure field is re-enabled when URL becomes valid * Docker mcp gateway (#603) * Log a message with implicit URI changes Small update for #542 * Update docker container to default to stdio Replaces #541 * fix: Rider config path and add MCP registry manifest (#604) - Fix RiderConfigurator to use correct GitHub Copilot config path: - Windows: %LOCALAPPDATA%\github-copilot\intellij\mcp.json - macOS: ~/Library/Application Support/github-copilot/intellij/mcp.json - Linux: ~/.config/github-copilot/intellij/mcp.json - Add mcp.json for GitHub MCP Registry support: - Enables users to install via coplaydev/unity-mcp - Uses uvx with mcpforunityserver from PyPI * Use click.echo instead of print statements * Standardize whitespace * Minor tweak in docs * Use `wait` params * Unrelated but project scoped tools should be off by default * Update lock file * Whitespace cleanup * Update custom_tool_service.py to skip global registration for any tool name that already exists as a built‑in. * Avoid silently falling back to the first Unity session when a specific unity_instance was requested but not found. If a client passes a unity_instance that doesn’t match any session, this code will still route the command to the first available session, which can send commands to the wrong project in multi‑instance environments. Instead, when a unity_instance is provided but no matching session_id is found, return an error (e.g. 400/404 with "Unity instance '' not found") and only default to the first session when no unity_instance was specified. Co-authored-by: sourcery-ai[bot] <58596630+sourcery-ai[bot]@users.noreply.github.com> * Update docs/CLI_USAGE.md Co-authored-by: sourcery-ai[bot] <58596630+sourcery-ai[bot]@users.noreply.github.com> * Updated the CLI command registration to only swallow missing optional modules and to surface real import-time failures, so broken command modules don’t get silently ignored. * Sorted __all__ alphabetically to satisfy RUF022 in __init__.py. * Validate --params is a JSON object before merging. Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> --------- Co-authored-by: Shutong Wu <51266340+Scriptwonder@users.noreply.github.com> Co-authored-by: dsarno <david@lighthaus.us> Co-authored-by: sourcery-ai[bot] <58596630+sourcery-ai[bot]@users.noreply.github.com> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Simpler implementation of #541, which it closes
Summary by Sourcery
Adjust client WebSocket host handling and update the server Docker image entrypoint for MCP gateway usage.
Bug Fixes:
Enhancements:
Documentation:
Summary by CodeRabbit
Release Notes
Bug Fixes
Chores
✏️ Tip: You can customize this high-level summary in your review settings.