Skip to content

Commit 4dbf51a

Browse files
committed
Fix merged state in pull request list output
1 parent eb47a99 commit 4dbf51a

2 files changed

Lines changed: 81 additions & 0 deletions

File tree

‎pkg/github/minimal_types.go‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1099,6 +1099,11 @@ func convertToMinimalPullRequest(pr *github.PullRequest) MinimalPullRequest {
10991099
Comments: pr.GetComments(),
11001100
}
11011101

1102+
// The list endpoint omits merged but includes merged_at.
1103+
if pr.Merged == nil && pr.MergedAt != nil {
1104+
m.Merged = !pr.MergedAt.IsZero()
1105+
}
1106+
11021107
if pr.CreatedAt != nil {
11031108
m.CreatedAt = pr.CreatedAt.Format(time.RFC3339)
11041109
}

‎pkg/github/pullrequests_test.go‎

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,9 +11,11 @@ import (
1111

1212
"github.com/github/github-mcp-server/v2/internal/githubv4mock"
1313
"github.com/github/github-mcp-server/v2/internal/toolsnaps"
14+
"github.com/github/github-mcp-server/v2/pkg/inventory"
1415
"github.com/github/github-mcp-server/v2/pkg/translations"
1516
"github.com/google/go-github/v92/github"
1617
"github.com/google/jsonschema-go/jsonschema"
18+
"github.com/modelcontextprotocol/go-sdk/mcp"
1719
"github.com/shurcooL/githubv4"
1820
"github.com/stretchr/testify/assert"
1921
"github.com/stretchr/testify/require"
@@ -745,6 +747,80 @@ func Test_ListPullRequests(t *testing.T) {
745747
}
746748
}
747749

750+
func Test_ListPullRequests_MergedState(t *testing.T) {
751+
for _, tc := range []struct {
752+
name string
753+
response string
754+
wantMerged bool
755+
}{
756+
{"merged list entry", `[{"number":42,"state":"closed","merged_at":"2026-10-08T11:25:51Z"}]`, true},
757+
{"closed unmerged", `[{"number":42,"state":"closed","merged_at":null}]`, false},
758+
{"open", `[{"number":42,"state":"open","merged_at":null}]`, false},
759+
{"explicit true", `[{"number":42,"state":"closed","merged":true}]`, true},
760+
{"explicit false", `[{"number":42,"state":"closed","merged":false,"merged_at":"2026-10-08T11:25:51Z"}]`, false},
761+
{"zero timestamp", `[{"number":42,"state":"closed","merged_at":"0001-01-01T00:00:00Z"}]`, false},
762+
} {
763+
for _, fields := range [][]string{nil, {"number", "merged", "merged_at"}} {
764+
name := "all fields"
765+
if fields != nil {
766+
name = "selected fields"
767+
}
768+
t.Run(tc.name+"/"+name, func(t *testing.T) {
769+
mockedClient := MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
770+
GetReposPullsByOwnerByRepo: func(w http.ResponseWriter, _ *http.Request) {
771+
w.Header().Set("Content-Type", "application/json")
772+
_, err := w.Write([]byte(tc.response))
773+
require.NoError(t, err)
774+
},
775+
})
776+
deps := BaseDeps{Client: mustNewGHClient(t, mockedClient)}
777+
serverTool := ListPullRequests(translations.NullTranslationHelper)
778+
handler := serverTool.Handler(deps)
779+
args := map[string]any{"owner": "owner", "repo": "repo", "state": "all"}
780+
if fields != nil {
781+
args["fields"] = fields
782+
}
783+
request := createMCPRequest(args)
784+
result, err := handler(ContextWithDeps(t.Context(), deps), &request)
785+
require.NoError(t, err)
786+
require.False(t, result.IsError)
787+
var textPRs []MinimalPullRequest
788+
require.NoError(t, json.Unmarshal([]byte(getTextResult(t, result).Text), &textPRs))
789+
require.Len(t, textPRs, 1)
790+
assert.Equal(t, tc.wantMerged, textPRs[0].Merged)
791+
792+
server := mcp.NewServer(&mcp.Implementation{Name: "test-server", Version: "v0.0.1"}, nil)
793+
server.AddReceivingMiddleware(InjectDepsMiddleware(deps))
794+
serverTool.RegisterFunc(server, deps)
795+
serverTransport, clientTransport := mcp.NewInMemoryTransports()
796+
serverSession, err := server.Connect(t.Context(), serverTransport, nil)
797+
require.NoError(t, err)
798+
t.Cleanup(func() { _ = serverSession.Close() })
799+
client := mcp.NewClient(&mcp.Implementation{Name: "test-client", Version: "v0.0.1"}, nil)
800+
clientSession, err := client.Connect(t.Context(), clientTransport, &mcp.ClientSessionOptions{
801+
ProtocolVersion: inventory.ProtocolVersionMultiRoundTrip,
802+
})
803+
require.NoError(t, err)
804+
t.Cleanup(func() { _ = clientSession.Close() })
805+
result, err = clientSession.CallTool(t.Context(), &mcp.CallToolParams{
806+
Name: serverTool.Tool.Name,
807+
Arguments: args,
808+
Meta: mcp.Meta{mcp.MetaKeyProtocolVersion: inventory.ProtocolVersionMultiRoundTrip},
809+
})
810+
require.NoError(t, err)
811+
require.False(t, result.IsError)
812+
structured, err := json.Marshal(result.StructuredContent)
813+
require.NoError(t, err)
814+
var typedPRs []ListPullRequestOutput
815+
require.NoError(t, json.Unmarshal(structured, &typedPRs))
816+
require.Len(t, typedPRs, 1)
817+
require.NotNil(t, typedPRs[0].Merged)
818+
assert.Equal(t, tc.wantMerged, *typedPRs[0].Merged)
819+
})
820+
}
821+
}
822+
}
823+
748824
func Test_MergePullRequest(t *testing.T) {
749825
// Verify tool definition once
750826
serverTool := MergePullRequest(translations.NullTranslationHelper)

0 commit comments

Comments
 (0)