Skip to content

Commit 81fd5e3

Browse files
committed
feat(server): detect and surface Buildkite API 401 errors
When the Buildkite API returns HTTP 401, tool handlers now propagate a structured ErrUnauthorized (a *jsonrpc.Error with code 401) instead of swallowing it as a generic tool result error. - handleBuildkiteError centralises error conversion across all tool handlers: 401 → ErrUnauthorized, other ErrorResponse → tool result error with raw body or message, everything else → tool result error - unauthorizedMiddleware intercepts ErrUnauthorized in the MCP middleware chain, logs a warning, signals the HTTP layer, and fires an optional OnUnauthorized callback for library consumers - NewHTTPUnauthorizedHandler wraps the streamable HTTP handler to return a real HTTP 401 (with WWW-Authenticate header and no body) instead of a 200 with a JSON-RPC error body; stale inner-handler headers are cleared before the 401 is written - WithOnUnauthorized option allows library consumers to register a reauth callback - All three log reader tools (search_logs, tail_logs, read_logs) now use handleBuildkiteError so 401s from the buildkite-logs library propagate correctly
1 parent 19b88f7 commit 81fd5e3

22 files changed

Lines changed: 424 additions & 105 deletions

internal/commands/http.go

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -55,9 +55,11 @@ func (c *HTTPCmd) Run(ctx context.Context, globals *Globals) error {
5555

5656
mux.HandleFunc("/health", healthHandler)
5757

58-
handler := mcp.NewStreamableHTTPHandler(factory, &mcp.StreamableHTTPOptions{
59-
Stateless: true,
60-
})
58+
handler := server.NewHTTPUnauthorizedHandler(
59+
mcp.NewStreamableHTTPHandler(factory, &mcp.StreamableHTTPOptions{
60+
Stateless: true,
61+
}),
62+
)
6163
mux.Handle("/mcp", handler)
6264

6365
log.Ctx(ctx).Info().

pkg/buildkite/access_token.go

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ import (
44
"context"
55

66
"github.com/buildkite/buildkite-mcp-server/pkg/trace"
7-
"github.com/buildkite/buildkite-mcp-server/pkg/utils"
87
"github.com/buildkite/go-buildkite/v4"
98
"github.com/modelcontextprotocol/go-sdk/mcp"
109
)
@@ -30,7 +29,7 @@ func AccessToken() (mcp.Tool, mcp.ToolHandlerFor[AccessTokenArgs, any], []string
3029
deps := DepsFromContext(ctx)
3130
token, _, err := deps.AccessTokensClient.Get(ctx)
3231
if err != nil {
33-
return utils.NewToolResultError(err.Error()), nil, nil
32+
return handleBuildkiteError(err)
3433
}
3534

3635
return mcpTextResult(span, &token)

pkg/buildkite/annotations.go

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ import (
44
"context"
55

66
"github.com/buildkite/buildkite-mcp-server/pkg/trace"
7-
"github.com/buildkite/buildkite-mcp-server/pkg/utils"
87
"github.com/buildkite/go-buildkite/v4"
98
"github.com/modelcontextprotocol/go-sdk/mcp"
109
"go.opentelemetry.io/otel/attribute"
@@ -51,7 +50,7 @@ func ListAnnotations() (mcp.Tool, mcp.ToolHandlerFor[ListAnnotationsArgs, any],
5150
ListOptions: paginationParams,
5251
})
5352
if err != nil {
54-
return utils.NewToolResultError(err.Error()), nil, nil
53+
return handleBuildkiteError(err)
5554
}
5655

5756
result := PaginatedResult[buildkite.Annotation]{

pkg/buildkite/artifacts.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -127,7 +127,7 @@ func ListArtifactsForBuild() (mcp.Tool, mcp.ToolHandlerFor[ListArtifactsForBuild
127127
ListOptions: paginationParams,
128128
})
129129
if err != nil {
130-
return utils.NewToolResultError(err.Error()), nil, nil
130+
return handleBuildkiteError(err)
131131
}
132132

133133
result := PaginatedResult[buildkite.Artifact]{
@@ -174,7 +174,7 @@ func ListArtifactsForJob() (mcp.Tool, mcp.ToolHandlerFor[ListArtifactsForJobArgs
174174
ListOptions: paginationParams,
175175
})
176176
if err != nil {
177-
return utils.NewToolResultError(err.Error()), nil, nil
177+
return handleBuildkiteError(err)
178178
}
179179

180180
result := PaginatedResult[buildkite.Artifact]{

pkg/buildkite/builds.go

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

33
import (
44
"context"
5-
"errors"
65
"strings"
76

87
"github.com/buildkite/buildkite-mcp-server/pkg/trace"
@@ -234,14 +233,7 @@ func ListBuilds() (mcp.Tool, mcp.ToolHandlerFor[ListBuildsArgs, any], []string)
234233
builds, resp, err = deps.BuildsClient.ListByOrg(ctx, args.OrgSlug, options)
235234
}
236235
if err != nil {
237-
var errResp *buildkite.ErrorResponse
238-
if errors.As(err, &errResp) {
239-
if errResp.RawBody != nil {
240-
return utils.NewToolResultError(string(errResp.RawBody)), nil, nil
241-
}
242-
}
243-
244-
return utils.NewToolResultError(err.Error()), nil, nil
236+
return handleBuildkiteError(err)
245237
}
246238

247239
headers := map[string]string{
@@ -292,14 +284,7 @@ func GetBuildTestEngineRuns() (mcp.Tool, mcp.ToolHandlerFor[GetBuildTestEngineRu
292284
IncludeTestEngine: true,
293285
})
294286
if err != nil {
295-
var errResp *buildkite.ErrorResponse
296-
if errors.As(err, &errResp) {
297-
if errResp.RawBody != nil {
298-
return utils.NewToolResultError(string(errResp.RawBody)), nil, nil
299-
}
300-
}
301-
302-
return utils.NewToolResultError(err.Error()), nil, nil
287+
return handleBuildkiteError(err)
303288
}
304289

305290
// Extract just the test engine runs data
@@ -348,14 +333,7 @@ func GetBuild() (mcp.Tool, mcp.ToolHandlerFor[GetBuildArgs, any], []string) {
348333
deps := DepsFromContext(ctx)
349334
build, _, err := deps.BuildsClient.Get(ctx, args.OrgSlug, args.PipelineSlug, args.BuildNumber, options)
350335
if err != nil {
351-
var errResp *buildkite.ErrorResponse
352-
if errors.As(err, &errResp) {
353-
if errResp.RawBody != nil {
354-
return utils.NewToolResultError(string(errResp.RawBody)), nil, nil
355-
}
356-
}
357-
358-
return utils.NewToolResultError(err.Error()), nil, nil
336+
return handleBuildkiteError(err)
359337
}
360338

361339
// Parse job states filter
@@ -470,14 +448,7 @@ func CreateBuild() (mcp.Tool, mcp.ToolHandlerFor[CreateBuildArgs, any], []string
470448
deps := DepsFromContext(ctx)
471449
build, _, err := deps.BuildsClient.Create(ctx, args.OrgSlug, args.PipelineSlug, createBuild)
472450
if err != nil {
473-
var errResp *buildkite.ErrorResponse
474-
if errors.As(err, &errResp) {
475-
if errResp.RawBody != nil {
476-
return utils.NewToolResultError(string(errResp.RawBody)), nil, nil
477-
}
478-
}
479-
480-
return utils.NewToolResultError(err.Error()), nil, nil
451+
return handleBuildkiteError(err)
481452
}
482453

483454
return mcpTextResult(span, &build)

pkg/buildkite/cluster_queue.go

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ import (
44
"context"
55

66
"github.com/buildkite/buildkite-mcp-server/pkg/trace"
7-
"github.com/buildkite/buildkite-mcp-server/pkg/utils"
87
"github.com/buildkite/go-buildkite/v4"
98
"github.com/modelcontextprotocol/go-sdk/mcp"
109
"go.opentelemetry.io/otel/attribute"
@@ -54,7 +53,7 @@ func ListClusterQueues() (mcp.Tool, mcp.ToolHandlerFor[ListClusterQueuesArgs, an
5453
ListOptions: paginationParams,
5554
})
5655
if err != nil {
57-
return utils.NewToolResultError(err.Error()), nil, nil
56+
return handleBuildkiteError(err)
5857
}
5958

6059
result := PaginatedResult[buildkite.ClusterQueue]{
@@ -93,7 +92,7 @@ func GetClusterQueue() (mcp.Tool, mcp.ToolHandlerFor[GetClusterQueueArgs, any],
9392
deps := DepsFromContext(ctx)
9493
queue, _, err := deps.ClusterQueuesClient.Get(ctx, args.OrgSlug, args.ClusterID, args.QueueID)
9594
if err != nil {
96-
return utils.NewToolResultError(err.Error()), nil, nil
95+
return handleBuildkiteError(err)
9796
}
9897

9998
return mcpTextResult(span, &queue)

pkg/buildkite/clusters.go

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ import (
44
"context"
55

66
"github.com/buildkite/buildkite-mcp-server/pkg/trace"
7-
"github.com/buildkite/buildkite-mcp-server/pkg/utils"
87
"github.com/buildkite/go-buildkite/v4"
98
"github.com/modelcontextprotocol/go-sdk/mcp"
109
"go.opentelemetry.io/otel/attribute"
@@ -51,7 +50,7 @@ func ListClusters() (mcp.Tool, mcp.ToolHandlerFor[ListClustersArgs, any], []stri
5150
ListOptions: paginationParams,
5251
})
5352
if err != nil {
54-
return utils.NewToolResultError(err.Error()), nil, nil
53+
return handleBuildkiteError(err)
5554
}
5655

5756
result := PaginatedResult[buildkite.Cluster]{
@@ -89,7 +88,7 @@ func GetCluster() (mcp.Tool, mcp.ToolHandlerFor[GetClusterArgs, any], []string)
8988
deps := DepsFromContext(ctx)
9089
cluster, _, err := deps.ClustersClient.Get(ctx, args.OrgSlug, args.ClusterID)
9190
if err != nil {
92-
return utils.NewToolResultError(err.Error()), nil, nil
91+
return handleBuildkiteError(err)
9392
}
9493

9594
return mcpTextResult(span, &cluster)

pkg/buildkite/errors.go

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
package buildkite
2+
3+
import (
4+
"errors"
5+
"net/http"
6+
7+
"github.com/buildkite/buildkite-mcp-server/pkg/utils"
8+
"github.com/buildkite/go-buildkite/v4"
9+
"github.com/modelcontextprotocol/go-sdk/jsonrpc"
10+
"github.com/modelcontextprotocol/go-sdk/mcp"
11+
)
12+
13+
// ErrUnauthorized is returned when the Buildkite API responds with HTTP 401.
14+
// It is a *jsonrpc.Error so the MCP SDK passes it through the middleware chain
15+
// rather than converting it to a tool result body. Library consumers can use
16+
// errors.Is to detect this and trigger a reauth flow.
17+
//
18+
// The error code 401 is a positive, non-standard JSON-RPC code (conventional
19+
// codes are negative). It is chosen deliberately for HTTP semantic alignment;
20+
// jsonrpc.Error.Is compares by code value only, so detection via errors.Is is
21+
// unaffected by the sign.
22+
//
23+
// Do not modify the fields of this value; treat it as a read-only sentinel.
24+
var ErrUnauthorized = &jsonrpc.Error{
25+
Code: http.StatusUnauthorized,
26+
Message: "buildkite: unauthorized",
27+
}
28+
29+
// handleBuildkiteError converts a Buildkite API error into tool handler return values.
30+
// On a 401 it returns (nil, nil, ErrUnauthorized) so the error propagates as a
31+
// JSON-RPC error and can be intercepted by middleware. On other errors it returns
32+
// a tool result error so the tool call succeeds at the JSON-RPC level but with an
33+
// error body.
34+
func handleBuildkiteError(err error) (*mcp.CallToolResult, any, error) {
35+
var errResp *buildkite.ErrorResponse
36+
if errors.As(err, &errResp) {
37+
if errResp.Response != nil && errResp.Response.StatusCode == http.StatusUnauthorized {
38+
return nil, nil, ErrUnauthorized
39+
}
40+
if errResp.RawBody != nil {
41+
return utils.NewToolResultError(string(errResp.RawBody)), nil, nil
42+
}
43+
if errResp.Message != "" {
44+
return utils.NewToolResultError(errResp.Message), nil, nil
45+
}
46+
}
47+
return utils.NewToolResultError(err.Error()), nil, nil
48+
}

pkg/buildkite/errors_test.go

Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,91 @@
1+
package buildkite
2+
3+
import (
4+
"fmt"
5+
"net/http"
6+
"testing"
7+
8+
"github.com/buildkite/go-buildkite/v4"
9+
"github.com/stretchr/testify/require"
10+
)
11+
12+
func TestHandleBuildkiteError_Unauthorized(t *testing.T) {
13+
errResp := &buildkite.ErrorResponse{
14+
Response: &http.Response{StatusCode: http.StatusUnauthorized},
15+
}
16+
17+
result, data, err := handleBuildkiteError(errResp)
18+
19+
require.Nil(t, result)
20+
require.Nil(t, data)
21+
require.ErrorIs(t, err, ErrUnauthorized)
22+
}
23+
24+
func TestHandleBuildkiteError_WithRawBody(t *testing.T) {
25+
errResp := &buildkite.ErrorResponse{
26+
Response: &http.Response{StatusCode: http.StatusUnprocessableEntity},
27+
RawBody: []byte(`{"message":"validation failed"}`),
28+
}
29+
30+
result, data, err := handleBuildkiteError(errResp)
31+
32+
require.NoError(t, err)
33+
require.Nil(t, data)
34+
require.NotNil(t, result)
35+
require.True(t, result.IsError)
36+
require.Len(t, result.Content, 1)
37+
textContent := getTextResult(t, result)
38+
require.JSONEq(t, `{"message":"validation failed"}`, textContent.Text)
39+
}
40+
41+
func TestHandleBuildkiteError_GenericError(t *testing.T) {
42+
genericErr := fmt.Errorf("connection refused")
43+
44+
result, data, err := handleBuildkiteError(genericErr)
45+
46+
require.NoError(t, err)
47+
require.Nil(t, data)
48+
require.NotNil(t, result)
49+
require.True(t, result.IsError)
50+
textContent := getTextResult(t, result)
51+
require.Equal(t, "connection refused", textContent.Text)
52+
}
53+
54+
func TestHandleBuildkiteError_NonUnauthorizedWithMessage(t *testing.T) {
55+
errResp := &buildkite.ErrorResponse{
56+
Response: &http.Response{StatusCode: http.StatusNotFound},
57+
Message: "pipeline not found",
58+
}
59+
60+
result, data, err := handleBuildkiteError(errResp)
61+
62+
require.NoError(t, err)
63+
require.Nil(t, data)
64+
require.NotNil(t, result)
65+
require.True(t, result.IsError)
66+
textContent := getTextResult(t, result)
67+
require.Equal(t, "pipeline not found", textContent.Text)
68+
}
69+
70+
func TestHandleBuildkiteError_NilResponse(t *testing.T) {
71+
// ErrorResponse with no underlying HTTP response (e.g. a network-level failure
72+
// that still produces an ErrorResponse). Must not be treated as a 401.
73+
errResp := &buildkite.ErrorResponse{
74+
Response: nil,
75+
Message: "connection reset",
76+
}
77+
78+
result, data, err := handleBuildkiteError(errResp)
79+
80+
require.NoError(t, err)
81+
require.Nil(t, data)
82+
require.NotNil(t, result)
83+
require.True(t, result.IsError)
84+
textContent := getTextResult(t, result)
85+
require.Equal(t, "connection reset", textContent.Text)
86+
}
87+
88+
func TestErrUnauthorized_IsWrappable(t *testing.T) {
89+
wrapped := fmt.Errorf("wrapped: %w", ErrUnauthorized)
90+
require.ErrorIs(t, wrapped, ErrUnauthorized)
91+
}

pkg/buildkite/joblogs.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -158,7 +158,7 @@ func SearchLogs() (mcp.Tool, mcp.ToolHandlerFor[SearchLogsParams, any], []string
158158
deps := DepsFromContext(ctx)
159159
reader, err := newParquetReader(ctx, deps.BuildkiteLogsClient, params.JobLogsBaseParams)
160160
if err != nil {
161-
return utils.NewToolResultError(fmt.Sprintf("Failed to create log reader: %v", err)), nil, nil
161+
return handleBuildkiteError(err)
162162
}
163163
defer reader.Close()
164164

@@ -236,7 +236,7 @@ func TailLogs() (mcp.Tool, mcp.ToolHandlerFor[TailLogsParams, any], []string) {
236236
deps := DepsFromContext(ctx)
237237
reader, err := newParquetReader(ctx, deps.BuildkiteLogsClient, params.JobLogsBaseParams)
238238
if err != nil {
239-
return utils.NewToolResultError(fmt.Sprintf("Failed to create log reader: %v", err)), nil, nil
239+
return handleBuildkiteError(err)
240240
}
241241
defer reader.Close()
242242

@@ -301,7 +301,7 @@ func ReadLogs() (mcp.Tool, mcp.ToolHandlerFor[ReadLogsParams, any], []string) {
301301
deps := DepsFromContext(ctx)
302302
reader, err := newParquetReader(ctx, deps.BuildkiteLogsClient, params.JobLogsBaseParams)
303303
if err != nil {
304-
return utils.NewToolResultError(fmt.Sprintf("Failed to create log reader: %v", err)), nil, nil
304+
return handleBuildkiteError(err)
305305
}
306306
defer reader.Close()
307307

0 commit comments

Comments
 (0)