mcp query diagnostics: restore strict ONLINE requirement and explain target non-executable failures

This change does two things:

1) Reverts executable-target policy to strict ONLINE backend status
- Query target resolution in Query_Tool_Handler is restored to require UPPER(status)='ONLINE' in runtime_mysql_servers/runtime_pgsql_servers.
- This matches expected semantics: non-ONLINE backends should not be considered executable MCP targets.

2) Replaces generic non-executable errors with actionable diagnostics
- Added Query_Tool_Handler::format_target_unavailable_error(target_id).
- All query-tool paths that previously returned the generic:
  'Target is not executable by this handler'
  now return a reasoned error with context, including:
  - target_id
  - protocol
  - hostgroup_id
  - auth_profile_id
  - concrete reason (for example: empty db_username in auth profile, or no ONLINE backend in runtime_*_servers)
- Internal logs in get_connection()/get_pgsql_connection() now emit the same detailed reason.

3) Improves MCP endpoint failure logging with SQL/target context
- In MCP_JSONRPC_Resource::handle_tools_call(), failed tool calls now log:
  - endpoint/tool/error
  - full arguments payload (existing)
  - additional parsed details when present:
    target_id, schema, sql (trimmed for safety)
- This makes it explicit what SQL was requested when a tool call fails before execution.

4) Protocol normalization retained for robustness
- MCP_Threads_Handler::load_target_auth_map() lowercases protocol values when loading runtime profile joins.
- This avoids protocol-casing drift causing inconsistent routing behavior.

Build validation:
- Successfully recompiled modified objects:
  - lib/obj/Query_Tool_Handler.oo
  - lib/obj/MCP_Endpoint.oo
  - lib/obj/MCP_Thread.oo

Resulting behavior expected on next test run:
- If a target is non-executable, the response and logs will state exactly why (ONLINE/backend/auth reason), instead of a generic error.
- Failure logs will also include the attempted SQL text (for tool calls that carry sql arguments), clarifying whether a backend query was actually executed or blocked before execution.
pull/5386/head
Rene Cannao 2 months ago
parent 49f811a638
commit f4bc1943fb

