Skip to content

Add godoc to mcptool AddTool/Connect/ListTools and fix agent.Tool references to tool.Tool - #685

Open
PratikDhanave wants to merge 2 commits into
microsoft:mainfrom
PratikDhanaveFork:docs-mcptool-godoc
Open

Add godoc to mcptool AddTool/Connect/ListTools and fix agent.Tool references to tool.Tool#685
PratikDhanave wants to merge 2 commits into
microsoft:mainfrom
PratikDhanaveFork:docs-mcptool-godoc

Conversation

@PratikDhanave

Copy link
Copy Markdown
Contributor

What

  • Add a one-line godoc to the three exported functions in tool/mcptool/mcp.go: AddTool, Connect, and ListTools. They previously printed as bare signatures under go doc.
  • Correct three stale references to a non-existent agent.Tool type (the package doc, an inline comment, and the mcpWrapper struct comment). There is no agent.Tool; the package actually exposes MCP tools as tool.Tool (ListTools returns []tool.Tool) and registers tool.FuncTool.

Why

The .NET and Python MCP integrations document their public connect/list/register entry points; leaving these Go equivalents undocumented and pointing at a type that does not exist is a cross-SDK parity and correctness gap for anyone browsing the API. The wording now names the types that are genuinely in play (tool.Tool, tool.FuncTool).

Testing

Docs-only change.

  • go build ./... passes.
  • go vet ./tool/mcptool/... passes.
  • go test ./tool/mcptool/... passes.
  • go doc ./tool/mcptool now shows the descriptions.
  • grep -n 'agent.Tool' tool/mcptool/mcp.go returns nothing.

Copilot AI review requested due to automatic review settings July 24, 2026 01:29
@PratikDhanave
PratikDhanave requested a review from a team as a code owner July 24, 2026 01:29

Copilot AI 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.

Pull request overview

Improves the public Go API documentation for the tool/mcptool package by adding missing godoc for exported entry points and correcting stale type references so that go doc reflects the actual tool.Tool / tool.FuncTool types used by the integration.

Changes:

  • Add one-line godoc comments for the exported AddTool, Connect, and ListTools functions.
  • Replace stale agent.Tool references in comments with tool.Tool to match the real exported types.
  • Update inline documentation around the MCP tool wrapper to reflect tool.Tool.
Comments suppressed due to low confidence (1)

tool/mcptool/mcp.go:60

  • In ListTools, the local variable name agentTool is now misleading since this package returns tool.Tool values (and the surrounding comment has been updated accordingly). Renaming it would improve readability and align terminology with the corrected docs.
	for _, mcpTool := range toolsResult.Tools {
		agentTool := newMCPToolWrapper(session, mcpTool)
		result = append(result, agentTool)
	}

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread tool/mcptool/mcp.go Outdated
Comment on lines +3 to +5
// Package mcp provides integration with the Model Context Protocol (MCP).
// It allows agents to connect to external MCP servers via stdio, HTTP, or WebSocket
// and expose their tools and prompts as agent.Tool instances.
// and expose their tools and prompts as tool.Tool instances.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good catch — fixed in 5b08887: renamed the godoc opener to Package mcptool to match the package name, and dropped the "and prompts" claim since only tools are exposed here.

@github-actions

This comment has been minimized.

@github-actions github-actions Bot added the parity-approved Go API consistency review found no parity issues label Jul 24, 2026
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

…erences

Document the three exported functions in tool/mcptool, which previously
printed as bare signatures under go doc. Also correct stale references to
a non-existent agent.Tool type (package doc, an inline comment, and the
mcpWrapper struct comment); the package exposes MCP tools as tool.Tool
and registers tool.FuncTool, matching the tool package it actually uses.
@github-actions

Copy link
Copy Markdown
Contributor

Parity Review: ✅ No issues found

This PR makes documentation-only changes to tool/mcptool/mcp.go:

  • Adds one-line godoc comments to three already-exported functions: AddTool, Connect, and ListTools
  • Corrects stale comment references from the non-existent agent.Tool type to the accurate tool.Tool / tool.FuncTool types

No exported Go API surface was added, removed, or changed. Behavior and signatures are identical to the pre-PR state.

Cross-repo parity: The .NET and Python MCP integrations already document their public connect/list/register entry points. This change brings the Go documentation into alignment with that practice — a net parity improvement with no divergence introduced.

Labels: The public-api-change label is not warranted (no API surface changed). The existing parity-approved label is correct and should remain.

Warning

Firewall blocked 1 domain

The following domain was blocked by the firewall during workflow execution:

  • awmgmcpg

To allow these domains, add them to the network.allowed list in your workflow frontmatter:

network:
  allowed:
    - defaults
    - "awmgmcpg"

See Network Configuration for more information.

Generated by Go API Consistency Review Agent · 20.2 AIC · ⌖ 5.55 AIC · ⊞ 5.9K ·

@gdams
gdams added this pull request to the merge queue Jul 29, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to a conflict with the base branch Jul 29, 2026
@gdams
gdams enabled auto-merge July 29, 2026 14:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

parity-approved Go API consistency review found no parity issues

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants