Merge pull request #249 from buildkite/feat_build_job_filtering

feat(builds): push job state filtering to API via go-buildkite v4.17.0

Author: wolfeidau

go.mod

@@ -5,7 +5,7 @@ go 1.25.0
 require (
 	github.com/alecthomas/kong v1.14.0
 	github.com/buildkite/buildkite-logs v0.8.0
-	github.com/buildkite/go-buildkite/v4 v4.16.0
+	github.com/buildkite/go-buildkite/v4 v4.17.0
 	github.com/google/jsonschema-go v0.4.2
 	github.com/mattn/go-isatty v0.0.20
 	github.com/microcosm-cc/bluemonday v1.0.27

go.sum

@@ -76,8 +76,8 @@ github.com/aymerick/douceur v0.2.0 h1:Mv+mAeH1Q+n9Fr+oyamOlAkUNPWPlA8PPGR0QAaYuP
 github.com/aymerick/douceur v0.2.0/go.mod h1:wlT5vV2O3h55X9m7iVYN0TBM0NH/MmbLnd30/FjWUq4=
 github.com/buildkite/buildkite-logs v0.8.0 h1:Zp+lIZDD4Ny3/fnGjAkR3gjDNG68sSqQKv/DnTKzCrw=
 github.com/buildkite/buildkite-logs v0.8.0/go.mod h1:32+BbDpjJAzL3yH/qkr8OTkMtlXPUFWlkp34NcM43dM=
-github.com/buildkite/go-buildkite/v4 v4.16.0 h1:uRZmOg6zfZOCpak1tizzlv9pq8Syt7WmeEb0Ov7r1NE=
-github.com/buildkite/go-buildkite/v4 v4.16.0/go.mod h1:8+7GiWBKwEPAWoZnRU/kpNCt46j1iVH8kFMMbD4YDfc=
+github.com/buildkite/go-buildkite/v4 v4.17.0 h1:OYy9PG5A15K4Up2dkZgvXP7esAqzQskA0VGXvciRUNQ=
+github.com/buildkite/go-buildkite/v4 v4.17.0/go.mod h1:8+7GiWBKwEPAWoZnRU/kpNCt46j1iVH8kFMMbD4YDfc=
 github.com/cenkalti/backoff v2.2.1+incompatible h1:tNowT99t7UNflLxfYYSlKYsBpXdEet03Pg2g16Swow4=
 github.com/cenkalti/backoff v2.2.1+incompatible/go.mod h1:90ReRw6GdpyfrHakVjL/QHaoyV4aDUVVkXQJJJ3NXXM=
 github.com/cenkalti/backoff/v5 v5.0.3 h1:ZN+IMa753KfX5hd8vVaMixjnqRZ3y8CuJKRKj1xcsSM=

pkg/buildkite/builds.go

@@ -36,7 +36,6 @@ type BuildSummary struct {
 	Message   string               `json:"message"`
 	WebURL    string               `json:"web_url"`
 	CreatedAt *buildkite.Timestamp `json:"created_at"`
-	JobsTotal int                  `json:"jobs_total"`
 }
 
 // JobEntry represents a lightweight job reference with just enough info to identify and filter jobs
@@ -106,12 +105,11 @@ func summarizeBuild(build buildkite.Build) BuildSummary {
 		Message:   build.Message,
 		WebURL:    build.WebURL,
 		CreatedAt: build.CreatedAt,
-		JobsTotal: len(build.Jobs),
 	}
 }
 
 // detailBuild converts a buildkite.Build to BuildDetail with job summary