@ -145,6 +145,7 @@ private:
* @brief Resolve target id (or default target if empty)
*/
const QueryTarget* resolve_target(const std::string& target_id);
std::string format_target_unavailable_error(const std::string& target_id) const;
/**
* @brief Get a connection from the pool

@ -469,6 +469,12 @@ json MCP_JSONRPC_Resource::handle_tools_call(const json& req_json) {
std::string tool_name = req_json["params"]["name"].get<std::string>();
json arguments = req_json["params"].contains("arguments") ? req_json["params"]["arguments"] : json::object();
auto trim_for_log = [](const std::string& input, size_t max_len) -> std::string {
if (input.size() <= max_len) {
return input;
}
return input.substr(0, max_len) + "...";
};
proxy_info("MCP TOOL CALL: endpoint='%s' tool='%s'\n", endpoint_name.c_str(), tool_name.c_str());
proxy_debug(PROXY_DEBUG_GENERIC, 2, "MCP tool call: %s with args: %s\n", tool_name.c_str(), arguments.dump().c_str());
@ -482,9 +488,20 @@ json MCP_JSONRPC_Resource::handle_tools_call(const json& req_json) {
// Tool execution failed - log the error with full context and return in MCP format
std::string error_msg = response.contains("error") ? response["error"].get<std::string>() : "Tool execution failed";
std::string args_str = arguments.dump();
std::string target_id = (arguments.contains("target_id") && arguments["target_id"].is_string()) ? arguments["target_id"].get<std::string>() : "";
std::string schema = (arguments.contains("schema") && arguments["schema"].is_string()) ? arguments["schema"].get<std::string>() : "";
std::string sql = (arguments.contains("sql") && arguments["sql"].is_string()) ? arguments["sql"].get<std::string>() : "";
proxy_error("MCP TOOL CALL FAILED: endpoint='%s' tool='%s' error='%s'\n",
endpoint_name.c_str(), tool_name.c_str(), error_msg.c_str());
proxy_error("MCP TOOL CALL FAILED: arguments='%s'\n", args_str.c_str());
if (!target_id.empty() || !schema.empty() || !sql.empty()) {
proxy_error(
"MCP TOOL CALL FAILED DETAILS: target_id='%s' schema='%s' sql='%s'\n",
target_id.c_str(),
schema.c_str(),
trim_for_log(sql, 512).c_str()
);
}
json mcp_result;
mcp_result["content"] = json::array();
json error_content;

@ -498,8 +498,8 @@ void Query_Tool_Handler::refresh_target_registry() {
std::ostringstream sql;
sql << "SELECT hostname, port FROM " << table_name
<< " WHERE hostgroup_id=" << hostgroup_id
<< " AND (status IS NULL OR UPPER(status) <> 'OFFLINE_HARD')"
<< " ORDER BY CASE WHEN UPPER(COALESCE(status,''))='ONLINE' THEN 0 ELSE 1 END, weight DESC, hostname, port";
<< " AND UPPER(status)='ONLINE'"
<< " ORDER BY weight DESC, hostname, port";
GloAdmin->admindb->execute_statement(sql.str().c_str(), &error, &cols, &affected_rows, &resultset);
if (error) {
proxy_warning("Query_Tool_Handler: endpoint resolution failed for %s/%d: %s\n",
@ -584,6 +584,67 @@ const Query_Tool_Handler::QueryTarget* Query_Tool_Handler::resolve_target(const
return NULL;
}
std::string Query_Tool_Handler::format_target_unavailable_error(const std::string& target_id) const {
const std::string resolved_target_id = target_id.empty() ? default_target_id : target_id;
if (resolved_target_id.empty()) {
if (target_registry.empty()) {
return "No MCP targets loaded in runtime_mcp_target_profiles";
}
std::ostringstream oss;
oss << "No executable default target available. Loaded targets: ";
for (size_t i = 0; i < target_registry.size(); i++) {
const QueryTarget& t = target_registry[i];
if (i) {
oss << ", ";
}
oss << t.target_id << "[protocol=" << t.protocol << ", hostgroup=" << t.hostgroup_id;
if (!t.executable) {
if (t.db_username.empty()) {
oss << ", reason=empty db_username in auth_profile_id=" << t.auth_profile_id;
} else if (t.host.empty()) {
oss << ", reason=no ONLINE backend in runtime_" << (t.protocol == "pgsql" ? "pgsql" : "mysql")
<< "_servers";
} else {
oss << ", reason=not executable";
}
} else {
oss << ", executable=1";
}
oss << "]";
}
return oss.str();
}
for (const auto& t : target_registry) {
if (t.target_id != resolved_target_id) {
continue;
}
if (t.executable) {
return "Target is executable";
}
std::ostringstream oss;
oss << "Target '" << t.target_id << "' is not executable"
<< " [protocol=" << t.protocol
<< ", hostgroup=" << t.hostgroup_id
<< ", auth_profile_id=" << t.auth_profile_id << "]";
if (t.db_username.empty()) {
oss << ": auth profile has empty db_username";
} else if (t.host.empty()) {
oss << ": no ONLINE backend in runtime_" << (t.protocol == "pgsql" ? "pgsql" : "mysql")
<< "_servers for hostgroup " << t.hostgroup_id;
} else {
oss << ": backend " << t.host << ":" << t.port << " resolved but target is still non-executable";
}
return oss.str();
}
return std::string("Unknown target_id: ") + resolved_target_id;
}
void* Query_Tool_Handler::get_connection(const std::string& target_id) {
const auto find_available_connection = [&](const std::string& resolved_target, const std::string& expected_auth_profile_id) -> void* {
pthread_mutex_lock(&pool_lock);
@ -603,7 +664,8 @@ void* Query_Tool_Handler::get_connection(const std::string& target_id) {
const std::string resolved_target = target_id.empty() ? default_target_id : target_id;
const QueryTarget* target = resolve_target(resolved_target);
if (target == NULL || !target->executable) {
proxy_error("Query_Tool_Handler: target '%s' is unknown or not executable\n", resolved_target.c_str());
std::string reason = format_target_unavailable_error(target_id);
proxy_error("Query_Tool_Handler: %s\n", reason.c_str());
return NULL;
}
@ -647,7 +709,8 @@ void* Query_Tool_Handler::get_pgsql_connection(const std::string& target_id) {
const std::string resolved_target = target_id.empty() ? default_target_id : target_id;
const QueryTarget* target = resolve_target(resolved_target);
if (target == NULL || !target->executable) {
proxy_error("Query_Tool_Handler: target '%s' is unknown or not executable\n", resolved_target.c_str());
std::string reason = format_target_unavailable_error(target_id);
proxy_error("Query_Tool_Handler: %s\n", reason.c_str());
return NULL;
}
@ -1560,7 +1623,7 @@ json Query_Tool_Handler::execute_tool(const std::string& tool_name, const json&
return result;
}
if (!target->executable) {
result = create_error_response("Target is not executable by this handler");
result = create_error_response(format_target_unavailable_error(target_id));
return result;
}
@ -2283,7 +2346,7 @@ json Query_Tool_Handler::execute_tool(const std::string& tool_name, const json&
return result;
}
if (!target->executable) {
result = create_error_response("Target is not executable by this handler");
result = create_error_response(format_target_unavailable_error(target_id));
return result;
}
@ -2408,7 +2471,7 @@ json Query_Tool_Handler::execute_tool(const std::string& tool_name, const json&
return result;
}
if (!target->executable) {
result = create_error_response("Target is not executable by this handler");
result = create_error_response(format_target_unavailable_error(target_id));
return result;
}

Loading…
Cancel
Save