fix: update error handling in line with mcp-go recommendations

AGENT.md

@@ -23,3 +23,4 @@
 - Error handling: return errors up the stack, log at top level
 - Package names: lowercase, descriptive (buildkite, commands, trace, tokens)
 - Use contexts for cancellation and tracing throughout
+- Use `mcp.NewToolResultError` or `mcp.NewToolResultErrorFromErr` to handle errors in tools.

pkg/buildkite/access_token.go

@@ -2,15 +2,11 @@ package buildkite
 
 import (
 	"context"
-	"encoding/json"
-	"fmt"
 
-	"github.com/buildkite/buildkite-mcp-server/pkg/tokens"
 	"github.com/buildkite/buildkite-mcp-server/pkg/trace"
 	"github.com/buildkite/go-buildkite/v4"
 	"github.com/mark3labs/mcp-go/mcp"
 	"github.com/mark3labs/mcp-go/server"
-	"go.opentelemetry.io/otel/attribute"
 )
 
 type AccessTokenClient interface {
@@ -33,15 +29,6 @@ func AccessToken(client AccessTokenClient) (tool mcp.Tool, handler server.ToolHa
 				return mcp.NewToolResultError(err.Error()), nil
 			}
 
-			r, err := json.Marshal(&token)
-			if err != nil {
-				return nil, fmt.Errorf("failed to marshal token: %w", err)
-			}
-
-			span.SetAttributes(
-				attribute.Int("estimated_tokens", tokens.EstimateTokens(string(r))),
-			)
-
-			return mcp.NewToolResultText(string(r)), nil
+			return mcpTextResult(span, &token)
 		}
 }

pkg/buildkite/annotations.go

@@ -2,10 +2,7 @@ package buildkite
 
 import (
 	"context"
-	"encoding/json"
-	"fmt"
 
-	"github.com/buildkite/buildkite-mcp-server/pkg/tokens"
 	"github.com/buildkite/buildkite-mcp-server/pkg/trace"
 	"github.com/buildkite/go-buildkite/v4"
 	"github.com/mark3labs/mcp-go/mcp"
@@ -82,16 +79,10 @@ func ListAnnotations(client AnnotationsClient) (tool mcp.Tool, handler server.To
 				},
 			}
 
-			r, err := json.Marshal(&result)
-			if err != nil {
-				return nil, fmt.Errorf("failed to marshal annotations: %w", err)
-			}
-
 			span.SetAttributes(
 				attribute.Int("item_count", len(annotations)),
-				attribute.Int("estimated_tokens", tokens.EstimateTokens(string(r))),
 			)
 
-			return mcp.NewToolResultText(string(r)), nil
+			return mcpTextResult(span, &result)
 		}
 }

pkg/buildkite/artifacts.go

@@ -192,15 +192,6 @@ func GetArtifact(client ArtifactsClient) (tool mcp.Tool, handler server.ToolHand
 				"encoding":   "base64",
 			}
 
-			r, err := json.Marshal(result)
-			if err != nil {
-				return nil, fmt.Errorf("failed to marshal artifact response: %w", err)
-			}
-
-			span.SetAttributes(
-				attribute.Int("estimated_tokens", tokens.EstimateTokens(string(r))),
-			)
-
-			return mcp.NewToolResultText(string(r)), nil
+			return mcpTextResult(span, &result)
 		}
 }

pkg/buildkite/buildkite.go

@@ -1,8 +1,14 @@
 package buildkite
 
 import (
+	"encoding/json"
+	"fmt"
+
+	"github.com/buildkite/buildkite-mcp-server/pkg/tokens"
 	"github.com/buildkite/go-buildkite/v4"
 	"github.com/mark3labs/mcp-go/mcp"
+	"go.opentelemetry.io/otel/attribute"
+	"go.opentelemetry.io/otel/trace"
 )
 
 type PaginatedResult[T any] struct {
@@ -110,3 +116,16 @@ func applyClientSidePagination[T any](items []T, params ClientSidePaginationPara
 		HasPrev:    params.Page > 1,
 	}
 }
+
+func mcpTextResult(span trace.Span, result any) (*mcp.CallToolResult, error) {
+	r, err := json.Marshal(result)
+	if err != nil {
+		return mcp.NewToolResultError(fmt.Sprintf("failed to marshal result: %v", err)), nil
+	}
+
+	span.SetAttributes(
+		attribute.Int("estimated_tokens", tokens.EstimateTokens(string(r))),
+	)
+
+	return mcp.NewToolResultText(string(r)), nil
+}

pkg/buildkite/builds.go

