Skip to content

Commit 44b24ed

Browse files
lpcoxlpcox
authored
fix: restore method-style log prefix in listMCPItems (#3471)
The generic `listMCPItems` helper introduced a log prefix format change from `listTools:` to `list tools:` (space-separated), breaking grep/alert compatibility with existing log consumers. - Removed the space in the format string to produce `list%s:` instead of `list %s:`, consistent with `paginateAll`'s existing log format on the same code path > [!WARNING] > > <details> > <summary>Firewall rules blocked me from connecting to one or more addresses (expand for details)</summary> > > #### I tried to connect to the following addresses, but was blocked by firewall rules: > > - `example.com` > - Triggering command: `/tmp/go-build558144745/b514/launcher.test /tmp/go-build558144745/b514/launcher.test -test.testlogfile=/tmp/go-build558144745/b514/testlog.txt -test.paniconexit0 -test.timeout=10m0s -o .cfg AkSVOkciX x_amd64/vet -p mime -lang=go1.25 x_amd64/vet .cfg�� 7174774/b400/_pkg_.a 64/src/internal/sysinfo/cpuinfo_linux.go x_amd64/vet /tmp/go-build114/usr/libexec/docker/cli-plugins/docker-compose t/transform x86_64-linux-gnu x_amd64/vet` (dns block) > - `invalid-host-that-does-not-exist-12345.com` > - Triggering command: `/tmp/go-build558144745/b496/config.test /tmp/go-build558144745/b496/config.test -test.testlogfile=/tmp/go-build558144745/b496/testlog.txt -test.paniconexit0 -test.timeout=10m0s /tmp/go-build558144745/b385/vet.cfg 5.0/deviceauth.go 5.0/oauth2.go x_amd64/vet --gdwarf-5 --64 -o x_amd64/vet 7174�� g_.a ache/go/1.25.8/x64/src/log/slog/internal/buffer/-nolocalimports x_amd64/vet --gdwarf-5 go-sdk/auth -o x_amd64/vet` (dns block) > - `nonexistent.local` > - Triggering command: `/tmp/go-build558144745/b514/launcher.test /tmp/go-build558144745/b514/launcher.test -test.testlogfile=/tmp/go-build558144745/b514/testlog.txt -test.paniconexit0 -test.timeout=10m0s -o .cfg AkSVOkciX x_amd64/vet -p mime -lang=go1.25 x_amd64/vet .cfg�� 7174774/b400/_pkg_.a 64/src/internal/sysinfo/cpuinfo_linux.go x_amd64/vet /tmp/go-build114/usr/libexec/docker/cli-plugins/docker-compose t/transform x86_64-linux-gnu x_amd64/vet` (dns block) > - `slow.example.com` > - Triggering command: `/tmp/go-build558144745/b514/launcher.test /tmp/go-build558144745/b514/launcher.test -test.testlogfile=/tmp/go-build558144745/b514/testlog.txt -test.paniconexit0 -test.timeout=10m0s -o .cfg AkSVOkciX x_amd64/vet -p mime -lang=go1.25 x_amd64/vet .cfg�� 7174774/b400/_pkg_.a 64/src/internal/sysinfo/cpuinfo_linux.go x_amd64/vet /tmp/go-build114/usr/libexec/docker/cli-plugins/docker-compose t/transform x86_64-linux-gnu x_amd64/vet` (dns block) > - `this-host-does-not-exist-12345.com` > - Triggering command: `/tmp/go-build558144745/b523/mcp.test /tmp/go-build558144745/b523/mcp.test -test.testlogfile=/tmp/go-build558144745/b523/testlog.txt -test.paniconexit0 -test.timeout=10m0s -I .cfg olang.org/grpc@v1.80.0/balancer_wrapper.go x_amd64/vet --gdwarf-5 g/grpc/internal/info -o x_amd64/vet .cfg�� mQkz/1HfPzv0pEfa2EMuomQkz -trimpath x_amd64/vet -p 7174774/b468/ -lang=go1.16 x_amd64/vet` (dns block) > > If you need me to access, download, or install something from one of these locations, you can either: > > - Configure [Actions setup steps](https://gh.io/copilot/actions-setup-steps) to set up my environment, which run before the firewall is enabled > - Add the appropriate URLs or hosts to the custom allowlist in this repository's [Copilot coding agent settings](https://github.com/github/gh-aw-mcpg/settings/copilot/coding_agent) (admins only) > > </details>
2 parents 81fdf1e + 6b264d1 commit 44b24ed

1 file changed

Lines changed: 52 additions & 40 deletions

File tree

‎internal/mcp/connection.go‎

Lines changed: 52 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -582,22 +582,40 @@ func paginateAll[T any](
582582
return all, nil
583583
}
584584

585-
func (c *Connection) listTools() (*Response, error) {
585+
// listMCPItems is a generic helper for the list* family of MCP operations.
586+
// It handles session validation, logging, pagination, and response marshalling,
587+
// eliminating the boilerplate that was previously duplicated across listTools,
588+
// listResources, and listPrompts.
589+
func listMCPItems[Item any, Result any](
590+
c *Connection,
591+
kind string,
592+
fetchPage func(cursor string) (paginatedPage[Item], error),
593+
buildResult func([]Item) Result,
594+
) (*Response, error) {
586595
if err := c.requireSession(); err != nil {
587596
return nil, err
588597
}
589-
logConn.Printf("listTools: requesting tool list from backend serverID=%s", c.serverID)
590-
tools, err := paginateAll(c.serverID, "tools", func(cursor string) (paginatedPage[*sdk.Tool], error) {
591-
result, err := c.getSDKSession().ListTools(c.ctx, &sdk.ListToolsParams{Cursor: cursor})
592-
if err != nil {
593-
return paginatedPage[*sdk.Tool]{}, err
594-
}
595-
return paginatedPage[*sdk.Tool]{Items: result.Tools, NextCursor: result.NextCursor}, nil
596-
})
598+
logConn.Printf("list%s: requesting %s list from backend serverID=%s", kind, kind, c.serverID)
599+
items, err := paginateAll(c.serverID, kind, fetchPage)
597600
if err != nil {
598601
return nil, err
599602
}
600-
return marshalToResponse(&sdk.ListToolsResult{Tools: tools})
603+
return marshalToResponse(buildResult(items))
604+
}
605+
606+
func (c *Connection) listTools() (*Response, error) {
607+
return listMCPItems(c, "tools",
608+
func(cursor string) (paginatedPage[*sdk.Tool], error) {
609+
result, err := c.getSDKSession().ListTools(c.ctx, &sdk.ListToolsParams{Cursor: cursor})
610+
if err != nil {
611+
return paginatedPage[*sdk.Tool]{}, err
612+
}
613+
return paginatedPage[*sdk.Tool]{Items: result.Tools, NextCursor: result.NextCursor}, nil
614+
},
615+
func(items []*sdk.Tool) *sdk.ListToolsResult {
616+
return &sdk.ListToolsResult{Tools: items}
617+
},
618+
)
601619
}
602620

603621
func (c *Connection) callTool(params interface{}) (*Response, error) {
@@ -616,21 +634,18 @@ func (c *Connection) callTool(params interface{}) (*Response, error) {
616634
}
617635

618636
func (c *Connection) listResources() (*Response, error) {
619-
if err := c.requireSession(); err != nil {
620-
return nil, err
621-
}
622-
logConn.Printf("listResources: requesting resource list from backend serverID=%s", c.serverID)
623-
resources, err := paginateAll(c.serverID, "resources", func(cursor string) (paginatedPage[*sdk.Resource], error) {
624-
result, err := c.getSDKSession().ListResources(c.ctx, &sdk.ListResourcesParams{Cursor: cursor})
625-
if err != nil {
626-
return paginatedPage[*sdk.Resource]{}, err
627-
}
628-
return paginatedPage[*sdk.Resource]{Items: result.Resources, NextCursor: result.NextCursor}, nil
629-
})
630-
if err != nil {
631-
return nil, err
632-
}
633-
return marshalToResponse(&sdk.ListResourcesResult{Resources: resources})
637+
return listMCPItems(c, "resources",
638+
func(cursor string) (paginatedPage[*sdk.Resource], error) {
639+
result, err := c.getSDKSession().ListResources(c.ctx, &sdk.ListResourcesParams{Cursor: cursor})
640+
if err != nil {
641+
return paginatedPage[*sdk.Resource]{}, err
642+
}
643+
return paginatedPage[*sdk.Resource]{Items: result.Resources, NextCursor: result.NextCursor}, nil
644+
},
645+
func(items []*sdk.Resource) *sdk.ListResourcesResult {
646+
return &sdk.ListResourcesResult{Resources: items}
647+
},
648+
)
634649
}
635650

636651
func (c *Connection) readResource(params interface{}) (*Response, error) {
@@ -646,21 +661,18 @@ func (c *Connection) readResource(params interface{}) (*Response, error) {
646661
}
647662

648663
func (c *Connection) listPrompts() (*Response, error) {
649-
if err := c.requireSession(); err != nil {
650-
return nil, err
651-
}
652-
logConn.Printf("listPrompts: requesting prompt list from backend serverID=%s", c.serverID)
653-
prompts, err := paginateAll(c.serverID, "prompts", func(cursor string) (paginatedPage[*sdk.Prompt], error) {
654-
result, err := c.getSDKSession().ListPrompts(c.ctx, &sdk.ListPromptsParams{Cursor: cursor})
655-
if err != nil {
656-
return paginatedPage[*sdk.Prompt]{}, err
657-
}
658-
return paginatedPage[*sdk.Prompt]{Items: result.Prompts, NextCursor: result.NextCursor}, nil
659-
})
660-
if err != nil {
661-
return nil, err
662-
}
663-
return marshalToResponse(&sdk.ListPromptsResult{Prompts: prompts})
664+
return listMCPItems(c, "prompts",
665+
func(cursor string) (paginatedPage[*sdk.Prompt], error) {
666+
result, err := c.getSDKSession().ListPrompts(c.ctx, &sdk.ListPromptsParams{Cursor: cursor})
667+
if err != nil {
668+
return paginatedPage[*sdk.Prompt]{}, err
669+
}
670+
return paginatedPage[*sdk.Prompt]{Items: result.Prompts, NextCursor: result.NextCursor}, nil
671+
},
672+
func(items []*sdk.Prompt) *sdk.ListPromptsResult {
673+
return &sdk.ListPromptsResult{Prompts: items}
674+
},
675+
)
664676
}
665677

666678
func (c *Connection) getPrompt(params interface{}) (*Response, error) {

0 commit comments

Comments
 (0)