-
Couldn't load subscription status.
- Fork 38
MCP register tools #57
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
Conversation
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.
15 issues found across 9 files
React with 👍 or 👎 to teach cubic. You can also tag @cubic-dev-ai to give feedback, ask questions, or re-run the review.
plugins/communication_protocols/http/src/utcp_http/http_communication_protocol.py
Outdated
Show resolved
Hide resolved
plugins/communication_protocols/http/src/utcp_http/sse_communication_protocol.py
Show resolved
Hide resolved
plugins/communication_protocols/http/src/utcp_http/sse_communication_protocol.py
Outdated
Show resolved
Hide resolved
plugins/communication_protocols/socket/src/utcp_socket/tcp_communication_protocol.py
Show resolved
Hide resolved
plugins/communication_protocols/socket/src/utcp_socket/tcp_communication_protocol.py
Outdated
Show resolved
Hide resolved
plugins/communication_protocols/gql/src/utcp_gql/gql_communication_protocol.py
Show resolved
Hide resolved
plugins/communication_protocols/gql/src/utcp_gql/gql_communication_protocol.py
Show resolved
Hide resolved
plugins/communication_protocols/gql/src/utcp_gql/gql_communication_protocol.py
Outdated
Show resolved
Hide resolved
plugins/communication_protocols/mcp/src/utcp_mcp/mcp_communication_protocol.py
Show resolved
Hide resolved
plugins/communication_protocols/mcp/src/utcp_mcp/mcp_communication_protocol.py
Outdated
Show resolved
Hide resolved
…ication_protocol.py Co-authored-by: cubic-dev-ai[bot] <191113872+cubic-dev-ai[bot]@users.noreply.github.com>
…cation_protocol.py Co-authored-by: cubic-dev-ai[bot] <191113872+cubic-dev-ai[bot]@users.noreply.github.com>
…tion_protocol.py Co-authored-by: cubic-dev-ai[bot] <191113872+cubic-dev-ai[bot]@users.noreply.github.com>
…tion_protocol.py Co-authored-by: cubic-dev-ai[bot] <191113872+cubic-dev-ai[bot]@users.noreply.github.com>
…tion_protocol.py Co-authored-by: cubic-dev-ai[bot] <191113872+cubic-dev-ai[bot]@users.noreply.github.com>
…ication_protocol.py Co-authored-by: cubic-dev-ai[bot] <191113872+cubic-dev-ai[bot]@users.noreply.github.com>
plugins/communication_protocols/http/src/utcp_http/streamable_http_communication_protocol.py
Outdated
Show resolved
Hide resolved
|
Btw, after changing that, try running utcp-agent with playwright MCP. And see if browser searching works. That's the goal. |
|
@cubic-dev-ai review |
@h3xxit I've started the AI code review. It'll take a few minutes to complete. |
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.
7 issues found across 12 files
React with 👍 or 👎 to teach cubic. You can also tag @cubic-dev-ai to give feedback, ask questions, or re-run the review.
|
|
||
| if "." not in tool_name: | ||
| is_resource, name = self._is_resource(tool_name) | ||
| server_name, target_resource = normalize(await self._get_tool_server(name, tool_call_template) if not is_resource else await self._get_resource_server(name, tool_call_template)) |
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.
Fully qualified tool names parse the server but then ignore it, re-searching all servers; this can call the wrong server or fail when a specific server was requested.
Prompt for AI agents
Address the following comment on plugins/communication_protocols/mcp/src/utcp_mcp/mcp_communication_protocol.py at line 308:
<comment>Fully qualified tool names parse the server but then ignore it, re-searching all servers; this can call the wrong server or fail when a specific server was requested.</comment>
<file context>
@@ -260,59 +257,148 @@ async def call_tool(self, caller: 'UtcpClient', tool_name: str, tool_args: Dict[
+
+ if "." not in tool_name:
+ is_resource, name = self._is_resource(tool_name)
+ server_name, target_resource = normalize(await self._get_tool_server(name, tool_call_template) if not is_resource else await self._get_resource_server(name, tool_call_template))
+ return McpCommunicationProtocol._ParseToolResult(None, server_name, name, is_resource, target_resource)
+
</file context>
| if not logger.hasHandlers(): # Only add default handler if user didn't configure logging | ||
| handler = logging.StreamHandler(sys.stderr) | ||
| handler.setFormatter(logging.Formatter("%(asctime)s [%(levelname)s] %(filename)s:%(lineno)d - %(message)s")) | ||
| logger.addHandler(handler) |
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.
Adding a handler without disabling propagation can cause duplicate log records if root/global handlers are configured later; set propagate=False after adding the default handler.
(Based on your team's feedback about plugin logs not showing by default, this keeps the default handler while preventing duplicates when applications also configure logging.)
Prompt for AI agents
Address the following comment on plugins/communication_protocols/gql/src/utcp_gql/gql_communication_protocol.py at line 19:
<comment>Adding a handler without disabling propagation can cause duplicate log records if root/global handlers are configured later; set propagate=False after adding the default handler.
(Based on your team's feedback about plugin logs not showing by default, this keeps the default handler while preventing duplicates when applications also configure logging.)</comment>
<file context>
@@ -12,6 +13,13 @@
+if not logger.hasHandlers(): # Only add default handler if user didn't configure logging
+ handler = logging.StreamHandler(sys.stderr)
+ handler.setFormatter(logging.Formatter("%(asctime)s [%(levelname)s] %(filename)s:%(lineno)d - %(message)s"))
+ logger.addHandler(handler)
+ logger.setLevel(logging.INFO)
+
</file context>
| logger.addHandler(handler) | |
| logger.addHandler(handler); logger.propagate = False |
| if not logger.hasHandlers(): # Only add default handler if user didn't configure logging | ||
| handler = logging.StreamHandler(sys.stderr) | ||
| handler.setFormatter(logging.Formatter("%(asctime)s [%(levelname)s] %(filename)s:%(lineno)d - %(message)s")) | ||
| logger.addHandler(handler) |
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.
Adding a handler without disabling propagation can cause duplicate logs when global handlers are configured later; set logger.propagate = False when installing the default handler.
(Based on your team's feedback about restoring plugin logs by default; this keeps logs visible without duplicating them when global logging is configured.)
Prompt for AI agents
Address the following comment on plugins/communication_protocols/socket/src/utcp_socket/tcp_communication_protocol.py at line 23:
<comment>Adding a handler without disabling propagation can cause duplicate logs when global handlers are configured later; set logger.propagate = False when installing the default handler.
(Based on your team's feedback about restoring plugin logs by default; this keeps logs visible without duplicating them when global logging is configured.)</comment>
<file context>
@@ -16,6 +17,12 @@
+if not logger.hasHandlers(): # Only add default handler if user didn't configure logging
+ handler = logging.StreamHandler(sys.stderr)
+ handler.setFormatter(logging.Formatter("%(asctime)s [%(levelname)s] %(filename)s:%(lineno)d - %(message)s"))
+ logger.addHandler(handler)
+ logger.setLevel(logging.INFO)
+
</file context>
| if not logger.hasHandlers(): # Only add default handler if user didn't configure logging | ||
| handler = logging.StreamHandler(sys.stderr) | ||
| handler.setFormatter(logging.Formatter("%(asctime)s [%(levelname)s] %(filename)s:%(lineno)d - %(message)s")) | ||
| logger.addHandler(handler) |
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.
Adding a handler at import time without disabling propagation can cause duplicate logs if the application configures handlers later; set logger.propagate = False when attaching the default handler.
(Based on your team's feedback about restoring plugin logs by default, this change keeps logs visible while preventing duplicates after later logging configuration.)
Prompt for AI agents
Address the following comment on plugins/communication_protocols/text/src/utcp_text/text_communication_protocol.py at line 32:
<comment>Adding a handler at import time without disabling propagation can cause duplicate logs if the application configures handlers later; set logger.propagate = False when attaching the default handler.
(Based on your team's feedback about restoring plugin logs by default, this change keeps logs visible while preventing duplicates after later logging configuration.)</comment>
<file context>
@@ -25,6 +26,13 @@
+if not logger.hasHandlers(): # Only add default handler if user didn't configure logging
+ handler = logging.StreamHandler(sys.stderr)
+ handler.setFormatter(logging.Formatter("%(asctime)s [%(levelname)s] %(filename)s:%(lineno)d - %(message)s"))
+ logger.addHandler(handler)
+ logger.setLevel(logging.INFO)
+
</file context>
| logger.addHandler(handler) | |
| logger.addHandler(handler); logger.propagate = False |
| if not logger.hasHandlers(): # Only add default handler if user didn't configure logging | ||
| handler = logging.StreamHandler(sys.stderr) | ||
| handler.setFormatter(logging.Formatter("%(asctime)s [%(levelname)s] %(filename)s:%(lineno)d - %(message)s")) | ||
| logger.addHandler(handler) |
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.
Adding a handler without disabling propagation can produce duplicate log entries if the app configures root/ancestor handlers after this module is imported; disable propagation when installing the default handler.
(Based on your team's feedback about restoring plugin logs by adding default logging handlers.)
Prompt for AI agents
Address the following comment on plugins/communication_protocols/http/src/utcp_http/sse_communication_protocol.py at line 28:
<comment>Adding a handler without disabling propagation can produce duplicate log entries if the app configures root/ancestor handlers after this module is imported; disable propagation when installing the default handler.
(Based on your team's feedback about restoring plugin logs by adding default logging handlers.)</comment>
<file context>
@@ -21,6 +22,12 @@
+if not logger.hasHandlers(): # Only add default handler if user didn't configure logging
+ handler = logging.StreamHandler(sys.stderr)
+ handler.setFormatter(logging.Formatter("%(asctime)s [%(levelname)s] %(filename)s:%(lineno)d - %(message)s"))
+ logger.addHandler(handler)
+ logger.setLevel(logging.INFO)
+
</file context>
| if not logger.hasHandlers(): # Only add default handler if user didn't configure logging | ||
| handler = logging.StreamHandler(sys.stderr) | ||
| handler.setFormatter(logging.Formatter("%(asctime)s [%(levelname)s] %(filename)s:%(lineno)d - %(message)s")) | ||
| logger.addHandler(handler) |
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.
Adding a handler without disabling propagation can cause duplicate logs when global/root handlers are configured later. Set logger.propagate = False after adding the handler.
(Based on your team's feedback about restoring plugin logs by default, this ensures logs are shown without duplication when users later configure global logging.)
Prompt for AI agents
Address the following comment on plugins/communication_protocols/http/src/utcp_http/http_communication_protocol.py at line 43:
<comment>Adding a handler without disabling propagation can cause duplicate logs when global/root handlers are configured later. Set logger.propagate = False after adding the handler.
(Based on your team's feedback about restoring plugin logs by default, this ensures logs are shown without duplication when users later configure global logging.)</comment>
<file context>
@@ -36,6 +37,12 @@
+if not logger.hasHandlers(): # Only add default handler if user didn't configure logging
+ handler = logging.StreamHandler(sys.stderr)
+ handler.setFormatter(logging.Formatter("%(asctime)s [%(levelname)s] %(filename)s:%(lineno)d - %(message)s"))
+ logger.addHandler(handler)
+ logger.setLevel(logging.INFO)
+
</file context>
| logger.addHandler(handler) | |
| logger.addHandler(handler); logger.propagate = False |
| if not logger.hasHandlers(): # Only add default handler if user didn't configure logging | ||
| handler = logging.StreamHandler(sys.stderr) | ||
| handler.setFormatter(logging.Formatter("%(asctime)s [%(levelname)s] %(filename)s:%(lineno)d - %(message)s")) | ||
| logger.addHandler(handler) |
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.
Adding a handler without disabling propagation can cause duplicate logs if global handlers are configured later; set propagate=False when attaching the default handler.
(Based on your team's feedback about restoring plugin logs by default, this ensures logs show once without duplication when apps later configure logging.)
Prompt for AI agents
Address the following comment on plugins/communication_protocols/cli/src/utcp_cli/cli_communication_protocol.py at line 42:
<comment>Adding a handler without disabling propagation can cause duplicate logs if global handlers are configured later; set propagate=False when attaching the default handler.
(Based on your team's feedback about restoring plugin logs by default, this ensures logs show once without duplication when apps later configure logging.)</comment>
<file context>
@@ -35,6 +36,12 @@
+if not logger.hasHandlers(): # Only add default handler if user didn't configure logging
+ handler = logging.StreamHandler(sys.stderr)
+ handler.setFormatter(logging.Formatter("%(asctime)s [%(levelname)s] %(filename)s:%(lineno)d - %(message)s"))
+ logger.addHandler(handler)
+ logger.setLevel(logging.INFO)
+
</file context>
Summary by cubic
Fixes MCP tool registration by expanding and registering each MCP server tool instead of a single wrapper. Restores plugin logs by adding default stderr logging across communication protocols.
Bug Fixes
Logging