Skip to content

Fix: MCP stdio bridge - Make tool responses MCP-compliant and add debugging#11

Merged
renecannao merged 17 commits intov3.1-MCP1_discoveryfrom
v3.1-MCP1_discovery_2
Jan 13, 2026
Merged

Fix: MCP stdio bridge - Make tool responses MCP-compliant and add debugging#11
renecannao merged 17 commits intov3.1-MCP1_discoveryfrom
v3.1-MCP1_discovery_2

Conversation

@renecannao
Copy link
Copy Markdown

Summary

This PR contains critical fixes and improvements to the MCP stdio bridge to make it work properly with Claude Code.

Root Cause & Fix

The main issue was that tool call responses were not following the MCP specification format. Per MCP spec, tool responses must have:

{
  "result": {
    "content": [{"type": "text", "text": "..."}],
    "isError": false
  }
}

The ProxySQL MCP server was returning raw data instead, causing Claude Code to timeout waiting for properly formatted responses.

Changes

C++ Server (lib/MCP_Endpoint.cpp):

  • Modified handle_tools_call() to wrap tool results in MCP-compliant content array format
  • Added proper error handling with isError: true for failed tool executions

Python Bridge (scripts/mcp/proxysql_mcp_stdio_bridge.py):

  • Simplified to pass through responses (server now returns correct format)
  • Added logging to /tmp/proxysql_mcp_bridge.log for debugging

Documentation:

  • Added logging section to bridge README
  • Updated troubleshooting with debug mode instructions

Test Plan

  • list_schemas tool works
  • list_tables tool works
  • describe_table tool works
  • Claude Code receives responses without timeout
  • Logging works for debugging

Commits Overview

This branch includes the debugging journey:

  1. Initial bridge implementation
  2. Various debugging attempts (stdout buffering, stderr redirection)
  3. Discovery of MCP spec compliance issue
  4. Fix in C++ server to return correct format
  5. Simplification of bridge

Add a Python stdio-based MCP server that bridges to ProxySQL's HTTPS
MCP endpoint, enabling Claude Code to use ProxySQL MCP tools directly.

The bridge:
- Implements stdio MCP server protocol (for Claude Code)
- Acts as MCP client to ProxySQL's HTTPS endpoint
- Supports initialize, tools/list, tools/call methods
- Handles authentication via Bearer tokens
- Configurable via environment variables

Usage:
- Configure in Claude Code MCP settings
- Set PROXYSQL_MCP_ENDPOINT environment variable
- Optional: PROXYSQL_MCP_TOKEN for auth
Add PROXYSQL_MCP_DEBUG environment variable to enable verbose logging
of all stdio communication and ProxySQL HTTP requests/responses.
ProxySQL MCP wraps tool responses in {"result": {...}, "success": true}.
The bridge now unwraps this to return just the actual result to Claude Code.

This fixes the LLM error 'The prompt parameter was not received normally'
which occurred because the LLM was receiving the malformed nested structure.
- Unwrap ProxySQL's {"success": ..., "result": ...} wrapper in tool responses
  for MCP protocol compliance
- Fix proxysql.cfg missing closing brace for mcp_variables section
…bridge

- Redirect stderr to /tmp/proxysql_mcp_bridge.log for debugging
- Add extreme verbosity with timestamps for all stdin/stdout/HTTP traffic
- CRITICAL FIX: Set stdout to line-buffered mode to prevent responses
  from being buffered and never reaching Claude Code (causing timeouts)
- Log all HTTP requests/responses to ProxySQL MCP server
- Log all message handling and unwrapping operations
… buffering

The previous sys.stdout.reconfigure(line_buffering=True) didn't work when
stderr is redirected. Now we create a new io.TextIOWrapper around
sys.stdout.buffer with line_buffering=False, ensuring immediate flush.

Also sets PYTHONUNBUFFERED=1 for extra safety.
…timeout

- Revert the stdout replacement changes (was probably not the issue)
- Add detailed logging to _writeline to see exactly what's happening
  when writing to stdout
The MCP protocol requires tool call results to be wrapped in content items
with type and text fields. This matches what other MCP servers do.

