Conversation
- Change default port from 8081 to 80 for HTTP and 443 for HTTPS - Add TLS_HOST, TLS_LISTEN, and TLS_DIRECTORY_URL environment variables - Implement autocert manager for automatic certificate provisioning - Add HTTP to HTTPS redirect when TLS is enabled - Update documentation and examples to reflect new defaults - Update dependencies to latest versions
- Add getEnvBoolWithDefault helper function to parse boolean environment variables - Add tlsAcceptTOS flag and TLS_ACCEPT_TOS environment variable - Update autocert.Manager to use configurable TOS acceptance instead of automatic acceptance - Allows users to explicitly control whether to accept ACME terms of service
- Add proper goroutine synchronization with WaitGroup - Implement graceful shutdown for both HTTP and HTTPS servers - Move server logging to after server setup for better clarity - Add context-based shutdown with 5-second timeout
Replace error channel with proper exit coordination and error collection to handle errors from both HTTP and HTTPS servers gracefully.
- Make exit channel buffered to prevent goroutine blocking - Filter out http.ErrServerClosed from error reporting during graceful shutdown - Add wg.Wait() to ensure both servers complete shutdown before returning
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR adds automatic TLS certificate support via Let's Encrypt ACME protocol, changing the application from a development-focused service to a production-ready web service with proper HTTPS support.
- Implements automatic TLS certificate provisioning using Let's Encrypt with HTTP to HTTPS redirect
- Changes default ports from 8081 to standard web ports (80/443) for production deployment
- Adds new TLS configuration environment variables and updates all examples and documentation
Reviewed Changes
Copilot reviewed 6 out of 7 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/mcp-proxy/main.go | Adds TLS server implementation with autocert manager and dual HTTP/HTTPS server setup |
| main.go | Updates command-line flags and defaults, adds TLS configuration parameters |
| go.mod | Updates Go dependencies to newer versions |
| example/docker-compose.yaml | Updates port mapping from 8081 to 80 |
| example/.mcp.json | Updates URL from localhost:8081 to localhost |
| README.md | Updates documentation with new TLS environment variables and port changes |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
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.
Summary
New Environment Variables
TLS_HOST: Host name for automatic TLS certificate generationTLS_LISTEN: Address to listen on for HTTPS (default::443)TLS_DIRECTORY_URL: ACME directory URL for certificates (default: Let's Encrypt production)