gopls/internal/mcp: more tuning of tools and prompts
After further testing, the go_file_metadata tool does not seem
particularly useful replace it with a go_file_context tool, which
summarizes APIs used within the file.
For golang/go#73580
Change-Id: I8a41f916b7f2a71dac5b6b800088be595ecd8c21
Reviewed-on: https://go-review.googlesource.com/c/tools/+/687355
Reviewed-by: Madeline Kalil <mkalil@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Hongxiang Jiang <hxjiang@golang.org>
diff --git a/gopls/internal/cmd/mcp_test.go b/gopls/internal/cmd/mcp_test.go
index 2be71fd..5fa3b48 100644
--- a/gopls/internal/cmd/mcp_test.go
+++ b/gopls/internal/cmd/mcp_test.go
@@ -231,7 +231,7 @@
}()
var (
- tool = "go_file_metadata"
+ tool = "go_file_context"
args = map[string]any{"file": filepath.Join(tree, "a.go")}
)
res, err := mcpSession.CallTool(ctx, &mcp.CallToolParams{Name: tool, Arguments: args})
diff --git a/gopls/internal/mcp/context.go b/gopls/internal/mcp/context.go
index 6bcd07c..ae2b470 100644
--- a/gopls/internal/mcp/context.go
+++ b/gopls/internal/mcp/context.go
@@ -66,7 +66,7 @@
{
fmt.Fprintf(&result, "%s (current file):\n", pgf.URI.Base())
result.WriteString("```go\n")
- if err := writeFileSummary(ctx, snapshot, pgf.URI, &result, false); err != nil {
+ if err := writeFileSummary(ctx, snapshot, pgf.URI, &result, false, nil); err != nil {
return nil, err
}
result.WriteString("```\n\n")
@@ -81,7 +81,7 @@
fmt.Fprintf(&result, "%s:\n", file.URI.Base())
result.WriteString("```go\n")
- if err := writeFileSummary(ctx, snapshot, file.URI, &result, false); err != nil {
+ if err := writeFileSummary(ctx, snapshot, file.URI, &result, false, nil); err != nil {
return nil, err
}
result.WriteString("```\n\n")
@@ -153,7 +153,7 @@
for _, f := range md.CompiledGoFiles {
fmt.Fprintf(&buf, "%s:\n", f.Base())
buf.WriteString("```go\n")
- if err := writeFileSummary(ctx, snapshot, f, &buf, true); err != nil {
+ if err := writeFileSummary(ctx, snapshot, f, &buf, true, nil); err != nil {
return "" // ignore error
}
buf.WriteString("```\n\n")
@@ -163,7 +163,7 @@
// writeFileSummary writes the file summary to the string builder based on
// the input file URI.
-func writeFileSummary(ctx context.Context, snapshot *cache.Snapshot, f protocol.DocumentURI, out *strings.Builder, onlyExported bool) error {
+func writeFileSummary(ctx context.Context, snapshot *cache.Snapshot, f protocol.DocumentURI, out *strings.Builder, onlyExported bool, declsToSummarize map[string]bool) error {
fh, err := snapshot.ReadFile(ctx, f)
if err != nil {
return err
@@ -173,52 +173,60 @@
return err
}
- // Copy everything before the first non-import declaration:
- // package decl, imports decl(s), and all comments (excluding copyright).
- {
- endPos := pgf.File.FileEnd
+ // If we're summarizing specific declarations, we don't need to copy the header.
+ if declsToSummarize == nil {
+ // Copy everything before the first non-import declaration:
+ // package decl, imports decl(s), and all comments (excluding copyright).
+ {
+ endPos := pgf.File.FileEnd
- outerloop:
- for _, decl := range pgf.File.Decls {
- switch decl := decl.(type) {
- case *ast.FuncDecl:
- if decl.Doc != nil {
- endPos = decl.Doc.Pos()
- } else {
- endPos = decl.Pos()
+ outerloop:
+ for _, decl := range pgf.File.Decls {
+ switch decl := decl.(type) {
+ case *ast.FuncDecl:
+ if decl.Doc != nil {
+ endPos = decl.Doc.Pos()
+ } else {
+ endPos = decl.Pos()
+ }
+ break outerloop
+ case *ast.GenDecl:
+ if decl.Tok == token.IMPORT {
+ continue
+ }
+ if decl.Doc != nil {
+ endPos = decl.Doc.Pos()
+ } else {
+ endPos = decl.Pos()
+ }
+ break outerloop
}
- break outerloop
- case *ast.GenDecl:
- if decl.Tok == token.IMPORT {
- continue
- }
- if decl.Doc != nil {
- endPos = decl.Doc.Pos()
- } else {
- endPos = decl.Pos()
- }
- break outerloop
}
- }
- startPos := pgf.File.FileStart
- if copyright := golang.CopyrightComment(pgf.File); copyright != nil {
- startPos = copyright.End()
- }
+ startPos := pgf.File.FileStart
+ if copyright := golang.CopyrightComment(pgf.File); copyright != nil {
+ startPos = copyright.End()
+ }
- text, err := pgf.PosText(startPos, endPos)
- if err != nil {
- return err
- }
+ text, err := pgf.PosText(startPos, endPos)
+ if err != nil {
+ return err
+ }
- out.Write(bytes.TrimSpace(text))
- out.WriteString("\n\n")
+ out.Write(bytes.TrimSpace(text))
+ out.WriteString("\n\n")
+ }
}
// Write func decl and gen decl.
for _, decl := range pgf.File.Decls {
switch decl := decl.(type) {
case *ast.FuncDecl:
+ if declsToSummarize != nil {
+ if _, ok := declsToSummarize[decl.Name.Name]; !ok {
+ continue
+ }
+ }
if onlyExported {
if !decl.Name.IsExported() {
continue
@@ -251,6 +259,28 @@
continue
}
+ // If we are summarizing specific decls, check if any of them are in this GenDecl.
+ if declsToSummarize != nil {
+ found := false
+ for _, spec := range decl.Specs {
+ switch spec := spec.(type) {
+ case *ast.TypeSpec:
+ if _, ok := declsToSummarize[spec.Name.Name]; ok {
+ found = true
+ }
+ case *ast.ValueSpec:
+ for _, name := range spec.Names {
+ if _, ok := declsToSummarize[name.Name]; ok {
+ found = true
+ }
+ }
+ }
+ }
+ if !found {
+ continue
+ }
+ }
+
// Dump the entire GenDecl (exported or unexported)
// including doc comment without any filtering to the output.
if !onlyExported {
@@ -301,6 +331,11 @@
switch spec := spec.(type) {
case *ast.TypeSpec:
+ if declsToSummarize != nil {
+ if _, ok := declsToSummarize[spec.Name.Name]; !ok {
+ continue
+ }
+ }
// TODO(hxjiang): only keep the exported field of
// struct spec and exported method of interface spec.
if !spec.Name.IsExported() {
@@ -323,6 +358,17 @@
}
case *ast.ValueSpec:
+ if declsToSummarize != nil {
+ found := false
+ for _, name := range spec.Names {
+ if _, ok := declsToSummarize[name.Name]; ok {
+ found = true
+ }
+ }
+ if !found {
+ continue
+ }
+ }
// TODO(hxjiang): only keep the exported identifier.
if !slices.ContainsFunc(spec.Names, (*ast.Ident).IsExported) {
continue
diff --git a/gopls/internal/mcp/file_context.go b/gopls/internal/mcp/file_context.go
new file mode 100644
index 0000000..2a3338f
--- /dev/null
+++ b/gopls/internal/mcp/file_context.go
@@ -0,0 +1,104 @@
+// Copyright 2025 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package mcp
+
+import (
+ "context"
+ "fmt"
+ "go/ast"
+ "go/types"
+ "strings"
+
+ "golang.org/x/tools/gopls/internal/golang"
+ "golang.org/x/tools/gopls/internal/protocol"
+ "golang.org/x/tools/internal/mcp"
+)
+
+type fileContextParams struct {
+ File string `json:"file"`
+}
+
+func (h *handler) fileContextTool() *mcp.ServerTool {
+ return mcp.NewServerTool(
+ "go_file_context",
+ "Summarizes a file's cross-file dependencies",
+ h.fileContextHandler,
+ mcp.Input(
+ mcp.Property("file", mcp.Description("the absolute path to the file")),
+ ),
+ )
+}
+
+func (h *handler) fileContextHandler(ctx context.Context, _ *mcp.ServerSession, params *mcp.CallToolParamsFor[fileContextParams]) (*mcp.CallToolResultFor[any], error) {
+ fh, snapshot, release, err := h.fileOf(ctx, params.Arguments.File)
+ if err != nil {
+ return nil, err
+ }
+ defer release()
+
+ pkg, pgf, err := golang.NarrowestPackageForFile(ctx, snapshot, fh.URI())
+ if err != nil {
+ return nil, err
+ }
+
+ info := pkg.TypesInfo()
+ if info == nil {
+ return nil, fmt.Errorf("no types info for package %q", pkg.Metadata().PkgPath)
+ }
+
+ // Group objects defined in other files by file URI.
+ otherFiles := make(map[protocol.DocumentURI]map[string]bool)
+ addObj := func(obj types.Object) {
+ if obj == nil {
+ return
+ }
+ pos := obj.Pos()
+ if !pos.IsValid() {
+ return
+ }
+ objFile := pkg.FileSet().File(pos)
+ if objFile == nil {
+ return
+ }
+ uri := protocol.URIFromPath(objFile.Name())
+ if uri == fh.URI() {
+ return
+ }
+ if _, ok := otherFiles[uri]; !ok {
+ otherFiles[uri] = make(map[string]bool)
+ }
+ otherFiles[uri][obj.Name()] = true
+ }
+
+ for cur := range pgf.Cursor.Preorder((*ast.Ident)(nil)) {
+ id := cur.Node().(*ast.Ident)
+ addObj(info.Uses[id])
+ addObj(info.Defs[id])
+ }
+
+ var result strings.Builder
+ fmt.Fprintf(&result, "File `%s` is in package %q.\n", params.Arguments.File, pkg.Metadata().PkgPath)
+ fmt.Fprintf(&result, "Below is a summary of the APIs it uses from other files.\n")
+ fmt.Fprintf(&result, "To read the full API of any package, use go_package_api.\n")
+ for uri, decls := range otherFiles {
+ pkgPath := "UNKNOWN"
+ md, err := snapshot.NarrowestMetadataForFile(ctx, uri)
+ if err != nil {
+ if ctx.Err() != nil {
+ return nil, ctx.Err()
+ }
+ } else {
+ pkgPath = string(md.PkgPath)
+ }
+ fmt.Fprintf(&result, "Referenced declarations from %s (package %q):\n", uri.Path(), pkgPath)
+ result.WriteString("```go\n")
+ if err := writeFileSummary(ctx, snapshot, uri, &result, false, decls); err != nil {
+ return nil, err
+ }
+ result.WriteString("```\n\n")
+ }
+
+ return textResult(result.String()), nil
+}
diff --git a/gopls/internal/mcp/instructions.md b/gopls/internal/mcp/instructions.md
index 30b714e..25b8245 100644
--- a/gopls/internal/mcp/instructions.md
+++ b/gopls/internal/mcp/instructions.md
@@ -1,51 +1,46 @@
# The gopls MCP server
-These instructions describe how to efficiently work in the Go programming
-language using the gopls MCP server. They are intended to be provided as context
-for an interactive session using the gopls MCP tool: you can load this file
-directly into a session where the gopls MCP server is connected.
+These instructions describe how to efficiently work in the Go programming language using the gopls MCP server. You can load this file directly into a session where the gopls MCP server is connected.
## Detecting a Go workspace
-Use the `go_workspace` tool to learn about the Go workspace. These instructions
-apply whenever that tool indicates that the user is in a Go workspace.
+At the start of every session, you MUST use the `go_workspace` tool to learn about the Go workspace. The rest of these instructions apply whenever that tool indicates that the user is in a Go workspace.
-## Go Programming Guidelines
+## Go programming workflows
-These guidelines MUST be followed whenever working in a Go workspace. There
-are two workflows described below: the 'Read Workflow' must be followed when
-the user asks a question about a Go workspace. The 'Edit Workflow' must be
-followed when the user edits a Go workspace.
+These guidelines MUST be followed whenever working in a Go workspace. There are two workflows described below: the 'Read Workflow' must be followed when the user asks a question about a Go workspace. The 'Edit Workflow' must be followed when the user edits a Go workspace.
-You may re-do parts of each workflow as necessary to recover from errors.
-However, you cannot skip any steps.
+You may re-do parts of each workflow as necessary to recover from errors. However, you must not skip any steps.
-### Read Workflow
+### Read workflow
-1. **Search the workspace:** When the user asks about a symbol, use
- `go_search` to search for the symbol in question. If you find no matches,
- search for a substring of the user's referenced symbol. If `go_search`
- fails, you may fall back to regular textual search.
-2. **Read files:** Read the relevant file(s). Use the `go_file_metadata` tool
- to get package information for the file.
-3. **Understand packages:** If the user is asking about the use of one or more Go
- package, use the `go_package_outline` command to summarize their API.
+The goal of the read workflow is to understand the codebase.
-### Editing Workflow
+1. **Understand the workspace layout**: Start by using `go_workspace` to understand the overall structure of the workspace, such as whether it's a module, a workspace, or a GOPATH project.
-1. **Read first:** Before making any edits, follow the Read Workflow to
- understand the user's request.
-2. **Find references:** Before modifying the definition of any symbol, use the
- `go_symbol_references` tool to find references to that identifier. These
- references may need to be updated after editing the symbol. Read files
- containing references to evaluate if any further edits are required.
-3. **Make edits:** Make the primary edit, as well as any edits to references.
-4. **Run diagnostics:** Every time, after making edits to one or more files,
- you must call the `go_diagnostics` tool, passing the paths to the edited
- files, to verify that the build is not broken. Apply edits to fix any
- relevant diagnostics, and re-run the `go_diagnostics` tool to verify the
- fixes. It is OK to ignore 'hint' or 'info' diagnostics if they are not
- relevant.
-5. **Run tests** run `go test` for any packages that were edited. Invoke `go
- test` with the package paths returned from `go_file_metadata`. Fix any test
- failures.
+2. **Find relevant symbols**: If you're looking for a specific type, function, or variable, use `go_search`. This is a fuzzy search that will help you locate symbols even if you don't know the exact name or location.
+ EXAMPLE: search for the 'Server' type: `go_search({"query":"server"})`
+
+3. **Understand a file and its intra-package dependencies**: When you have a file path and want to understand its contents and how it connects to other files *in the same package*, use `go_file_context`. This tool will show you a summary of the declarations from other files in the same package that are used by the current file. `go_file_context` MUST be used immediately after reading any Go file for the first time, and MAY be re-used if dependencies have changed.
+ EXAMPLE: to understand `server.go`'s dependencies on other files in its package: `go_file_context({"file":"/path/to/server.go"})`
+
+4. **Understand a package's public API**: When you need to understand what a package provides to external code (i.e., its public API), use `go_package_api`. This is especially useful for understanding third-party dependencies or other packages in the same monorepo.
+ EXAMPLE: to see the API of the `storage` package: `go_package_api({"packagePaths":["example.com/internal/storage"]})`
+
+### Editing workflow
+
+The editing workflow is iterative. You should cycle through these steps until the task is complete.
+
+1. **Read first**: Before making any edits, follow the Read Workflow to understand the user's request and the relevant code.
+
+2. **Find references**: Before modifying the definition of any exported symbol, use the `go_symbol_references` tool to find all references to that identifier. This is critical for understanding the impact of your change. Read the files containing references to evaluate if any further edits are required.
+ EXAMPLE: `go_symbol_references({"file":"/path/to/server.go","symbol":"Server.Run"})`
+
+3. **Make edits**: Make the primary edit, as well as any edits to references you identified in the previous step.
+
+4. **Check for errors**: After every code modification, you MUST call the `go_diagnostics` tool. Pass the paths of the files you have edited. This tool will report any build or analysis errors.
+ EXAMPLE: `go_diagnostics({"files":["/path/to/server.go"]})`
+
+5. **Fix errors**: If `go_diagnostics` reports any errors, fix them. The tool may provide suggested quick fixes in the form of diffs. You should review these diffs and apply them if they are correct. Once you've applied a fix, re-run `go_diagnostics` to confirm that the issue is resolved. It is OK to ignore 'hint' or 'info' diagnostics if they are not relevant to the current task.
+
+6. **Run tests**: Once `go_diagnostics` reports no errors (and ONLY once there are no errors), run the tests for the packages you have changed. You can do this with `go test [packagePath...]`. Don't run `go test ./...` unless the user explicitly requests it, as doing so may slow down the iteration loop.
diff --git a/gopls/internal/mcp/mcp.go b/gopls/internal/mcp/mcp.go
index d00b34b..a396d15 100644
--- a/gopls/internal/mcp/mcp.go
+++ b/gopls/internal/mcp/mcp.go
@@ -161,13 +161,15 @@
defaultTools := []*mcp.ServerTool{
h.workspaceTool(),
- h.fileMetadataTool(),
h.outlineTool(),
h.workspaceDiagnosticsTool(),
h.symbolReferencesTool(),
h.searchTool(),
+ h.fileContextTool(),
}
disabledTools := append(defaultTools,
+ // The fileMetadata tool is redundant with fileContext.
+ h.fileMetadataTool(),
// The context tool returns context for all imports, which can consume a
// lot of tokens. Conservatively, rely on the model selecting the imports
// to summarize using the outline tool.
diff --git a/gopls/internal/mcp/outline.go b/gopls/internal/mcp/outline.go
index 3cf7560..1c7d2d1 100644
--- a/gopls/internal/mcp/outline.go
+++ b/gopls/internal/mcp/outline.go
@@ -19,7 +19,7 @@
func (h *handler) outlineTool() *mcp.ServerTool {
return mcp.NewServerTool(
- "go_package_outline",
+ "go_package_api",
"Provides a summary of a Go package API",
h.outlineHandler,
mcp.Input(
diff --git a/gopls/internal/test/marker/testdata/mcptools/file_context.txt b/gopls/internal/test/marker/testdata/mcptools/file_context.txt
new file mode 100644
index 0000000..9122ed5
--- /dev/null
+++ b/gopls/internal/test/marker/testdata/mcptools/file_context.txt
@@ -0,0 +1,43 @@
+This test exercises the "go_file_context" MCP tool.
+
+-- flags --
+-mcp
+-ignore_extra_diags
+
+-- go.mod --
+module example.com
+
+-- a/main.go --
+package main
+
+import "example.com/a/other"
+
+func main() { //@mcptool("go_file_context", `{"file": "$WORKDIR/a/main.go"}`, output=content)
+ other.Foo()
+ _ = other.Bar
+}
+
+-- a/other/other.go --
+package other
+
+// Foo should have a doc comment.
+func Foo() {
+ // The body should be ignored
+}
+
+var Bar int // line comments get dropped
+
+var Baz string // Baz is not referenced
+
+-- @content --
+File `$WORKDIR/a/main.go` is in package "example.com/a".
+Below is a summary of the APIs it uses from other files.
+To read the full API of any package, use go_package_api.
+Referenced declarations from $WORKDIR/a/other/other.go (package "example.com/a/other"):
+```go
+// Foo should have a doc comment.
+func Foo()
+
+var Bar int
+```
+
diff --git a/gopls/internal/test/marker/testdata/mcptools/file_metadata.txt b/gopls/internal/test/marker/testdata/mcptools/file_metadata.txt
index cbab0fd..ac1fa6f 100644
--- a/gopls/internal/test/marker/testdata/mcptools/file_metadata.txt
+++ b/gopls/internal/test/marker/testdata/mcptools/file_metadata.txt
@@ -3,6 +3,12 @@
-- flags --
-mcp
+-- settings.json --
+{
+ "mcpTools": {
+ "go_file_metadata": true
+ }
+}
-- go.mod --
module example.com/cmd
diff --git a/gopls/internal/test/marker/testdata/mcptools/outline.txt b/gopls/internal/test/marker/testdata/mcptools/package_api.txt
similarity index 74%
rename from gopls/internal/test/marker/testdata/mcptools/outline.txt
rename to gopls/internal/test/marker/testdata/mcptools/package_api.txt
index 42bc5d7..3b0c199 100644
--- a/gopls/internal/test/marker/testdata/mcptools/outline.txt
+++ b/gopls/internal/test/marker/testdata/mcptools/package_api.txt
@@ -1,4 +1,4 @@
-This test exercises the "go_package_outline" MCP tool.
+This test exercises the "go_package_api" MCP tool.
-- flags --
-mcp
@@ -6,7 +6,7 @@
-- go.mod --
module example.com/mod
-//@mcptool("go_package_outline", `{"PackagePaths":["example.com/mod/lib"]}`, output=outline)
+//@mcptool("go_package_api", `{"PackagePaths":["example.com/mod/lib"]}`, output=outline)
go 1.21
