From 8c3920524d52372c6b1cde4cda928bf4c1f858a0 Mon Sep 17 00:00:00 2001 From: Hashim1999164 <64767361+Hashim1999164@users.noreply.github.com> Date: Sun, 16 Aug 2026 17:16:35 +0500 Subject: [PATCH] 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. --- pkg/errors/error.go | 52 ++++++++++++++++++++++++++++++++- pkg/errors/error_test.go | 47 +++++++++++++++++++++++++++++ pkg/github/repositories_test.go | 30 +++++++++++++++++++ 3 files changed, 128 insertions(+), 1 deletion(-) diff --git a/pkg/errors/error.go b/pkg/errors/error.go index cb4e8b1f0f..205da46ced 100644 --- a/pkg/errors/error.go +++ b/pkg/errors/error.go @@ -6,6 +6,7 @@ import ( stderrors "errors" "fmt" "net/http" + "strings" "time" "github.com/github/github-mcp-server/pkg/utils" @@ -191,7 +192,56 @@ func NewGitHubAPIErrorResponse(ctx context.Context, message string, resp *github "%s: GitHub secondary rate limit exceeded. Wait before retrying.", message)) } - return utils.NewToolResultErrorFromErr(message, err) + return utils.NewToolResultErrorFromErr(message, formattedGitHubAPIError(err)) +} + +// formattedGitHubAPIError unwraps a github.ErrorResponse so tool results include +// nested validation messages (for example repository ruleset violations) instead +// of go-github's compact 422 dump. +func formattedGitHubAPIError(err error) error { + var ghErr *github.ErrorResponse + if !stderrors.As(err, &ghErr) { + return err + } + + var parts []string + switch { + case ghErr.Response != nil && ghErr.Response.StatusCode != 0 && ghErr.Message != "": + parts = append(parts, fmt.Sprintf("HTTP %d %s", ghErr.Response.StatusCode, ghErr.Message)) + case ghErr.Response != nil && ghErr.Response.StatusCode != 0: + parts = append(parts, fmt.Sprintf("HTTP %d", ghErr.Response.StatusCode)) + case ghErr.Message != "": + parts = append(parts, ghErr.Message) + } + + for _, item := range ghErr.Errors { + detail := strings.TrimSpace(item.Message) + if detail == "" { + var bits []string + if item.Resource != "" { + bits = append(bits, item.Resource) + } + if item.Field != "" { + bits = append(bits, item.Field) + } + if item.Code != "" { + bits = append(bits, item.Code) + } + detail = strings.Join(bits, " ") + } + if detail != "" { + parts = append(parts, detail) + } + } + + if ghErr.DocumentationURL != "" { + parts = append(parts, "See "+ghErr.DocumentationURL) + } + + if len(parts) == 0 { + return err + } + return stderrors.New(strings.Join(parts, "\n")) } // NewGitHubGraphQLErrorResponse returns an mcp.NewToolResultError and retains the error in the context for access via middleware diff --git a/pkg/errors/error_test.go b/pkg/errors/error_test.go index 414b7008f7..c16556ea9a 100644 --- a/pkg/errors/error_test.go +++ b/pkg/errors/error_test.go @@ -687,3 +687,50 @@ func TestNewGitHubAPIErrorResponse_RateLimits(t *testing.T) { assert.Contains(t, text, "validation failed") }) } + +func TestNewGitHubAPIErrorResponse_ValidationMessages(t *testing.T) { + t.Run("ruleset ErrorResponse includes nested validation messages", func(t *testing.T) { + ctx := ContextWithGitHubErrors(context.Background()) + + originalErr := &github.ErrorResponse{ + Response: &http.Response{StatusCode: http.StatusUnprocessableEntity}, + Message: "Validation Failed", + Errors: []github.Error{ + { + Resource: "GitRef", + Field: "ref", + Code: "custom", + Message: "ref name does not match the required pattern 'feature/*'", + }, + }, + DocumentationURL: "https://docs.github.com/rest/git/refs#create-a-reference", + } + + result := NewGitHubAPIErrorResponse(ctx, "failed to create branch", nil, originalErr) + + text := requireErrorText(t, result) + assert.Contains(t, text, "failed to create branch") + assert.Contains(t, text, "HTTP 422 Validation Failed") + assert.Contains(t, text, "ref name does not match the required pattern 'feature/*'") + assert.Contains(t, text, "See https://docs.github.com/rest/git/refs#create-a-reference") + assert.NotContains(t, text, "Resource:") + }) + + t.Run("wrapped ErrorResponse is still unwrapped", func(t *testing.T) { + ctx := ContextWithGitHubErrors(context.Background()) + + originalErr := fmt.Errorf("create ref: %w", &github.ErrorResponse{ + Response: &http.Response{StatusCode: http.StatusUnprocessableEntity}, + Message: "Validation Failed", + Errors: []github.Error{ + {Message: "Changes must be made through a pull request."}, + }, + }) + + result := NewGitHubAPIErrorResponse(ctx, "failed to create branch", nil, originalErr) + + text := requireErrorText(t, result) + assert.Contains(t, text, "Changes must be made through a pull request.") + assert.NotContains(t, text, "create ref:") + }) +} diff --git a/pkg/github/repositories_test.go b/pkg/github/repositories_test.go index 332b212a17..5db6211f6a 100644 --- a/pkg/github/repositories_test.go +++ b/pkg/github/repositories_test.go @@ -1095,6 +1095,36 @@ func Test_CreateBranch(t *testing.T) { expectError: true, expectedErrMsg: "failed to create branch", }, + { + name: "create branch surfaces ruleset validation details", + mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + GetReposGitRefByOwnerByRepoByRef: mockResponse(t, http.StatusOK, mockSourceRef), + "GET /repos/owner/repo/git/ref/heads/main": mockResponse(t, http.StatusOK, mockSourceRef), + PostReposGitRefsByOwnerByRepo: func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusUnprocessableEntity) + _, _ = w.Write([]byte(`{ + "message": "Validation Failed", + "documentation_url": "https://docs.github.com/rest/git/refs#create-a-reference", + "errors": [ + { + "resource": "GitRef", + "field": "ref", + "code": "custom", + "message": "ref name does not match the required pattern 'feature/*'" + } + ] + }`)) + }, + }), + requestArgs: map[string]any{ + "owner": "owner", + "repo": "repo", + "branch": "hotfix", + "from_branch": "main", + }, + expectError: true, + expectedErrMsg: "ref name does not match the required pattern 'feature/*'", + }, } for _, tc := range tests {