The class is missing an __init__ method to initialize the _oauth_tokens dictionary that would be used for OAuth2 token caching (referenced in comment ID 006). Without this initialization, the _handle_oauth2 method will fail with an AttributeError when it tries to access self._oauth_tokens. Add an __init__ method that initializes self._oauth_tokens = {}.

The pyproject.toml references a README.md file that does not exist in the gnmi plugin directory. This will cause packaging issues. Either create a README.md file with appropriate documentation (similar to the GraphQL plugin's README) or remove the readme field from the project configuration.

The gRPC channel created on lines 174-178 is never closed. gRPC channels should be properly closed after use to avoid resource leaks. Consider wrapping the channel usage in a try-finally block or using async context manager pattern to ensure the channel is closed even if the streaming operation is interrupted.

The getattr(msg_mod, "SubscriptionList".upper(), None) is incorrect and will always evaluate to None. It attempts to get an attribute named "SUBSCRIPTIONLIST" (all uppercase string literal) instead of the subscription mode enum value. This should likely retrieve the subscription mode from the message module based on the mode variable, such as getattr(msg_mod, f"SubscriptionList.Mode.{mode}", 0) or similar, depending on the protobuf structure.

The empty pass statement in the TLS branch serves no purpose and makes the code harder to read. Consider removing it or adding a comment explaining why TLS is always allowed.

Import of 'Field' is not used.

The GnmiCommunicationProtocol class is missing a close() method. The GraphQL protocol implements this method (line 228-229 in gql_communication_protocol.py) to clean up OAuth tokens. For consistency and proper resource cleanup, the gNMI protocol should also implement this method.

The error message "Unsupported gNMI operation" is generic and doesn't indicate what operation was provided. Consider including the actual operation value in the error message for easier debugging, e.g., f"Unsupported gNMI operation: {op}".

While catching specific exception types (UtcpSerializerValidationError, ValueError) is better than a bare except Exception, you should not suppress the variable e in the except clause. The caught exception is logged via logger.exception() which will include the traceback, but explicitly using the exception variable would be more clear. Also, the except clause on line 112 is inconsistent - it only catches UtcpSerializerValidationError, not ValueError. Consider standardizing the exception handling across both branches.

The test suite lacks coverage for the core call_tool and call_tool_streaming methods in GnmiCommunicationProtocol. While test_register_manual_and_tools validates tool registration, there are no tests for actual RPC calls (capabilities, get, set) or streaming subscribe. The GraphQL plugin includes comprehensive tests that call tools (lines 103-110 in test_graphql_protocol.py). Consider adding similar tests with mocked gRPC stubs to verify the request building and response handling logic.

@copilot l request to apply changes based on this feedback to the current PR

P2: GraphQL variable types are inferred only from Python runtime values and default to String, which will break schemas expecting lists, input objects, or ID types. This can cause GraphQL validation errors for many operations.

Check if this issue is valid — if so, understand the root cause and fix it. At plugins/communication_protocols/gql/src/utcp_gql/gql_communication_protocol.py, line 202:

<comment>GraphQL variable types are inferred only from Python runtime values and default to String, which will break schemas expecting lists, input objects, or ID types. This can cause GraphQL validation errors for many operations.</comment>

<file context>
@@ -39,98 +58,172 @@ async def _handle_oauth2(self, auth: OAuth2Auth) -> str:
+                elif isinstance(v, float):
+                    t = "Float"
+                else:
+                    t = "String"
+                defs.append(f"${k}: {t}")
+            arg_str = ", ".join(defs)
</file context>

P2: GraphQL queries built here omit a selection set, which will fail validation for any root field that returns an object/list type. Since the protocol no longer accepts a caller-provided query, object-returning tools cannot be executed successfully.

Check if this issue is valid — if so, understand the root cause and fix it. At plugins/communication_protocols/gql/src/utcp_gql/gql_communication_protocol.py, line 209:

<comment>GraphQL queries built here omit a selection set, which will fail validation for any root field that returns an object/list type. Since the protocol no longer accepts a caller-provided query, object-returning tools cannot be executed successfully.</comment>

<file context>
@@ -39,98 +58,172 @@ async def _handle_oauth2(self, auth: OAuth2Auth) -> str:
             arg_pass = f"({arg_pass})" if arg_pass else ""
-            gql_str = f"{op_type} {var_defs} {{ {tool_name}{arg_pass} }}"
+
+            gql_str = f"{op_type} {var_defs} {{ {base_tool_name}{arg_pass} }}"
             document = gql_query(gql_str)
-            result = await session.execute(document, variable_values=tool_args)
</file context>

P2: gNMI path parsing ignores list keys, so paths like interface[name=eth0] are encoded with the brackets in PathElem.name and no key map, preventing access to specific list instances.

Check if this issue is valid — if so, understand the root cause and fix it. At plugins/communication_protocols/gnmi/src/utcp_gnmi/gnmi_communication_protocol.py, line 147:

<comment>gNMI path parsing ignores list keys, so paths like interface[name=eth0] are encoded with the brackets in PathElem.name and no key map, preventing access to specific list instances.</comment>

<file context>
@@ -0,0 +1,299 @@
+                for p in paths:
+                    path_msg = getattr(msg_mod, "Path")()
+                    for elem in [e for e in p.strip("/").split("/") if e]:
+                        pe = getattr(msg_mod, "PathElem")(name=elem)
+                        path_msg.elem.append(pe)
+                    req.path.append(path_msg)
</file context>

Why are these modules imported here? Can some of these not be done at the top level?

The manual version should come from the manual endpoint. You can be opinionated on how the UtcpManual should be transfered accross the network, and ideally keept the transfered object as close to the original UtcpManual as possible, so that the user on the other side can basically just expose an endpoint that returns this UtcpManual json/object