Skip to content

Commit 99d6d65

Browse files
omgitsadsCopilot
omgitsads
and
Copilot
authored
fix: return non-nil empty completion results (#3665)
* fix: return non-nil empty completion results Return an empty values array for prompt and non-resource completions to prevent nil result finalization panics. Add regression coverage for serialization, resource routing, and errors. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> * fix: normalize empty resource completions Ensure successful repository resource completion responses serialize empty values as an array when a resolver has no matches. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
1 parent 4bcdcdf commit 99d6d65

4 files changed

Lines changed: 111 additions & 8 deletions

File tree

‎pkg/github/repository_resource_completions.go‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,9 @@ var RepositoryResourceArgumentResolvers = map[string]CompleteHandler{
2828
func RepositoryResourceCompletionHandler(getClient GetClientFn) func(ctx context.Context, req *mcp.CompleteRequest) (*mcp.CompleteResult, error) {
2929
return func(ctx context.Context, req *mcp.CompleteRequest) (*mcp.CompleteResult, error) {
3030
if req.Params.Ref.Type != "ref/resource" {
31-
return nil, nil // Not a resource completion
31+
return &mcp.CompleteResult{
32+
Completion: mcp.CompletionResultDetails{Values: []string{}},
33+
}, nil
3234
}
3335

3436
argName := req.Params.Argument.Name
@@ -57,6 +59,9 @@ func RepositoryResourceCompletionHandler(getClient GetClientFn) func(ctx context
5759
if err != nil {
5860
return nil, err
5961
}
62+
if values == nil {
63+
values = []string{}
64+
}
6065
if len(values) > 100 {
6166
values = values[:100]
6267
}

‎pkg/github/repository_resource_completions_test.go‎

Lines changed: 47 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package github
22

33
import (
44
"context"
5+
"encoding/json"
56
"errors"
67
"fmt"
78
"testing"
@@ -20,16 +21,18 @@ func TestRepositoryResourceCompletionHandler(t *testing.T) {
2021
wantErr bool
2122
}{
2223
{
23-
name: "non-resource completion request",
24+
name: "prompt completion request",
2425
request: &mcp.CompleteRequest{
2526
Params: &mcp.CompleteParams{
2627
Ref: &mcp.CompleteReference{
27-
Type: "something-else",
28+
Type: "ref/prompt",
2829
},
2930
},
3031
},
31-
expected: nil,
32-
wantErr: false,
32+
expected: &mcp.CompleteResult{
33+
Completion: mcp.CompletionResultDetails{Values: []string{}},
34+
},
35+
wantErr: false,
3336
},
3437
{
3538
name: "invalid ref type",
@@ -40,8 +43,10 @@ func TestRepositoryResourceCompletionHandler(t *testing.T) {
4043
},
4144
},
4245
},
43-
expected: nil,
44-
wantErr: false,
46+
expected: &mcp.CompleteResult{
47+
Completion: mcp.CompletionResultDetails{Values: []string{}},
48+
},
49+
wantErr: false,
4550
},
4651
{
4752
name: "unknown argument",
@@ -65,6 +70,7 @@ func TestRepositoryResourceCompletionHandler(t *testing.T) {
6570
for _, tt := range tests {
6671
t.Run(tt.name, func(t *testing.T) {
6772
getClient := func(_ context.Context) (*github.Client, error) {
73+
assert.Equal(t, "ref/resource", tt.request.Params.Ref.Type, "non-resource completions must not fetch a client")
6874
return &github.Client{}, nil
6975
}
7076

@@ -77,10 +83,45 @@ func TestRepositoryResourceCompletionHandler(t *testing.T) {
7783
require.NoError(t, err)
7884
}
7985
assert.Equal(t, tt.expected, result)
86+
if !tt.wantErr {
87+
require.NotNil(t, result)
88+
data, err := json.Marshal(result)
89+
require.NoError(t, err)
90+
assert.JSONEq(t, `{"completion":{"values":[]}}`, string(data))
91+
}
8092
})
8193
}
8294
}
8395

96+
func TestRepositoryResourceCompletionHandler_EmptyResults(t *testing.T) {
97+
const argumentName = "owner"
98+
originalResolver := RepositoryResourceArgumentResolvers[argumentName]
99+
RepositoryResourceArgumentResolvers[argumentName] = func(context.Context, *github.Client, map[string]string, string) ([]string, error) {
100+
return nil, nil
101+
}
102+
t.Cleanup(func() {
103+
RepositoryResourceArgumentResolvers[argumentName] = originalResolver
104+
})
105+
106+
handler := RepositoryResourceCompletionHandler(func(context.Context) (*github.Client, error) {
107+
return &github.Client{}, nil
108+
})
109+
result, err := handler(t.Context(), &mcp.CompleteRequest{
110+
Params: &mcp.CompleteParams{
111+
Ref: &mcp.CompleteReference{Type: "ref/resource"},
112+
Argument: mcp.CompleteParamsArgument{Name: argumentName},
113+
},
114+
})
115+
116+
require.NoError(t, err)
117+
require.NotNil(t, result)
118+
assert.Equal(t, []string{}, result.Completion.Values)
119+
assert.Equal(t, 0, result.Completion.Total)
120+
data, err := json.Marshal(result)
121+
require.NoError(t, err)
122+
assert.JSONEq(t, `{"completion":{"values":[]}}`, string(data))
123+
}
124+
84125
func TestRepositoryResourceCompletionHandler_GetClientError(t *testing.T) {
85126
getClient := func(_ context.Context) (*github.Client, error) {
86127
return nil, errors.New("client error")

‎pkg/github/server.go‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -212,7 +212,9 @@ func CompletionsHandler(getClient GetClientFn) func(ctx context.Context, req *mc
212212
}
213213
return nil, fmt.Errorf("unsupported resource URI: %s", req.Params.Ref.URI)
214214
case "ref/prompt":
215-
return nil, nil
215+
return &mcp.CompleteResult{
216+
Completion: mcp.CompletionResultDetails{Values: []string{}},
217+
}, nil
216218
default:
217219
return nil, fmt.Errorf("unsupported ref type: %s", req.Params.Ref.Type)
218220
}

‎pkg/github/server_test.go‎

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -528,3 +528,58 @@ func TestCompletionsHandler_RejectsMissingRef(t *testing.T) {
528528
})
529529
}
530530
}
531+
532+
func TestCompletionsHandler(t *testing.T) {
533+
clientErr := errors.New("client error")
534+
tests := []struct {
535+
name string
536+
ref *mcp.CompleteReference
537+
wantErr string
538+
wantClient bool
539+
}{
540+
{
541+
name: "prompt completion",
542+
ref: &mcp.CompleteReference{Type: "ref/prompt"},
543+
},
544+
{
545+
name: "repository resource completion",
546+
ref: &mcp.CompleteReference{Type: "ref/resource", URI: "repo://{owner}/{repo}/contents/{path}"},
547+
wantErr: clientErr.Error(),
548+
wantClient: true,
549+
},
550+
{
551+
name: "unsupported resource URI",
552+
ref: &mcp.CompleteReference{Type: "ref/resource", URI: "unsupported://resource"},
553+
wantErr: "unsupported resource URI: unsupported://resource",
554+
},
555+
{
556+
name: "unsupported ref type",
557+
ref: &mcp.CompleteReference{Type: "unsupported"},
558+
wantErr: "unsupported ref type: unsupported",
559+
},
560+
}
561+
for _, tt := range tests {
562+
t.Run(tt.name, func(t *testing.T) {
563+
clientCalled := false
564+
handler := CompletionsHandler(func(context.Context) (*gogithub.Client, error) {
565+
clientCalled = true
566+
return nil, clientErr
567+
})
568+
result, err := handler(t.Context(), &mcp.CompleteRequest{
569+
Params: &mcp.CompleteParams{Ref: tt.ref},
570+
})
571+
assert.Equal(t, tt.wantClient, clientCalled)
572+
if tt.wantErr != "" {
573+
require.EqualError(t, err, tt.wantErr)
574+
assert.Nil(t, result)
575+
return
576+
}
577+
require.NoError(t, err)
578+
require.NotNil(t, result)
579+
assert.Equal(t, []string{}, result.Completion.Values)
580+
data, err := json.Marshal(result)
581+
require.NoError(t, err)
582+
assert.JSONEq(t, `{"completion":{"values":[]}}`, string(data))
583+
})
584+
}
585+
}

0 commit comments

Comments
 (0)