Skip to content

Feat/tls host auto detect#21

Merged
hrntknr merged 1 commit intomainfrom
feat/tls-host-auto-detect
Aug 18, 2025
Merged

Feat/tls host auto detect#21
hrntknr merged 1 commit intomainfrom
feat/tls-host-auto-detect

Conversation

@hrntknr
Copy link
Copy Markdown
Member

@hrntknr hrntknr commented Aug 18, 2025

Summary

(Brief description of what this PR accomplishes)

Type of Change

  • feat: A new feature
  • fix: A bug fix
  • docs: Documentation only changes
  • style: Changes that do not affect the meaning of the code (white-space, formatting, missing semi-colons, etc)
  • refactor: A code change that neither fixes a bug nor adds a feature
  • perf: A code change that improves performance
  • test: Adding missing tests or correcting existing tests
  • build: Changes that affect the build system or external dependencies
  • ci: Changes to our CI configuration files and scripts
  • chore: Other changes that don't modify src or test files
  • revert: Reverts a previous commit

Related Issues

Copilot AI review requested due to automatic review settings August 18, 2025 10:19
…anagement

- Add TLS_HOST_AUTO_DETECT option to automatically detect TLS host from external URL
- Improve server startup/shutdown handling for both HTTP and HTTPS modes
- Update README with clearer TLS setup instructions and simplified examples
- Add command logging for stdio transport debugging
@hrntknr hrntknr force-pushed the feat/tls-host-auto-detect branch from 4a28d1d to 4822e35 Compare August 18, 2025 10:20
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR implements automatic TLS host detection and adds support for MCP (Model Context Protocol) backends beyond HTTP reverse proxies. The change allows the application to automatically configure TLS certificates for domains specified in the external URL, and introduces the ability to proxy to command-line MCP servers via stdio transport in addition to existing HTTP/SSE transport support.

Key changes:

  • Added automatic TLS host detection from external URL when tlsHostAutoDetect is enabled
  • Introduced ProxyBackend for handling MCP server commands via stdio transport
  • Refactored proxy configuration to accept either HTTP URLs or command arguments as proxy targets

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
pkg/proxy/main.go Updated proxy router to accept generic http.Handler instead of specific reverse proxy
pkg/mcp-proxy/main.go Added TLS auto-detection logic and support for MCP backend commands
pkg/backend/main.go New MCP backend implementation for stdio transport with command execution
main.go Added TLS auto-detect flag and removed specific proxy URL parameter
go.mod Added MCP-go library dependency and updated package versions
README.md Updated documentation to reflect new command-line usage patterns
Dockerfile Added runtime dependencies for Node.js, npm, Python, and uv

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment thread pkg/mcp-proxy/main.go
if tlsHost != "" {
if !tlsAcceptTOS {
if tlsHostDetected {
return errors.New("TLS host is auto-detected, but tlsAcceptTOS is not set to true. Please agree to the TOS or set tlsHostAutoDetect to false")
Copy link

Copilot AI Aug 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The error message should use consistent parameter naming. It mentions tlsHostAutoDetect but the actual parameter is tlsHostAutoDetect (with different casing). Consider using the exact parameter name for clarity.

Copilot uses AI. Check for mistakes.
@hrntknr hrntknr merged commit dc3c058 into main Aug 18, 2025
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants