Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 0 additions & 9 deletions internal/ghmcp/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
12 changes: 12 additions & 0 deletions pkg/github/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Comment on lines +113 to +115

return ghServer, nil
}

Expand Down
123 changes: 123 additions & 0 deletions pkg/github/ui_resources_test.go
Original file line number Diff line number Diff line change
@@ -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
}
Loading