-// filteredJobs is used for job_summary stats, while build.Jobs is used for jobs_total
+// filteredJobs is used for job_summary stats and lightweight job entries
 func detailBuild(build buildkite.Build, filteredJobs []buildkite.Job) BuildDetail {
 	summary := summarizeBuild(build)
 
@@ -134,7 +132,7 @@ func detailBuild(build buildkite.Build, filteredJobs []buildkite.Job) BuildDetai
 	}
 
 	return BuildDetail{
-		BuildSummary: summary, // jobs_total reflects ALL jobs (unfiltered)
+		BuildSummary: summary,
 		Source:       build.Source,
 		Author:       build.Author,
 		StartedAt:    build.StartedAt,
@@ -345,33 +343,23 @@ func GetBuild() (mcp.Tool, mcp.ToolHandlerFor[GetBuildArgs, any], []string) {
 				IncludeTestEngine: true,
 			}
 
+			// Push job state filtering down to the API
+			if args.JobState != "" {
+				states := strings.Split(args.JobState, ",")
+				jobStates := make([]string, len(states))
+				for i, state := range states {
+					jobStates[i] = strings.TrimSpace(state)
+				}
+				options.JobStates = jobStates
+			}
+
 			deps := DepsFromContext(ctx)
 			build, _, err := deps.BuildsClient.Get(ctx, args.OrgSlug, args.PipelineSlug, args.BuildNumber, options)
 			if err != nil {
 				return handleBuildkiteError(err)
 			}
 
-			// Parse job states filter
-			var requestedStates map[string]bool
-			if args.JobState != "" {
-				states := strings.Split(args.JobState, ",")
-				requestedStates = make(map[string]bool, len(states))
-				for _, state := range states {
-					requestedStates[strings.TrimSpace(state)] = true
-				}
-			}
-
-			// Filter jobs if states specified
 			jobs := build.Jobs
-			if requestedStates != nil {
-				filteredJobs := make([]buildkite.Job, 0)
-				for _, job := range build.Jobs {
-					if job.State != "" && requestedStates[job.State] {
-						filteredJobs = append(filteredJobs, job)
-					}
-				}
-				jobs = filteredJobs
-			}
 
 			// Strip agent details if not requested
 			if !args.IncludeAgent && len(jobs) > 0 {

pkg/buildkite/builds_test.go

@@ -104,7 +104,7 @@ func TestGetBuildDefault(t *testing.T) {
 	assert.Contains(textContent.Text, `"state":"running"`)
 	assert.Contains(textContent.Text, `"total":0`)
 	assert.Contains(textContent.Text, `"by_state":{}`)
-	assert.Contains(textContent.Text, `"jobs_total":0`)
+	assert.NotContains(textContent.Text, `"jobs_total"`)
 }
 
 func TestGetBuildWithJobSummary(t *testing.T) {
@@ -204,7 +204,7 @@ func TestListBuilds(t *testing.T) {
 	assert.Contains(textContent.Text, `"id":"123"`)
 	assert.Contains(textContent.Text, `"number":1`)
 	assert.Contains(textContent.Text, `"state":"running"`)
-	assert.Contains(textContent.Text, `"jobs_total":0`)
+	assert.NotContains(textContent.Text, `"jobs_total"`)
 
 	// Verify default pagination parameters - new defaults
 	assert.NotNil(capturedOptions)
@@ -554,18 +554,38 @@ func TestCreateBuild(t *testing.T) {
 func TestGetBuildWithJobStateFilter(t *testing.T) {
 	assert := require.New(t)
 
+	// Mock returns jobs matching the requested states (simulating API-side filtering)
 	client := &MockBuildsClient{
 		GetFunc: func(ctx context.Context, org string, pipeline string, id string, opt *buildkite.BuildGetOptions) (buildkite.Build, *buildkite.Response, error) {
+			// Simulate API-side job state filtering
+			allJobs := []buildkite.Job{
+				{ID: "job1", Name: "Test 1", State: "passed", Agent: buildkite.Agent{ID: "agent1", Name: "Agent 1"}},
+				{ID: "job2", Name: "Test 2", State: "failed", Agent: buildkite.Agent{ID: "agent2", Name: "Agent 2"}},
+				{ID: "job3", Name: "Test 3", State: "failed", Agent: buildkite.Agent{ID: "agent3", Name: "Agent 3"}},
+				{ID: "job4", Name: "Test 4", State: "broken", Agent: buildkite.Agent{ID: "agent4", Name: "Agent 4"}},
+			}
+
+			// Filter jobs based on requested states (simulating what the API does)
+			var jobs []buildkite.Job
+			if len(opt.JobStates) > 0 {
+				stateSet := make(map[string]bool, len(opt.JobStates))
+				for _, s := range opt.JobStates {
+					stateSet[s] = true
+				}
+				for _, job := range allJobs {
+					if stateSet[job.State] {
+						jobs = append(jobs, job)
+					}
+				}
+			} else {
+				jobs = allJobs
+			}
+
 			return buildkite.Build{
 					ID:     "123",
 					Number: 1,
 					State:  "failed",
-					Jobs: []buildkite.Job{
-						{ID: "job1", Name: "Test 1", State: "passed", Agent: buildkite.Agent{ID: "agent1", Name: "Agent 1"}},
-						{ID: "job2", Name: "Test 2", State: "failed", Agent: buildkite.Agent{ID: "agent2", Name: "Agent 2"}},
-						{ID: "job3", Name: "Test 3", State: "failed", Agent: buildkite.Agent{ID: "agent3", Name: "Agent 3"}},
-						{ID: "job4", Name: "Test 4", State: "broken", Agent: buildkite.Agent{ID: "agent4", Name: "Agent 4"}},
-					},
+					Jobs:   jobs,
 				}, &buildkite.Response{
 					Response: &http.Response{StatusCode: 200},
 				}, nil
@@ -696,15 +716,16 @@ func TestGetBuildDetailedWithJobStateFilter(t *testing.T) {
 
 	client := &MockBuildsClient{
 		GetFunc: func(ctx context.Context, org string, pipeline string, id string, opt *buildkite.BuildGetOptions) (buildkite.Build, *buildkite.Response, error) {
+			assert.Equal([]string{"failed"}, opt.JobStates, "JobStates should be passed to the API")
+
+			// API returns only the filtered jobs
 			return buildkite.Build{
 					ID:     "123",
 					Number: 1,
 					State:  "failed",
 					Jobs: []buildkite.Job{
-						{ID: "job1", Name: "Test 1", State: "passed"},
 						{ID: "job2", Name: "Test 2", State: "failed"},
 						{ID: "job3", Name: "Test 3", State: "failed"},
-						{ID: "job4", Name: "Test 4", State: "broken"},
 					},
 				}, &buildkite.Response{
 					Response: &http.Response{StatusCode: 200},
@@ -729,17 +750,14 @@ func TestGetBuildDetailedWithJobStateFilter(t *testing.T) {
 	assert.NoError(err)
 
 	textContent := getTextResult(t, result)
-	// jobs_total should reflect ALL jobs (4)
-	assert.Contains(textContent.Text, `"jobs_total":4`)
+	assert.NotContains(textContent.Text, `"jobs_total"`)
 	// job_summary.total should reflect filtered jobs (2)
 	assert.Contains(textContent.Text, `"total":2`)
 	// job_summary.by_state should only show failed count
 	assert.Contains(textContent.Text, `"failed":2`)
 	// jobs array should contain only the filtered entries
 	assert.Contains(textContent.Text, `{"id":"job2","name":"Test 2","state":"failed"}`)
 	assert.Contains(textContent.Text, `{"id":"job3","name":"Test 3","state":"failed"}`)
-	assert.NotContains(textContent.Text, `{"id":"job1"`)
-	assert.NotContains(textContent.Text, `{"id":"job4"`)
 }
 
 func TestGetBuildInvalidDetailLevel(t *testing.T) {