Skip to content

refactor: replace JSON strings with typed proto messages in cloud tools#4583

Merged
amir20 merged 1 commit intomasterfrom
cloud-proto-typed-results
Apr 4, 2026
Merged

refactor: replace JSON strings with typed proto messages in cloud tools#4583
amir20 merged 1 commit intomasterfrom
cloud-proto-typed-results

Conversation

@amir20
Copy link
Copy Markdown
Owner

@amir20 amir20 commented Apr 4, 2026

Summary

  • Replaced opaque result_json string in CallToolResponse with a oneof result containing typed proto messages (ListHostsResult, ListContainersResult, ContainerStatsResult, ActionResult)
  • Replaced repeated string tools_json in ListToolsResponse with repeated ToolDefinition tools
  • Removed Go-side result structs and JSON marshaling from ExecuteTool — now returns *pb.CallToolResponse directly

Test plan

  • All existing cloud package tests updated and passing
  • Full project compiles (go build ./...)
  • Verify cloud server handles the new proto types correctly

🤖 Generated with Claude Code

…l responses

Cloud tool results and definitions were serialized as opaque JSON strings
inside proto messages, forcing the cloud server to parse unknown JSON.
Now uses proper proto message types (HostInfo, ContainerInfo, etc.) so
the cloud server receives structured data it can format as needed.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@claude
Copy link
Copy Markdown

claude bot commented Apr 4, 2026

Clean refactor — typed proto messages are strictly better than opaque JSON strings.

A few observations:

  • Missing test: get_running_container_stats has no test case (tools_test.go covers the other 3 new tools). Worth adding, especially the Stats == nil || Len() == 0 skip branch.
  • ParametersJson still opaque (tools.go:206, 251): ToolDefinition.parameters_json is still a raw JSON string. Slightly inconsistent with the stated goal of replacing JSON strings with typed messages — though understandable since parameter schemas are inherently dynamic.
  • Double ListAllContainers call (tools.go:357, 376, 402): list_running_containers, list_all_containers, and get_running_container_stats each call ListAllContainers independently. If a cloud client calls multiple tools per request cycle this isn't an issue, but worth noting.
  • Breaking interface change (tools.go:194): Adding Hosts() to ToolHostService breaks any implementations outside the package. Fine if this is internal-only, but worth confirming.

Overall the direction is correct — the typed result oneof in CallToolResponse and removing the intermediate Go result structs are good improvements.

@amir20 amir20 merged commit cfcaebe into master Apr 4, 2026
12 checks passed
@amir20 amir20 deleted the cloud-proto-typed-results branch April 4, 2026 14:55
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