@@ -349,12 +349,7 @@ func GetBuildTestEngineRuns(client BuildsClient) (tool mcp.Tool, handler mcp.Typ
 				testEngineRuns = build.TestEngine.Runs
 			}
 
-			r, err := json.Marshal(&testEngineRuns)
-			if err != nil {
-				return nil, fmt.Errorf("failed to marshal test engine runs: %w", err)
-			}
-
-			return mcp.NewToolResultText(string(r)), nil
+			return mcpTextResult(span, &testEngineRuns)
 		}
 }
 
@@ -435,12 +430,7 @@ func GetBuild(client BuildsClient) (tool mcp.Tool, handler mcp.TypedToolHandlerF
 				return mcp.NewToolResultError("detail_level must be 'summary', 'detailed', or 'full'"), nil
 			}
 
-			r, err := json.Marshal(result)
-			if err != nil {
-				return nil, fmt.Errorf("failed to marshal build: %w", err)
-			}
-
-			return mcp.NewToolResultText(string(r)), nil
+			return mcpTextResult(span, &result)
 		}
 }
 
@@ -550,11 +540,7 @@ func CreateBuild(client BuildsClient) (tool mcp.Tool, handler mcp.TypedToolHandl
 				return mcp.NewToolResultError(err.Error()), nil
 			}
 
-			r, err := json.Marshal(&build)
-			if err != nil {
-				return nil, fmt.Errorf("failed to marshal created build: %w", err)
-			}
-			return mcp.NewToolResultText(string(r)), nil
+			return mcpTextResult(span, &build)
 		}
 }
 
@@ -694,12 +680,7 @@ func WaitForBuild(client BuildsClient) (tool mcp.Tool, handler mcp.TypedToolHand
 			// default to detailed
 			result := detailBuild(build)
 
-			r, err := json.Marshal(result)
-			if err != nil {
-				return nil, fmt.Errorf("failed to marshal build: %w", err)
-			}
-
-			return mcp.NewToolResultText(string(r)), nil
+			return mcpTextResult(span, &result)
 		}
 }
 

pkg/buildkite/cluster_queue.go

@@ -2,10 +2,7 @@ package buildkite
 
 import (
 	"context"
-	"encoding/json"
-	"fmt"
 
-	"github.com/buildkite/buildkite-mcp-server/pkg/tokens"
 	"github.com/buildkite/buildkite-mcp-server/pkg/trace"
 	"github.com/buildkite/go-buildkite/v4"
 	"github.com/mark3labs/mcp-go/mcp"
@@ -75,17 +72,11 @@ func ListClusterQueues(client ClusterQueuesClient) (tool mcp.Tool, handler serve
 				},
 			}
 
-			r, err := json.Marshal(&result)
-			if err != nil {
-				return nil, fmt.Errorf("failed to marshal cluster queues response: %w", err)
-			}
-
 			span.SetAttributes(
 				attribute.Int("item_count", len(queues)),
-				attribute.Int("estimated_tokens", tokens.EstimateTokens(string(r))),
 			)
 
-			return mcp.NewToolResultText(string(r)), nil
+			return mcpTextResult(span, &result)
 		}
 }
 
@@ -134,15 +125,6 @@ func GetClusterQueue(client ClusterQueuesClient) (tool mcp.Tool, handler server.
 				return mcp.NewToolResultError(err.Error()), nil
 			}
 
-			r, err := json.Marshal(queue)
-			if err != nil {
-				return nil, fmt.Errorf("failed to marshal cluster queue response: %w", err)
-			}
-
-			span.SetAttributes(
-				attribute.Int("estimated_tokens", tokens.EstimateTokens(string(r))),
-			)
-
-			return mcp.NewToolResultText(string(r)), nil
+			return mcpTextResult(span, &queue)
 		}
 }

pkg/buildkite/clusters.go

@@ -2,10 +2,7 @@ package buildkite
 
 import (
 	"context"
-	"encoding/json"
-	"fmt"
 
-	"github.com/buildkite/buildkite-mcp-server/pkg/tokens"
 	"github.com/buildkite/buildkite-mcp-server/pkg/trace"
 	"github.com/buildkite/go-buildkite/v4"
 	"github.com/mark3labs/mcp-go/mcp"
@@ -66,17 +63,11 @@ func ListClusters(client ClustersClient) (tool mcp.Tool, handler server.ToolHand
 				},
 			}
 
-			r, err := json.Marshal(&result)
-			if err != nil {
-				return nil, fmt.Errorf("failed to marshal clusters response: %w", err)
-			}
-
 			span.SetAttributes(
 				attribute.Int("item_count", len(clusters)),
-				attribute.Int("estimated_tokens", tokens.EstimateTokens(string(r))),
 			)
 
