Skip to content

docs: clarify required discovery service type - #340

Open
darkdi wants to merge 1 commit into
google:mainfrom
darkdi:docs/discovery-service-type
Open

docs: clarify required discovery service type#340
darkdi wants to merge 1 commit into
google:mainfrom
darkdi:docs/discovery-service-type

Conversation

@darkdi

@darkdi darkdi commented Sep 1, 2026

Copy link
Copy Markdown

Summary

  • mark type as required in the discover_remote_services input schema description
  • list the three accepted values so schema-validation errors are self-explanatory

Addresses #322.

Testing

  • go test ./internal/node -run TestHandleDiscoverRemoteServices -count=1
  • gofmt -d internal/node/mcp_handlers.go

A full go test ./internal/node run was also attempted; an unrelated macOS-only test setup failure occurs because the generated Unix socket path is 119 bytes while the kernel limit is 103.

@google-cla

google-cla Bot commented Sep 1, 2026

Copy link
Copy Markdown

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request updates the JSON schema description for the Type field in DiscoverRemoteServicesParams to clarify that it is required and lists the allowed values. The review feedback points out that a2a is not supported by the underlying protobuf definition and should be removed from the description.

// DiscoverRemoteServicesParams defines the parameters for the discover_remote_services tool.
type DiscoverRemoteServicesParams struct {
Type string `json:"type" jsonschema:"Service type (mcp, inference, a2a)"`
Type string `json:"type" jsonschema:"Required. One of: mcp, inference, a2a."`

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The jsonschema tag lists a2a as one of the accepted values (One of: mcp, inference, a2a.). However, the underlying ServiceType enum in api/sam.proto only defines SERVICE_TYPE_MCP and SERVICE_TYPE_INFERENCE. Passing a2a will fail during api.ParseServiceType(params.Type) and return an error. Please remove a2a from the description unless it is supported by the protobuf definition.

Suggested change
Type string `json:"type" jsonschema:"Required. One of: mcp, inference, a2a."`
Type string `json:"type" jsonschema:"Required. One of: mcp, inference."`

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A2A will be added by @kaisoz soon

@darkdi

darkdi commented Sep 1, 2026

Copy link
Copy Markdown
Author

CI note: the failing e2e-android job timed out in mobile_e2e.sh while waiting 15 seconds for the host-node /mcp endpoint (exit 124). The same failure occurred in run 33445421284 on another PR, and a subsequent run on that branch passed. This patch only changes a jsonschema description; the other 12 checks, including Go tests, lint, Bats, and mesh E2E, passed. I do not have repository permission to rerun the failed job. Could a maintainer rerun e2e-android after the CLA is signed?

@aojea

aojea commented Sep 2, 2026

Copy link
Copy Markdown
Collaborator

please rebase to get the latest ci fixes and sign the cla, thanks

@darkdi
darkdi force-pushed the docs/discovery-service-type branch from 508e6e4 to 4f73afb Compare September 4, 2026 18:26
@darkdi

darkdi commented Sep 4, 2026

Copy link
Copy Markdown
Author

Rebased onto main and force pushed. The commit also carries the address that is on my GitHub account now, it went out under an old one before. CLA is next on my list.

@darkdi

darkdi commented Sep 5, 2026

Copy link
Copy Markdown
Author

@googlebot I signed it!

@darkdi
darkdi force-pushed the docs/discovery-service-type branch from 4f73afb to 8c2d263 Compare September 6, 2026 08:41
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.

2 participants