Skip to content

Commit 7965b35

Browse files
Hashim1999164SamMorrowDrums
authored andcommitted
Show nested GitHub API validation messages in tool errors
create_branch currently forwards the compact 422 dump, which hides ruleset details the GitHub UI already shows. Unwrap ErrorResponse so agents can see each validation message and recover.
1 parent 8ec6249 commit 7965b35

3 files changed

Lines changed: 128 additions & 1 deletion

File tree

pkg/errors/error.go

Lines changed: 51 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import (
66
stderrors "errors"
77
"fmt"
88
"net/http"
9+
"strings"
910
"time"
1011

1112
"github.com/github/github-mcp-server/pkg/utils"
@@ -191,7 +192,56 @@ func NewGitHubAPIErrorResponse(ctx context.Context, message string, resp *github
191192
"%s: GitHub secondary rate limit exceeded. Wait before retrying.", message))
192193
}
193194

194-
return utils.NewToolResultErrorFromErr(message, err)
195+
return utils.NewToolResultErrorFromErr(message, formattedGitHubAPIError(err))
196+
}
197+
198+
// formattedGitHubAPIError unwraps a github.ErrorResponse so tool results include
199+
// nested validation messages (for example repository ruleset violations) instead
200+
// of go-github's compact 422 dump.
201+
func formattedGitHubAPIError(err error) error {
202+
var ghErr *github.ErrorResponse
203+
if !stderrors.As(err, &ghErr) {
204+
return err
205+
}
206+
207+
var parts []string
208+
switch {
209+
case ghErr.Response != nil && ghErr.Response.StatusCode != 0 && ghErr.Message != "":
210+
parts = append(parts, fmt.Sprintf("HTTP %d %s", ghErr.Response.StatusCode, ghErr.Message))
211+
case ghErr.Response != nil && ghErr.Response.StatusCode != 0:
212+
parts = append(parts, fmt.Sprintf("HTTP %d", ghErr.Response.StatusCode))
213+
case ghErr.Message != "":
214+
parts = append(parts, ghErr.Message)
215+
}
216+
217+
for _, item := range ghErr.Errors {
218+
detail := strings.TrimSpace(item.Message)
219+
if detail == "" {
220+
var bits []string
221+
if item.Resource != "" {
222+
bits = append(bits, item.Resource)
223+
}
224+
if item.Field != "" {
225+
bits = append(bits, item.Field)
226+
}
227+
if item.Code != "" {
228+
bits = append(bits, item.Code)
229+
}
230+
detail = strings.Join(bits, " ")
231+
}
232+
if detail != "" {
233+
parts = append(parts, detail)
234+
}
235+
}
236+
237+
if ghErr.DocumentationURL != "" {
238+
parts = append(parts, "See "+ghErr.DocumentationURL)
239+
}
240+
241+
if len(parts) == 0 {
242+
return err
243+
}
244+
return stderrors.New(strings.Join(parts, "\n"))
195245
}
196246

197247
// NewGitHubGraphQLErrorResponse returns an mcp.NewToolResultError and retains the error in the context for access via middleware

pkg/errors/error_test.go

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -687,3 +687,50 @@ func TestNewGitHubAPIErrorResponse_RateLimits(t *testing.T) {
687687
assert.Contains(t, text, "validation failed")
688688
})
689689
}
690+
691+
func TestNewGitHubAPIErrorResponse_ValidationMessages(t *testing.T) {
692+
t.Run("ruleset ErrorResponse includes nested validation messages", func(t *testing.T) {
693+
ctx := ContextWithGitHubErrors(context.Background())
694+
695+
originalErr := &github.ErrorResponse{
696+
Response: &http.Response{StatusCode: http.StatusUnprocessableEntity},
697+
Message: "Validation Failed",
698+
Errors: []github.Error{
699+
{
700+
Resource: "GitRef",
701+
Field: "ref",
702+
Code: "custom",
703+
Message: "ref name does not match the required pattern 'feature/*'",
704+
},
705+
},
706+
DocumentationURL: "https://docs.github.com/rest/git/refs#create-a-reference",
707+
}
708+
709+
result := NewGitHubAPIErrorResponse(ctx, "failed to create branch", nil, originalErr)
710+
711+
text := requireErrorText(t, result)
712+
assert.Contains(t, text, "failed to create branch")
713+
assert.Contains(t, text, "HTTP 422 Validation Failed")
714+
assert.Contains(t, text, "ref name does not match the required pattern 'feature/*'")
715+
assert.Contains(t, text, "See https://docs.github.com/rest/git/refs#create-a-reference")
716+
assert.NotContains(t, text, "Resource:")
717+
})
718+
719+
t.Run("wrapped ErrorResponse is still unwrapped", func(t *testing.T) {
720+
ctx := ContextWithGitHubErrors(context.Background())
721+
722+
originalErr := fmt.Errorf("create ref: %w", &github.ErrorResponse{
723+
Response: &http.Response{StatusCode: http.StatusUnprocessableEntity},
724+
Message: "Validation Failed",
725+
Errors: []github.Error{
726+
{Message: "Changes must be made through a pull request."},
727+
},
728+
})
729+
730+
result := NewGitHubAPIErrorResponse(ctx, "failed to create branch", nil, originalErr)
731+
732+
text := requireErrorText(t, result)
733+
assert.Contains(t, text, "Changes must be made through a pull request.")
734+
assert.NotContains(t, text, "create ref:")
735+
})
736+
}

pkg/github/repositories_test.go

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1098,6 +1098,36 @@ func Test_CreateBranch(t *testing.T) {
10981098
expectError: true,
10991099
expectedErrMsg: "failed to create branch",
11001100
},
1101+
{
1102+
name: "create branch surfaces ruleset validation details",
1103+
mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
1104+
GetReposGitRefByOwnerByRepoByRef: mockResponse(t, http.StatusOK, mockSourceRef),
1105+
"GET /repos/owner/repo/git/ref/heads/main": mockResponse(t, http.StatusOK, mockSourceRef),
1106+
PostReposGitRefsByOwnerByRepo: func(w http.ResponseWriter, _ *http.Request) {
1107+
w.WriteHeader(http.StatusUnprocessableEntity)
1108+
_, _ = w.Write([]byte(`{
1109+
"message": "Validation Failed",
1110+
"documentation_url": "https://docs.github.com/rest/git/refs#create-a-reference",
1111+
"errors": [
1112+
{
1113+
"resource": "GitRef",
1114+
"field": "ref",
1115+
"code": "custom",
1116+
"message": "ref name does not match the required pattern 'feature/*'"
1117+
}
1118+
]
1119+
}`))
1120+
},
1121+
}),
1122+
requestArgs: map[string]any{
1123+
"owner": "owner",
1124+
"repo": "repo",
1125+
"branch": "hotfix",
1126+
"from_branch": "main",
1127+
},
1128+
expectError: true,
1129+
expectedErrMsg: "ref name does not match the required pattern 'feature/*'",
1130+
},
11011131
}
11021132

11031133
for _, tc := range tests {

0 commit comments

Comments
 (0)