-			return mcp.NewToolResultText(string(r)), nil
+			return mcpTextResult(span, &result)
 		}
 }
 
@@ -116,15 +107,6 @@ func GetCluster(client ClustersClient) (tool mcp.Tool, handler server.ToolHandle
 				return mcp.NewToolResultError(err.Error()), nil
 			}
 
-			r, err := json.Marshal(cluster)
-			if err != nil {
-				return nil, fmt.Errorf("failed to marshal cluster response: %w", err)
-			}
-
-			span.SetAttributes(
-				attribute.Int("estimated_tokens", tokens.EstimateTokens(string(r))),
-			)
-
-			return mcp.NewToolResultText(string(r)), nil
+			return mcpTextResult(span, &cluster)
 		}
 }

pkg/buildkite/joblogs.go

@@ -2,14 +2,12 @@ package buildkite
 
 import (
 	"context"
-	"encoding/json"
 	"fmt"
 	"iter"
 	"regexp"
 	"time"
 
 	buildkitelogs "github.com/buildkite/buildkite-logs"
-	"github.com/buildkite/buildkite-mcp-server/pkg/tokens"
 	"github.com/buildkite/buildkite-mcp-server/pkg/trace"
 	"github.com/mark3labs/mcp-go/mcp"
 	"go.opentelemetry.io/otel/attribute"
@@ -258,17 +256,11 @@ func SearchLogs(client BuildkiteLogsClient) (tool mcp.Tool, handler mcp.TypedToo
 				QueryTimeMS: queryTime.Milliseconds(),
 			}
 
-			r, err := json.Marshal(&response)
-			if err != nil {
-				return nil, fmt.Errorf("failed to marshal search results: %w", err)
-			}
-
 			span.SetAttributes(
 				attribute.Int("item_count", len(results)),
-				attribute.Int("estimated_tokens", tokens.EstimateTokens(string(r))),
 			)
 
-			return mcp.NewToolResultText(string(r)), nil
+			return mcpTextResult(span, &response)
 		}
 }
 
@@ -359,17 +351,11 @@ func TailLogs(client BuildkiteLogsClient) (tool mcp.Tool, handler mcp.TypedToolH
 				QueryTimeMS: queryTime.Milliseconds(),
 			}
 
-			r, err := json.Marshal(&response)
-			if err != nil {
-				return nil, fmt.Errorf("failed to marshal tail results: %w", err)
-			}
-
 			span.SetAttributes(
 				attribute.Int("item_count", len(entries)),
-				attribute.Int("estimated_tokens", tokens.EstimateTokens(string(r))),
 			)
 
-			return mcp.NewToolResultText(string(r)), nil
+			return mcpTextResult(span, &response)
 		}
 }
 
@@ -443,16 +429,7 @@ func GetLogsInfo(client BuildkiteLogsClient) (tool mcp.Tool, handler mcp.TypedTo
 				QueryTimeMS: queryTime.Milliseconds(),
 			}
 
-			r, err := json.Marshal(&response)
-			if err != nil {
-				return nil, fmt.Errorf("failed to marshal file info: %w", err)
-			}
-
-			span.SetAttributes(
-				attribute.Int("estimated_tokens", tokens.EstimateTokens(string(r))),
-			)
-
-			return mcp.NewToolResultText(string(r)), nil
+			return mcpTextResult(span, &response)
 		}
 }
 
@@ -547,16 +524,10 @@ func ReadLogs(client BuildkiteLogsClient) (tool mcp.Tool, handler mcp.TypedToolH
 				QueryTimeMS: queryTime.Milliseconds(),
 			}
 
-			r, err := json.Marshal(&response)
-			if err != nil {
-				return nil, fmt.Errorf("failed to marshal read results: %w", err)
-			}
-
 			span.SetAttributes(
 				attribute.Int("item_count", len(entries)),
-				attribute.Int("estimated_tokens", tokens.EstimateTokens(string(r))),
 			)
 
-			return mcp.NewToolResultText(string(r)), nil
+			return mcpTextResult(span, &response)
 		}
 }

pkg/buildkite/jobs.go

@@ -421,11 +421,6 @@ func UnblockJob(client JobsClient) (tool mcp.Tool, handler mcp.TypedToolHandlerF
 				return mcp.NewToolResultError(err.Error()), nil
 			}
 
-			r, err := json.Marshal(job)
-			if err != nil {
-				return nil, fmt.Errorf("failed to marshal job: %w", err)
-			}
-
-			return mcp.NewToolResultText(string(r)), nil
+			return mcpTextResult(span, &job)
 		}
 }