Register MCP App UI resources in shared server constructor
Build and Test Go Project / build (macos-latest) (push) Has been cancelled
Build and Test Go Project / build (ubuntu-latest) (push) Has been cancelled
Build and Test Go Project / build (windows-latest) (push) Has been cancelled
CodeQL / Analyze (go) (push) Has been cancelled
CodeQL / Analyze (actions) (push) Has been cancelled
CodeQL / Analyze (javascript) (push) Has been cancelled
Docker / build (push) Has been cancelled
GoReleaser Release / release (push) Has been cancelled
MCP Server Diff / mcp-diff (push) Has been cancelled
MCP Server Diff / mcp-diff-http (push) Has been cancelled
Publish to MCP Registry / publish (push) Has been cancelled
Build and Test Go Project / build (macos-latest) (push) Has been cancelled
Build and Test Go Project / build (ubuntu-latest) (push) Has been cancelled
Build and Test Go Project / build (windows-latest) (push) Has been cancelled
CodeQL / Analyze (go) (push) Has been cancelled
CodeQL / Analyze (actions) (push) Has been cancelled
CodeQL / Analyze (javascript) (push) Has been cancelled
Docker / build (push) Has been cancelled
GoReleaser Release / release (push) Has been cancelled
MCP Server Diff / mcp-diff (push) Has been cancelled
MCP Server Diff / mcp-diff-http (push) Has been cancelled
Publish to MCP Registry / publish (push) Has been cancelled
The remote/HTTP server never called RegisterUIResources, so when the remote_mcp_ui_apps feature flag was enabled per-request, tools like issue_write and create_pull_request would advertise a ui:// resource URI in their _meta.ui block but the resource itself was not registered. The client's follow-up resources/read call then failed with -32002 'Resource not found' (the error surfaced as 'Error loading MCP App: MPC -32002: Resource not found' in VS Code). The stdio bootstrap also gated registration on featureChecker called with context.Background(), which can't see per-request flag overrides. Move RegisterUIResources into pkg/github.NewMCPServer (the shared constructor used by both stdio and HTTP), gated only on UIAssetsAvailable(). The resources are inert static HTML; the inventory still strips _meta.ui from tools per-request via stripMCPAppsMetadata, so the URI is only advertised to clients when the flag is on for that request. Fixes #2467 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This commit is contained in:
@@ -173,15 +173,6 @@ func NewStdioMCPServer(ctx context.Context, cfg github.MCPServerConfig) (*mcp.Se
|
||||
return nil, fmt.Errorf("failed to create GitHub MCP server: %w", err)
|
||||
}
|
||||
|
||||
// Register MCP App UI resources if the remote_mcp_ui_apps feature flag is enabled
|
||||
// and UI assets are available (requires running script/build-ui).
|
||||
// We check availability to allow the feature flag to be enabled without
|
||||
// requiring a UI build (graceful degradation).
|
||||
mcpAppsEnabled, _ := featureChecker(context.Background(), github.MCPAppsFeatureFlag)
|
||||
if mcpAppsEnabled && github.UIAssetsAvailable() {
|
||||
github.RegisterUIResources(ghServer)
|
||||
}
|
||||
|
||||
ghServer.AddReceivingMiddleware(addUserAgentsMiddleware(cfg, clients.restUATransp, clients.gqlHTTP))
|
||||
|
||||
return ghServer, nil
|
||||
|
||||
@@ -102,6 +102,18 @@ func NewMCPServer(ctx context.Context, cfg *MCPServerConfig, deps ToolDependenci
|
||||
// Register GitHub tools/resources/prompts from the inventory.
|
||||
inv.RegisterAll(ctx, ghServer, deps)
|
||||
|
||||
// Register MCP App UI resources whenever the embedded UI assets are
|
||||
// available. The resources are static HTML and are only referenced by
|
||||
// tools when the remote_mcp_ui_apps feature flag is enabled for the
|
||||
// request (the inventory strips the _meta.ui block otherwise via
|
||||
// stripMCPAppsMetadata), so registering them unconditionally is safe.
|
||||
// Registering here — rather than in the stdio bootstrap — ensures the
|
||||
// remote/HTTP server also serves them, fixing the "-32002 Resource not
|
||||
// found" error clients hit after the tool returns a ui:// URI.
|
||||
if UIAssetsAvailable() {
|
||||
RegisterUIResources(ghServer)
|
||||
}
|
||||
|
||||
return ghServer, nil
|
||||
}
|
||||
|
||||
|
||||
@@ -0,0 +1,123 @@
|
||||
package github
|
||||
|
||||
import (
|
||||
"context"
|
||||
"testing"
|
||||
|
||||
"github.com/github/github-mcp-server/pkg/inventory"
|
||||
"github.com/modelcontextprotocol/go-sdk/mcp"
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
)
|
||||
|
||||
// TestRegisterUIResources_ReadableViaClient verifies that each UI resource URI
|
||||
// advertised by an MCP App-enabled tool (e.g. issue_write, create_pull_request,
|
||||
// get_me) actually resolves to a registered resource on the server.
|
||||
//
|
||||
// Regression test for the "Error loading MCP App: MPC -32002: Resource not
|
||||
// found" bug reported in issue #2467, where the HTTP/remote server returned a
|
||||
// resource URI in the tool's _meta.ui block but never registered the matching
|
||||
// resource — so the follow-up resources/read call from the client failed.
|
||||
func TestRegisterUIResources_ReadableViaClient(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
if !UIAssetsAvailable() {
|
||||
t.Skip("UI assets not built; run script/build-ui to enable this test")
|
||||
}
|
||||
|
||||
srv := mcp.NewServer(&mcp.Implementation{Name: "test", Version: "0.0.1"}, nil)
|
||||
RegisterUIResources(srv)
|
||||
|
||||
// Connect an in-memory client/server pair and read each advertised URI.
|
||||
st, ct := mcp.NewInMemoryTransports()
|
||||
|
||||
type clientResult struct {
|
||||
session *mcp.ClientSession
|
||||
err error
|
||||
}
|
||||
clientCh := make(chan clientResult, 1)
|
||||
go func() {
|
||||
client := mcp.NewClient(&mcp.Implementation{Name: "test-client"}, nil)
|
||||
cs, err := client.Connect(context.Background(), ct, nil)
|
||||
clientCh <- clientResult{session: cs, err: err}
|
||||
}()
|
||||
|
||||
ss, err := srv.Connect(context.Background(), st, nil)
|
||||
require.NoError(t, err)
|
||||
t.Cleanup(func() { _ = ss.Close() })
|
||||
|
||||
got := <-clientCh
|
||||
require.NoError(t, got.err)
|
||||
t.Cleanup(func() { _ = got.session.Close() })
|
||||
|
||||
uris := []string{
|
||||
GetMeUIResourceURI,
|
||||
IssueWriteUIResourceURI,
|
||||
PullRequestWriteUIResourceURI,
|
||||
}
|
||||
for _, uri := range uris {
|
||||
t.Run(uri, func(t *testing.T) {
|
||||
res, err := got.session.ReadResource(context.Background(), &mcp.ReadResourceParams{URI: uri})
|
||||
require.NoError(t, err, "resource %s should be registered (got -32002 means it isn't)", uri)
|
||||
require.NotNil(t, res)
|
||||
require.NotEmpty(t, res.Contents)
|
||||
assert.Equal(t, uri, res.Contents[0].URI)
|
||||
assert.Equal(t, MCPAppMIMEType, res.Contents[0].MIMEType)
|
||||
assert.NotEmpty(t, res.Contents[0].Text, "UI resource should return HTML body")
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// TestNewMCPServer_RegistersUIResources verifies that NewMCPServer — the
|
||||
// shared constructor used by both the stdio and HTTP entry points — registers
|
||||
// the UI resources when UI assets are embedded. Previously this registration
|
||||
// only happened in the stdio bootstrap, so remote/HTTP clients hit -32002.
|
||||
func TestNewMCPServer_RegistersUIResources(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
if !UIAssetsAvailable() {
|
||||
t.Skip("UI assets not built; run script/build-ui to enable this test")
|
||||
}
|
||||
|
||||
srv, err := NewMCPServer(context.Background(), &MCPServerConfig{
|
||||
Version: "test",
|
||||
Translator: stubTranslator,
|
||||
}, stubDeps{t: stubTranslator}, mustEmptyInventory(t))
|
||||
require.NoError(t, err)
|
||||
|
||||
st, ct := mcp.NewInMemoryTransports()
|
||||
|
||||
type clientResult struct {
|
||||
session *mcp.ClientSession
|
||||
err error
|
||||
}
|
||||
clientCh := make(chan clientResult, 1)
|
||||
go func() {
|
||||
client := mcp.NewClient(&mcp.Implementation{Name: "test-client"}, nil)
|
||||
cs, err := client.Connect(context.Background(), ct, nil)
|
||||
clientCh <- clientResult{session: cs, err: err}
|
||||
}()
|
||||
|
||||
ss, err := srv.Connect(context.Background(), st, nil)
|
||||
require.NoError(t, err)
|
||||
t.Cleanup(func() { _ = ss.Close() })
|
||||
|
||||
got := <-clientCh
|
||||
require.NoError(t, got.err)
|
||||
t.Cleanup(func() { _ = got.session.Close() })
|
||||
|
||||
res, err := got.session.ReadResource(context.Background(), &mcp.ReadResourceParams{URI: IssueWriteUIResourceURI})
|
||||
require.NoError(t, err)
|
||||
require.NotNil(t, res)
|
||||
require.NotEmpty(t, res.Contents)
|
||||
assert.Equal(t, MCPAppMIMEType, res.Contents[0].MIMEType)
|
||||
}
|
||||
|
||||
// mustEmptyInventory builds an empty inventory for tests that only care about
|
||||
// resources/prompts registered outside the inventory (such as the UI resources).
|
||||
func mustEmptyInventory(t *testing.T) *inventory.Inventory {
|
||||
t.Helper()
|
||||
inv, err := NewInventory(stubTranslator).WithToolsets([]string{}).Build()
|
||||
require.NoError(t, err)
|
||||
return inv
|
||||
}
|
||||
Reference in New Issue
Block a user