Conversation
There was a problem hiding this comment.
Pull request overview
Adds a new Kubernetes Cluster tool category to the Servers.com MCP server, enabling inspection of managed Kubernetes clusters and their nodes, plus label updates, and documents the new tools.
Changes:
- Registers a new Kubernetes Cluster tool group in the main tool registry.
- Adds Kubernetes cluster + node tools (
list/get/updatecluster,list/getnodes) backed byserverscom-go-client. - Updates tool documentation (
TOOLS.md) and adjusts high-level docs to reflect the new category/tool count.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| internal/tools/tools.go | Registers the new Kubernetes Cluster tool category during server initialization. |
| internal/tools/kubernetes_clusters.go | Implements Kubernetes cluster and node MCP tools (list/get/update + node inspection). |
| TOOLS.md | Documents the new Kubernetes Cluster tools and node role/type notes. |
| README.md | Updates the reported tool/category counts (but needs a follow-up doc consistency update). |
| .github/copilot-instructions.md | Updates contributor documentation to include the new tool file and API tag mapping. |
| ## Available Tools | ||
|
|
||
| 73 tools across 6 categories — see **[TOOLS.md](TOOLS.md)** for the full reference. | ||
| 78 tools across 7 categories — see **[TOOLS.md](TOOLS.md)** for the full reference. | ||
|
|
There was a problem hiding this comment.
The tool count/category summary is updated, but the later “Project Structure” tree in this README still lists internal/tools/ ending at rbs.go and doesn’t include the new kubernetes_clusters.go, which makes the documentation inconsistent. Please update the tree section as well.
| | Remote Block Storage (RBS) | `rbs.go` | | ||
| | Kubernetes Cluster | `kubernetes_clusters.go` | | ||
|
|
||
| Not yet implemented: Cloud Computing, Cloud Block Storage, Load Balancers, Network Pool, Kubernetes Cluster, Racks, Billing/Account, Metrics. | ||
| Not yet implemented: Cloud Computing, Cloud Block Storage, Load Balancers, Network Pool, Racks, Billing/Account, Metrics. |
There was a problem hiding this comment.
Now that Kubernetes Cluster tools are implemented and mapped here, the earlier “Top-level services” section should also document h.client.KubernetesClusters alongside the other client services to keep contributor guidance accurate.
|
|
||
| type updateKubernetesClusterArgs struct { | ||
| ClusterID string `json:"cluster_id" jsonschema:"cluster ID,required"` | ||
| Labels map[string]string `json:"labels,omitempty" jsonschema:"key-value labels (replaces all existing labels)"` |
There was a problem hiding this comment.
update_kubernetes_cluster currently allows calls with only cluster_id because labels is optional. That makes the tool behavior ambiguous (no-op vs clearing labels depending on API semantics). Consider making labels required in the args schema and/or returning a clear error when args.Labels == nil so callers can’t accidentally send an empty update.
| Labels map[string]string `json:"labels,omitempty" jsonschema:"key-value labels (replaces all existing labels)"` | |
| Labels map[string]string `json:"labels" jsonschema:"key-value labels (replaces all existing labels),required"` |
| type listKubernetesClusterNodesArgs struct { | ||
| ClusterID string `json:"cluster_id" jsonschema:"cluster ID,required"` | ||
| } |
There was a problem hiding this comment.
listKubernetesClusterNodesArgs duplicates kubernetesClusterIDArgs (both only contain cluster_id). Reusing the existing args type would reduce duplication and keep the API surface consistent if the ID field’s schema/description changes later.
| type listKubernetesClusterNodesArgs struct { | |
| ClusterID string `json:"cluster_id" jsonschema:"cluster ID,required"` | |
| } | |
| type listKubernetesClusterNodesArgs = kubernetesClusterIDArgs |
No description provided.