Before: {"result": [{"name": "testdb", ...}]}
After:  {"result": [{"type": "text", "text": "[{\"name\": \"testdb\", ...}]"}]}

This should fix the issue where Claude Code was timing out waiting for responses.
Remove all the unwrapping and TextContent wrapping logic that was added.
Go back to the original simple pass-through that just returns the result
from ProxySQL directly. The original format was correct.
The TextIOWrapper may have buffering issues when stderr is redirected.
Writing directly to the binary buffer with encoded bytes ensures
immediate delivery of responses to Claude Code.
The sys.stderr redirection may be interfering with stdout/stdin pipes.
Now logging writes directly to a file handle instead of through sys.stderr.
Restore to exact original code from commit 01c182c.
The original code is clean and simple - just passes through responses.
Only added minimal file-based logging for debugging.
Added _log() calls to track:
- stdout writes (bytes and content preview)
- tools/call handler (name, response, result)
- main startup

Log is written to /tmp/proxysql_mcp_bridge.log
Per MCP spec (https://modelcontextprotocol.io/specification/2025-11-25/server/tools):
- Tool call responses MUST have a "content" array
- Each content item has "type" and "text" fields
- Response includes "isError" boolean

This was the root cause - Claude Code was waiting for the correct format.
The ProxySQL MCP server now wraps tool results in the correct MCP format:
- result.content: array of content items (type: "text", text: "...")
- result.isError: boolean

Per MCP spec: https://modelcontextprotocol.io/specification/2025-11-25/server/tools

Also simplified the bridge to pass through results directly since the server
now returns the correct format.
Added documentation for:
- Log file location (/tmp/proxysql_mcp_bridge.log)
- What information is logged
- How to use logs for debugging
@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Jan 13, 2026

📝 Walkthrough

Summary by CodeRabbit

  • New Features

    • Introduced a ProxySQL MCP stdio bridge that enables seamless integration with Claude Code via stdio-based MCP communication, translating requests to ProxySQL's HTTPS MCP endpoint.
  • Documentation

    • Added comprehensive README covering bridge setup, configuration via environment variables, integration instructions, and troubleshooting guidance.
  • Bug Fixes

    • Fixed configuration syntax error in the MCP variables block.

✏️ Tip: You can customize this high-level summary in your review settings.

Walkthrough

The pull request adds a new Python-based stdio bridge that relays MCP messages between Claude Code and ProxySQL's HTTPS endpoint, modifies the C++ tool handler to wrap results in MCP-compliant format, includes documentation for the bridge setup, and fixes a configuration syntax issue.

Changes

Cohort / File(s) Summary
MCP Tool Result Wrapping
lib/MCP_Endpoint.cpp
Modified handle_tools_call to wrap tool execution results in MCP-compliant format. Interprets ProxySQL wrapper with "success" and "result" fields; returns MCP error content on failure or unwraps "result" on success. Always formats final response as MCP object with single text content item and isError flag.
ProxySQL MCP Stdio Bridge
scripts/mcp/proxysql_mcp_stdio_bridge.py
New Python script implementing stdio-based MCP bridge with two main classes: ProxySQLMCPEndpoint manages HTTPS connection to ProxySQL with bearer token and SSL configuration; StdioMCPServer bridges stdio I/O to ProxySQL, handling JSON-RPC message parsing, routing (initialize, tools/list, tools/call), and translating responses. Includes environment-based configuration and lightweight timestamped logging.
Bridge Documentation
scripts/mcp/STDIO_BRIDGE_README.md
New comprehensive guide documenting the stdio bridge functionality, installation, configuration via environment variables, Claude Code integration, supported MCP methods, available tools, usage examples, logging, and troubleshooting steps.
Configuration Syntax Fix
src/proxysql.cfg
Added closing brace for mcp_variables block to correct configuration syntax.

Sequence Diagram(s)

sequenceDiagram
    participant Claude as Claude Code
    participant Bridge as Stdio Bridge
    participant ProxySQL as ProxySQL MCP
    
    Claude->>Bridge: JSON-RPC initialize (stdin)
    Bridge->>ProxySQL: HTTP POST initialize (with token)
    ProxySQL-->>Bridge: Initialize response
    Bridge->>Claude: JSON-RPC initialize response (stdout)
    
    Claude->>Bridge: JSON-RPC tools/list (stdin)
    Bridge->>ProxySQL: HTTP POST tools/list
    ProxySQL-->>Bridge: Tools list response
    Bridge->>Claude: JSON-RPC tools/list response (stdout)
    
    Claude->>Bridge: JSON-RPC tools/call (stdin)
    Bridge->>ProxySQL: HTTP POST tools/call
    ProxySQL->>ProxySQL: Execute tool<br/>Wrap in MCP format
    ProxySQL-->>Bridge: Tool result (MCP-wrapped)
    Bridge->>Claude: JSON-RPC tools/call response (stdout)
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Poem

🐰 A bridge now spans the stdio stream,
From Claude's code to ProxySQL's dream,
With JSON-RPC tales and tokens bright,
Tools wrapped in format, oh what a sight!
Async hops through HTTPS flight! 🌉✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 71.43% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main fix: making MCP stdio bridge tool responses compliant with the MCP specification and adding debugging support.
Description check ✅ Passed The description clearly relates to the changeset, explaining the root cause (non-compliant MCP format), the fixes applied across C++/Python files, and verification through test plans.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings

Comment @coderabbitai help to get the list of available commands and usage tips.

@gemini-code-assist
Copy link
Copy Markdown

Summary of Changes

Hello @renecannao, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request significantly improves the integration between Claude Code and ProxySQL's Model Context Protocol (MCP) endpoint. The core issue of tool call responses not adhering to the MCP specification, which caused timeouts in Claude Code, has been addressed. By standardizing the response format directly within the ProxySQL C++ server and enhancing the Python stdio bridge with debugging logs, this change ensures reliable and transparent communication, facilitating smoother operation and easier troubleshooting for users.

Highlights

  • MCP Compliance for Tool Responses: The C++ server now correctly formats tool call results according to the MCP specification, including proper error handling for success and failure cases.
  • Enhanced Debugging Capabilities: The Python stdio bridge now includes robust file-based logging to /tmp/proxysql_mcp_bridge.log, providing detailed insights into communication and tool calls for easier troubleshooting.
  • Streamlined Python Bridge Logic: The Python bridge has been simplified, as the C++ server now handles the complex MCP response formatting, allowing the bridge to merely pass through the compliant responses.
  • Comprehensive Documentation: New and updated documentation for the stdio bridge covers installation, configuration, supported methods, example usage, and detailed troubleshooting steps, including how to utilize the new logging.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request makes critical fixes to the MCP stdio bridge for Claude Code compatibility by ensuring tool responses adhere to the MCP specification. It also adds valuable debugging capabilities through logging and documentation updates.

My review focuses on improving the maintainability and robustness of the new C++ and Python code. I've suggested refactoring the C++ code to reduce duplication and improve clarity. For the new Python bridge script, my recommendations include making the logging more flexible, improving exception handling to be more specific, and updating to modern asyncio practices to ensure future compatibility and prevent potential issues with task cancellation.

Comment on lines +173 to +176
except json.JSONDecodeError as e:
await self._write_error(-32700, f"Parse error: {e}", "")
except Exception as e:
await self._write_error(-32603, f"Internal error: {e}", "")
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

high

The broad except Exception can catch and suppress asyncio.CancelledError, which would prevent the task from being properly cancelled. This can lead to resource leaks or unexpected behavior during shutdown. It's better to explicitly catch asyncio.CancelledError and re-raise it to allow for proper task cancellation.

Suggested change
except json.JSONDecodeError as e:
await self._write_error(-32700, f"Parse error: {e}", "")
except Exception as e:
await self._write_error(-32603, f"Internal error: {e}", "")
except json.JSONDecodeError as e:
await self._write_error(-32700, f"Parse error: {e}", "")
except asyncio.CancelledError:
raise
except Exception as e:
await self._write_error(-32603, f"Internal error: {e}", "")

Comment on lines +348 to +383
if (response.is_object() && response.contains("success") && response.contains("result")) {
bool success = response["success"].get<bool>();
if (!success) {
// Tool execution failed - return error in MCP format
json mcp_result;
mcp_result["content"] = json::array();
json error_content;
error_content["type"] = "text";
std::string error_msg = response.contains("error") ? response["error"].get<std::string>() : "Tool execution failed";
error_content["text"] = error_msg;
mcp_result["content"].push_back(error_content);
mcp_result["isError"] = true;
return mcp_result;
}
// Success - wrap result in MCP-compliant format with content array
// Per MCP spec: https://modelcontextprotocol.io/specification/2025-11-25/server/tools
json actual_result = response["result"];
json mcp_result;
mcp_result["content"] = json::array();
json text_content;
text_content["type"] = "text";
text_content["text"] = actual_result.dump(2); // Pretty-print JSON with 2-space indent
mcp_result["content"].push_back(text_content);
mcp_result["isError"] = false;
return mcp_result;
}

// Fallback: wrap response in MCP format (for compatibility with non-standard handlers)
json mcp_result;
mcp_result["content"] = json::array();
json text_content;
text_content["type"] = "text";
text_content["text"] = response.dump(2);
mcp_result["content"].push_back(text_content);
mcp_result["isError"] = false;
return mcp_result;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

The logic for handling successful tool executions and the fallback case contains duplicated code for wrapping the response in the MCP format. This can be refactored to reduce redundancy and improve maintainability.

Additionally, if the tool's result is already a string, calling .dump() on it will add extra quotes, which is likely not the desired behavior. It's better to check if the result is a string and use its value directly in that case.

The suggested refactoring addresses both of these points by first determining the content to be wrapped and then performing the wrapping in a single, shared block of code.

	if (response.is_object() && response.contains("success") && response.contains("result")) {
		bool success = response["success"].get<bool>();
		if (!success) {
			// Tool execution failed - return error in MCP format
			json mcp_result;
			mcp_result["content"] = json::array();
			json error_content;
			error_content["type"] = "text";
			std::string error_msg = response.contains("error") ? response["error"].get<std::string>() : "Tool execution failed";
			error_content["text"] = error_msg;
			mcp_result["content"].push_back(error_content);
			mcp_result["isError"] = true;
			return mcp_result;
		}
		// Success - use the "result" field as the content to be wrapped
		response = response["result"];
	}

	// Wrap the response (or the 'result' field) in MCP-compliant format
	// Per MCP spec: https://modelcontextprotocol.io/specification/2025-11-25/server/tools
	json mcp_result;
	json text_content;
	text_content["type"] = "text";

	if (response.is_string()) {
		text_content["text"] = response.get<std::string>();
	} else {
		text_content["text"] = response.dump(2);  // Pretty-print JSON with 2-space indent
	}

	mcp_result["content"] = json::array({text_content});
	mcp_result["isError"] = false;
	return mcp_result;

Comment on lines +37 to +40
_log_file = open("/tmp/proxysql_mcp_bridge.log", "a", buffering=1)
def _log(msg):
_log_file.write(f"[{datetime.now().strftime('%H:%M:%S.%f')[:-3]}] {msg}\n")
_log_file.flush()
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

Hardcoding the log file path to /tmp/ can be brittle, as this path may not exist or be writable on all systems (e.g., Windows). It's better practice to make this configurable.

Consider using an environment variable to specify the log file path, falling back to the current default if it's not set. This would make the script more portable and flexible for users in different environments.

Suggested change
_log_file = open("/tmp/proxysql_mcp_bridge.log", "a", buffering=1)
def _log(msg):
_log_file.write(f"[{datetime.now().strftime('%H:%M:%S.%f')[:-3]}] {msg}\n")
_log_file.flush()
_log_file_path = os.getenv("PROXYSQL_MCP_BRIDGE_LOG", "/tmp/proxysql_mcp_bridge.log")
_log_file = open(_log_file_path, "a", buffering=1)
def _log(msg):
_log_file.write(f"[{datetime.now().strftime('%H:%M:%S.%f')[:-3]}] {msg}\n")
_log_file.flush()

Comment on lines +98 to +116
except httpx.HTTPStatusError as e:
return {
"jsonrpc": "2.0",
"error": {
"code": -32000,
"message": f"HTTP error: {e.response.status_code}",
"data": str(e)
},
"id": request.get("id", "")
}
except Exception as e:
return {
"jsonrpc": "2.0",
"error": {
"code": -32603,
"message": f"Internal error: {str(e)}"
},
"id": request.get("id", "")
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

Catching a broad Exception can hide bugs and make debugging more difficult. It's better to handle specific exceptions that you expect from httpx.

I suggest adding a specific except block for httpx.RequestError to handle network-related issues like connection errors or timeouts. This will provide more specific and helpful error messages to the user and improve the robustness of the client.

        except httpx.HTTPStatusError as e:
            return {
                "jsonrpc": "2.0",
                "error": {
                    "code": -32000,
                    "message": f"HTTP error: {e.response.status_code}",
                    "data": str(e)
                },
                "id": request.get("id", "")
            }
        except httpx.RequestError as e:
            return {
                "jsonrpc": "2.0",
                "error": {
                    "code": -32002,
                    "message": f"Request to ProxySQL failed: {e}"
                },
                "id": request.get("id", "")
            }
        except Exception as e:
            return {
                "jsonrpc": "2.0",
                "error": {
                    "code": -32603,
                    "message": f"Internal error: {str(e)}"
                },
                "id": request.get("id", "")
            }

async def _readline(self) -> Optional[str]:
"""Read a line from stdin."""
loop = asyncio.get_event_loop()
line = await loop.run_in_executor(None, sys.stdin.readline)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

asyncio.get_event_loop() is deprecated since Python 3.10 and its behavior can be unexpected in some contexts. You should use asyncio.get_running_loop() instead, which is available since Python 3.7 and is the recommended way to get the current running event loop.

Suggested change
line = await loop.run_in_executor(None, sys.stdin.readline)
loop = asyncio.get_running_loop()

async def _writeline(self, data: Any):
"""Write JSON data to stdout."""
loop = asyncio.get_event_loop()
output = json.dumps(data, ensure_ascii=False) + "\n"
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

As in _readline, asyncio.get_event_loop() is deprecated. Please use asyncio.get_running_loop() for better compatibility and to follow modern asyncio best practices.

Suggested change
output = json.dumps(data, ensure_ascii=False) + "\n"
loop = asyncio.get_running_loop()

Python bridge (scripts/mcp/proxysql_mcp_stdio_bridge.py):
- Make log file path configurable via PROXYSQL_MCP_BRIDGE_LOG env var
- Add httpx.RequestError exception handling for network issues
- Fix asyncio.CancelledError not being re-raised (HIGH priority)
- Replace deprecated asyncio.get_event_loop() with get_running_loop()

C++ server (lib/MCP_Endpoint.cpp):
- Refactor handle_tools_call() to reduce code duplication
- Handle string responses directly without calling .dump()
- Single shared wrapping block for all response types

Per review: #11
@renecannao
Copy link
Copy Markdown
Author

@coderabbitai review

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Jan 13, 2026

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In @scripts/mcp/proxysql_mcp_stdio_bridge.py:
- Around line 336-345: The endpoint validation is unreachable because endpoint
is assigned a default; change the os.getenv call that sets endpoint (currently
using "https://127.0.0.1:6071/mcp/query") to not supply a default (so it returns
None/empty when unset) and keep the existing if not endpoint check to exit with
an error; ensure any logging of endpoint (the _log call) tolerates a None/empty
value or adjust it to log that the env var is missing.
🧹 Nitpick comments (5)
scripts/mcp/STDIO_BRIDGE_README.md (2)

43-57: Replace hardcoded user-specific path with a placeholder.

Line 48 contains a hardcoded path /home/rene/proxysql-vec/scripts/mcp/... which is specific to a particular development environment. Use a generic placeholder to make the documentation more portable.

📝 Suggested change
       "command": "python3",
-      "args": ["/home/rene/proxysql-vec/scripts/mcp/proxysql_mcp_stdio_bridge.py"],
+      "args": ["/path/to/proxysql-vec/scripts/mcp/proxysql_mcp_stdio_bridge.py"],
       "env": {

7-12: Consider adding a language identifier to the ASCII diagram code block.

Per markdownlint MD040, fenced code blocks should have a language specified. For ASCII diagrams, you can use text or plaintext as the language identifier for consistency.

📝 Suggested change
-```
+```text
 ┌─────────────┐     stdio      ┌──────────────────┐     HTTPS      ┌──────────┐
 │  Claude Code│  ──────────>  │  stdio Bridge    │  ──────────>  │ ProxySQL │
 │  (MCP Client)│               │  (this script)   │               │  MCP     │
 └─────────────┘               └──────────────────┘               └──────────┘
 ```
scripts/mcp/proxysql_mcp_stdio_bridge.py (3)

38-42: Log file is never explicitly closed.

The file opened at line 39 remains open for the lifetime of the process. While this typically works because the OS closes file handles on process exit, consider registering an atexit handler for cleaner resource management, especially if the bridge is imported as a module rather than run directly.

📝 Suggested improvement
+import atexit
+
 # Minimal logging to file for debugging
 # Log path can be configured via PROXYSQL_MCP_BRIDGE_LOG environment variable
 _log_file_path = os.getenv("PROXYSQL_MCP_BRIDGE_LOG", "/tmp/proxysql_mcp_bridge.log")
 _log_file = open(_log_file_path, "a", buffering=1)
+atexit.register(_log_file.close)
+
 def _log(msg):
     _log_file.write(f"[{datetime.now().strftime('%H:%M:%S.%f')[:-3]}] {msg}\n")
     _log_file.flush()

199-206: Minor style issues flagged by Ruff.

Line 206 uses an f-string without placeholders. This is a minor style issue.

📝 Suggested fix
-        _log(f"WRITE stdout: flushed")
+        _log("WRITE stdout: flushed")

257-272: Unused params argument in _handle_initialize.

The params argument is not used. Consider logging the client info from params for debugging purposes, or remove the argument if it's not needed.

📝 Option: Log client info for debugging
     async def _handle_initialize(self, req_id: str, params: Dict[str, Any]) -> Dict[str, Any]:
         """Handle initialize request."""
+        client_info = params.get("clientInfo", {})
+        _log(f"initialize: client={client_info.get('name', 'unknown')}")
         return {
             "jsonrpc": "2.0",
📜 Review details

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9d6a217 and 606fe2e.

📒 Files selected for processing (4)
  • lib/MCP_Endpoint.cpp
  • scripts/mcp/STDIO_BRIDGE_README.md
  • scripts/mcp/proxysql_mcp_stdio_bridge.py
  • src/proxysql.cfg
🧰 Additional context used
🪛 markdownlint-cli2 (0.18.1)
scripts/mcp/STDIO_BRIDGE_README.md

7-7: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

🪛 Ruff (0.14.11)
scripts/mcp/proxysql_mcp_stdio_bridge.py

38-38: Probable insecure usage of temporary file or directory: "/tmp/proxysql_mcp_bridge.log"

(S108)


90-90: Avoid specifying long messages outside the exception class

(TRY003)


119-119: Do not catch blind exception: Exception

(BLE001)


124-124: Use explicit conversion flag

Replace with conversion flag

(RUF010)


188-188: Do not catch blind exception: Exception

(BLE001)


203-203: Use explicit conversion flag

Replace with conversion flag

(RUF010)


206-206: f-string without any placeholders

Remove extraneous f prefix

(F541)


257-257: Unused method argument: params

(ARG002)


355-355: Do not catch blind exception: Exception

(BLE001)

🔇 Additional comments (4)
src/proxysql.cfg (1)

60-71: LGTM!

The mcp_variables block is now properly closed with the brace, and the blank line separator before genai_variables improves readability and follows the pattern used elsewhere in this config file.

lib/MCP_Endpoint.cpp (1)

344-380: MCP-compliant response wrapping looks correct.

The logic properly:

  1. Unwraps ProxySQL's internal {"success": ..., "result": ...} format
  2. Handles failure cases with isError: true
  3. Wraps successful results in the MCP content array format

One minor note: the spec URL comment on line 367 references 2025-11-25, but the protocol version used elsewhere is 2024-11-05. Consider updating the comment to match or use the correct specification date.

scripts/mcp/proxysql_mcp_stdio_bridge.py (2)

163-189: Main server loop and error handling look good.

The loop correctly:

  • Uses async context manager for the ProxySQL client
  • Sends the notifications/initialized notification on startup
  • Re-raises asyncio.CancelledError for proper task cancellation (line 186-187)
  • Catches JSON parse errors separately from general exceptions

45-127: ProxySQLMCPEndpoint class implementation is solid.

Good use of async context manager pattern, proper error handling with JSON-RPC compliant error responses, and appropriate timeout configuration (120s). The broad Exception catch at line 119 is acceptable here as a catch-all for unexpected errors in an RPC context.

Comment on lines +336 to +345
endpoint = os.getenv("PROXYSQL_MCP_ENDPOINT", "https://127.0.0.1:6071/mcp/query")
token = os.getenv("PROXYSQL_MCP_TOKEN", "")
insecure_ssl = os.getenv("PROXYSQL_MCP_INSECURE_SSL", "0").lower() in ("1", "true", "yes")

_log(f"START: endpoint={endpoint}, insecure_ssl={insecure_ssl}")

# Validate endpoint
if not endpoint:
sys.stderr.write("Error: PROXYSQL_MCP_ENDPOINT environment variable is required\n")
sys.exit(1)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Unreachable code: endpoint validation always passes.

Line 336 provides a default value for endpoint, so the check on line 343 (if not endpoint) will never be true. Either remove the default to make the environment variable truly required, or remove the dead check.

🐛 Option A: Make endpoint truly required (recommended)
-    endpoint = os.getenv("PROXYSQL_MCP_ENDPOINT", "https://127.0.0.1:6071/mcp/query")
+    endpoint = os.getenv("PROXYSQL_MCP_ENDPOINT")
     token = os.getenv("PROXYSQL_MCP_TOKEN", "")
     insecure_ssl = os.getenv("PROXYSQL_MCP_INSECURE_SSL", "0").lower() in ("1", "true", "yes")
Option B: Remove dead code and keep default
     _log(f"START: endpoint={endpoint}, insecure_ssl={insecure_ssl}")

-    # Validate endpoint
-    if not endpoint:
-        sys.stderr.write("Error: PROXYSQL_MCP_ENDPOINT environment variable is required\n")
-        sys.exit(1)
-
     # Run the server
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
endpoint = os.getenv("PROXYSQL_MCP_ENDPOINT", "https://127.0.0.1:6071/mcp/query")
token = os.getenv("PROXYSQL_MCP_TOKEN", "")
insecure_ssl = os.getenv("PROXYSQL_MCP_INSECURE_SSL", "0").lower() in ("1", "true", "yes")
_log(f"START: endpoint={endpoint}, insecure_ssl={insecure_ssl}")
# Validate endpoint
if not endpoint:
sys.stderr.write("Error: PROXYSQL_MCP_ENDPOINT environment variable is required\n")
sys.exit(1)
endpoint = os.getenv("PROXYSQL_MCP_ENDPOINT")
token = os.getenv("PROXYSQL_MCP_TOKEN", "")
insecure_ssl = os.getenv("PROXYSQL_MCP_INSECURE_SSL", "0").lower() in ("1", "true", "yes")
_log(f"START: endpoint={endpoint}, insecure_ssl={insecure_ssl}")
# Validate endpoint
if not endpoint:
sys.stderr.write("Error: PROXYSQL_MCP_ENDPOINT environment variable is required\n")
sys.exit(1)
🤖 Prompt for AI Agents
In @scripts/mcp/proxysql_mcp_stdio_bridge.py around lines 336 - 345, The
endpoint validation is unreachable because endpoint is assigned a default;
change the os.getenv call that sets endpoint (currently using
"https://127.0.0.1:6071/mcp/query") to not supply a default (so it returns
None/empty when unset) and keep the existing if not endpoint check to exit with
an error; ensure any logging of endpoint (the _log call) tolerates a None/empty
value or adjust it to log that the env var is missing.

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.

1 participant