From d6014a14196202df645c5ec8e465d59c7f969667 Mon Sep 17 00:00:00 2001 From: Edmund Kohlwey Date: Fri, 12 Jun 2026 17:05:08 -0400 Subject: [PATCH] gitlab: Implement ChecksByChange with per-job pipeline detail MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Implements the GitLab side of the per-change checks API: rollup state from the MR's head pipeline, per-job name/state/url from the pipeline's job list, and the pipeline's web URL as the click target. - gateway: Pipeline grows ID and WebURL fields; new Job type (name + status + web_url); new PipelineJobsList API method wrapping /projects/:id/pipelines/:pipeline_id/jobs. - rollupFromPipelineStatus collapses GitLab's pipeline status enum to the upstream rollup taxonomy. Canceled/manual/skipped are user-initiated non-failures and roll up as passed (the operator's choice not to run them isn't a failure). Unknown statuses default to pending — GitLab adds new statuses over time and pending is the safer default. - pipelineState (the merge handler's path) keeps its 'no pipeline -> passed' semantics; ChecksByChange uses ChecksNone in that case so the indicator can distinguish. - ChecksByChange issues MergeRequestGet + PipelineJobsList per MR. Jobs are reported in pipeline order (stages first, then job order within stages); GitLab's job.status is already lowercase so it flows straight through as forge-native state. Integration fixtures regenerate on the next forgetest -update pass (real GitLab credentials). --- internal/forge/gitlab/checks_report.go | 89 +++++++++++++++++++-- internal/forge/gitlab/checks_report_test.go | 36 +++++++++ internal/gateway/gitlab/api.go | 36 +++++++++ 3 files changed, 154 insertions(+), 7 deletions(-) create mode 100644 internal/forge/gitlab/checks_report_test.go diff --git a/internal/forge/gitlab/checks_report.go b/internal/forge/gitlab/checks_report.go index 715981899..dc75f7383 100644 --- a/internal/forge/gitlab/checks_report.go +++ b/internal/forge/gitlab/checks_report.go @@ -2,18 +2,93 @@ package gitlab import ( "context" + "fmt" "go.abhg.dev/gs/internal/forge" + "go.abhg.dev/gs/internal/gateway/gitlab" ) -// ChecksByChange reports per-change rolled-up and per-run check state -// for each of the given changes. +// rollupFromPipelineStatus collapses a GitLab pipeline status into the +// forge rollup taxonomy. Canceled/manual/skipped pipelines are +// user-initiated non-failures and roll up as passed. Unknown statuses +// default to pending — GitLab adds new pipeline states over time and +// reporting them as pending is safer than reporting them as failed. +func rollupFromPipelineStatus(status string) forge.ChecksRollupState { + switch status { + case gitlab.PipelineStatusSuccess, + gitlab.PipelineStatusSkipped, + gitlab.PipelineStatusCanceled, + gitlab.PipelineStatusManual: + return forge.ChecksRollupPassed + case gitlab.PipelineStatusFailed: + return forge.ChecksRollupFailed + case gitlab.PipelineStatusCreated, + gitlab.PipelineStatusWaitingForResource, + gitlab.PipelineStatusPreparing, + gitlab.PipelineStatusPending, + gitlab.PipelineStatusRunning, + gitlab.PipelineStatusScheduled: + return forge.ChecksRollupPending + default: + return forge.ChecksRollupPending + } +} + +// ChecksByChange reports per-change rolled-up and per-job pipeline +// state for each of the given merge requests. // -// TODO: real implementation lands on a follow-up branch. -// This stub returns one nil per id to satisfy the [forge.Repository] -// interface while the schema branch lands standalone. +// One MergeRequestGet + (when a pipeline exists) one PipelineJobsList +// call per MR. The job list is reported in pipeline order (stages +// first, then job order within stages) which is the most useful order +// for both UI surfaces and tooltips. func (r *Repository) ChecksByChange( - _ context.Context, ids []forge.ChangeID, + ctx context.Context, ids []forge.ChangeID, ) ([]*forge.ChecksReport, error) { - return make([]*forge.ChecksReport, len(ids)), nil + out := make([]*forge.ChecksReport, len(ids)) + for i, id := range ids { + checks, err := r.changeChecks(ctx, id) + if err != nil { + return nil, fmt.Errorf("change %v: %w", id, err) + } + out[i] = checks + } + return out, nil +} + +func (r *Repository) changeChecks( + ctx context.Context, id forge.ChangeID, +) (*forge.ChecksReport, error) { + mrID := mustMR(id) + mr, _, err := r.client.MergeRequestGet(ctx, r.repoID, mrID.Number, nil) + if err != nil { + return nil, fmt.Errorf("get merge request: %w", err) + } + + if mr.HeadPipeline == nil { + return &forge.ChecksReport{Rollup: forge.ChecksRollupNone}, nil + } + + out := &forge.ChecksReport{ + Rollup: rollupFromPipelineStatus(mr.HeadPipeline.Status), + URL: mr.HeadPipeline.WebURL, + } + + if mr.HeadPipeline.ID == 0 { + return out, nil + } + + jobs, _, err := r.client.PipelineJobsList(ctx, r.repoID, mr.HeadPipeline.ID) + if err != nil { + return nil, fmt.Errorf("list pipeline jobs: %w", err) + } + + out.Runs = make([]forge.CheckRun, 0, len(jobs)) + for _, j := range jobs { + out.Runs = append(out.Runs, forge.CheckRun{ + Name: j.Name, + State: j.Status, // already lowercase per GitLab + URL: j.WebURL, + }) + } + return out, nil } diff --git a/internal/forge/gitlab/checks_report_test.go b/internal/forge/gitlab/checks_report_test.go new file mode 100644 index 000000000..b78d734d5 --- /dev/null +++ b/internal/forge/gitlab/checks_report_test.go @@ -0,0 +1,36 @@ +package gitlab + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "go.abhg.dev/gs/internal/forge" + "go.abhg.dev/gs/internal/gateway/gitlab" +) + +func TestRollupFromPipelineStatus(t *testing.T) { + tests := []struct { + status string + want forge.ChecksRollupState + }{ + {gitlab.PipelineStatusSuccess, forge.ChecksRollupPassed}, + {gitlab.PipelineStatusSkipped, forge.ChecksRollupPassed}, + // Canceled and manual are user-initiated non-failures. + {gitlab.PipelineStatusCanceled, forge.ChecksRollupPassed}, + {gitlab.PipelineStatusManual, forge.ChecksRollupPassed}, + {gitlab.PipelineStatusFailed, forge.ChecksRollupFailed}, + {gitlab.PipelineStatusPending, forge.ChecksRollupPending}, + {gitlab.PipelineStatusRunning, forge.ChecksRollupPending}, + {gitlab.PipelineStatusCreated, forge.ChecksRollupPending}, + {gitlab.PipelineStatusPreparing, forge.ChecksRollupPending}, + {gitlab.PipelineStatusWaitingForResource, forge.ChecksRollupPending}, + {gitlab.PipelineStatusScheduled, forge.ChecksRollupPending}, + {"unknown", forge.ChecksRollupPending}, + } + + for _, tt := range tests { + t.Run(tt.status, func(t *testing.T) { + assert.Equal(t, tt.want, rollupFromPipelineStatus(tt.status)) + }) + } +} diff --git a/internal/gateway/gitlab/api.go b/internal/gateway/gitlab/api.go index 6609e1464..5f167853b 100644 --- a/internal/gateway/gitlab/api.go +++ b/internal/gateway/gitlab/api.go @@ -228,6 +228,31 @@ func (c *Client) CommitStatusList( return response, resp, nil } +// PipelineJobsList lists the jobs of a pipeline, in pipeline order. +// +// GitLab API: +// https://docs.gitlab.com/api/jobs/#list-pipeline-jobs +func (c *Client) PipelineJobsList( + ctx context.Context, + projectID int64, + pipelineID int64, +) ([]*Job, *Response, error) { + var response []*Job + resp, err := c.get( + ctx, + fmt.Sprintf( + "projects/%d/pipelines/%d/jobs", + projectID, pipelineID, + ), + nil, + &response, + ) + if err != nil { + return nil, resp, err + } + return response, resp, nil +} + // MergeRequestNoteCreate creates a merge request note. // // GitLab API: @@ -545,6 +570,17 @@ type MergeRequest struct { type Pipeline struct { ID int64 `json:"id"` Status string `json:"status"` + WebURL string `json:"web_url,omitempty"` +} + +// Job is a single CI job within a pipeline. +// +// GitLab jobs API: +// https://docs.gitlab.com/api/jobs/ +type Job struct { + Name string `json:"name"` + Status string `json:"status"` + WebURL string `json:"web_url,omitempty"` } // CommitStatus matches the subset of GitLab commit status fields