Skip to content

fix: use MR pipelines endpoint for merged-results CI association #10

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
nkaewam merged 1 commit into from
Jul 28, 2026
Merged

fix: use MR pipelines endpoint for merged-results CI association #10

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 9 additions & 6 deletions internal/app/check.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"encoding/json"
"fmt"
"path/filepath"
"slices"
"sort"
"strconv"
"strings"
Expand Down Expand Up @@ -160,12 +161,12 @@ func (h *Handler) assessCheck(ctx context.Context, inv cli.Invocation, rc reposi
domainPipeline.Kind = stack.PipelineUnknown
}
if kind == stack.PipelineMergedResult {
associated, associationErr := rc.client.PipelineMergeRequests(
ctx, rc.project.ID.String(), head.ID.String())
mrPipelines, associationErr := rc.client.MergeRequestPipelines(
ctx, rc.project.ID.String(), member.IID)
if associationErr != nil {
return cli.Result{}, classifyGlab("read pipeline merge request association", associationErr)
return cli.Result{}, classifyGlab("read merge request pipelines", associationErr)
}
domainPipeline.AssociatedWithMR = pipelineAssociatedExactly(associated, member.IID)
domainPipeline.AssociatedWithMR = pipelineAssociatedWithMR(mrPipelines, head.ID.String())
}
if kind == stack.PipelineMergedResult {
// A merged-results pipeline runs against a synthetic commit, not
Expand Down Expand Up @@ -388,8 +389,10 @@ func classifyPipelineKind(p gitlab.Pipeline, memberIID int) stack.PipelineKind {
}
}

func pipelineAssociatedExactly(requests []gitlab.PipelineMergeRequest, memberIID int) bool {
return len(requests) == 1 && requests[0].IID == memberIID
func pipelineAssociatedWithMR(pipelines []gitlab.Pipeline, pipelineID string) bool {
return slices.ContainsFunc(pipelines, func(p gitlab.Pipeline) bool {
return p.ID.String() == pipelineID
})
}

func buildAPIStack(observedAt string, rc repositoryContext, selector api.Selector, mode stack.Mode,
Expand Down
97 changes: 81 additions & 16 deletions internal/app/pipeline_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,15 @@ package app
import (
"bytes"
"encoding/json"
"fmt"
"strconv"
"strings"
"testing"
"time"

"github.com/nkaewam/mrstack/internal/api"
"github.com/nkaewam/mrstack/internal/cli"
"github.com/nkaewam/mrstack/internal/gitexec"
"github.com/nkaewam/mrstack/internal/gitlab"
"github.com/nkaewam/mrstack/internal/stack"
)
Expand Down Expand Up @@ -158,27 +160,27 @@ func TestClassifyPipelineKind(t *testing.T) {
func TestCheckPipelineEvidenceIsSchemaValidForValidAndAmbiguousMergedResults(t *testing.T) {
tests := []struct {
name string
associated string
mrPipelines string
parents func(source, target string) []string
disposition string
}{
{
name: "valid", associated: `[{"iid":1}]`,
name: "valid", mrPipelines: `[{"id":9}]`,
parents: func(source, target string) []string { return []string{target, source} },
disposition: "ready",
},
{
name: "association mismatch", associated: `[{"iid":2}]`,
name: "association mismatch", mrPipelines: `[{"id":10}]`,
parents: func(source, target string) []string { return []string{source, target} },
disposition: "human_required",
},
{
name: "ambiguous association", associated: `[{"iid":1},{"iid":2}]`,
name: "missing pipeline", mrPipelines: `[]`,
parents: func(source, target string) []string { return []string{source, target} },
disposition: "human_required",
},
{
name: "parent mismatch", associated: `[{"iid":1}]`,
name: "parent mismatch", mrPipelines: `[{"id":9}]`,
parents: func(source, target string) []string { return []string{source, strings.Repeat("d", 40)} },
disposition: "human_required",
},
Expand All @@ -205,7 +207,7 @@ func TestCheckPipelineEvidenceIsSchemaValidForValidAndAmbiguousMergedResults(t *
"id":9,"sha":"` + syntheticOID + `","ref":"refs/merge-requests/1/merge",
"status":"success","source":"merge_request_event","web_url":"https://gitlab.example/pipelines/9"
}`)
responses["/projects/42/pipelines/9/merge_requests"] = json.RawMessage(tt.associated)
responses["/projects/42/merge_requests/1/pipelines?per_page=100"] = json.RawMessage(tt.mrPipelines)
responses["/projects/42/repository/commits/"+syntheticOID] = json.RawMessage(
`{"id":"` + syntheticOID + `","parent_ids":` + string(parentsJSON) + `}`)
handler := &Handler{
Expand Down Expand Up @@ -311,6 +313,9 @@ func TestCheckExactSourcePipelineDoesNotRequireMRAssociation(t *testing.T) {
if strings.Contains(strings.Join(call, " "), "/pipelines/9/merge_requests") {
t.Fatalf("exact-source %s pipeline queried unnecessary MR association: %q", tt.name, call)
}
if strings.Contains(strings.Join(call, " "), "/merge_requests/1/pipelines") {
t.Fatalf("exact-source %s pipeline queried MR pipelines: %q", tt.name, call)
}
}
var envelope struct {
Disposition string `json:"disposition"`
Expand All @@ -335,19 +340,79 @@ func TestCheckExactSourcePipelineDoesNotRequireMRAssociation(t *testing.T) {
}
}

func TestPipelineAssociationRequiresExactlySelectedMR(t *testing.T) {
if !pipelineAssociatedExactly([]gitlab.PipelineMergeRequest{{IID: 7}}, 7) {
t.Fatal("one exact association was rejected")
func TestCheckMergedResultsDoesNotCallBrokenPipelineToMREndpoint(t *testing.T) {
repo, _, mainOID, sourceOID, _ := createStackRepository(t)
syntheticOID := strings.Repeat("c", 40)
payload := []map[string]any{{
"iid": 1, "state": "opened", "source_branch": "feature/one", "target_branch": "main",
"sha": sourceOID, "web_url": "https://gitlab.example/group/project/-/merge_requests/1",
"author": map[string]any{"id": 7, "username": "developer"},
"diff_refs": map[string]any{"base_sha": mainOID, "head_sha": sourceOID, "start_sha": mainOID},
"head_pipeline": map[string]any{"id": 9},
}}
var calls [][]string
stateDir, stacksDir := testDirs(t)
responses := glabProjectResponses(true)
addPerMREndpoints(responses, payload)
responses["/projects/42/pipelines/9"] = json.RawMessage(`{
"id":9,"sha":"` + syntheticOID + `","ref":"refs/merge-requests/1/merge",
"status":"success","source":"merge_request_event","web_url":"https://gitlab.example/pipelines/9"
}`)
responses["/projects/42/merge_requests/1/pipelines?per_page=100"] = json.RawMessage(`[{"id":9}]`)
responses["/projects/42/repository/commits/"+syntheticOID] = json.RawMessage(
`{"id":"` + syntheticOID + `","parent_ids":["` + mainOID + `","` + sourceOID + `"]}`)
handler := &Handler{
Runner: fakeGlabRunner{
calls: &calls,
responses: responses,
dynamic: func(endpoint string, _ []string) (gitexec.Result, error) {
if endpoint == "/projects/42/pipelines/9/merge_requests" {
return gitexec.Result{}, fmt.Errorf("glab: HTTP 404")
}
return gitexec.Result{}, nil
},
},
Dir: repo, StateDir: stateDir, StacksDir: stacksDir,
Now: func() time.Time { return time.Date(2026, 7, 25, 12, 0, 0, 0, time.UTC) },
}
registerNamedStack(t, handler, testStackName, 1)
var stdout, stderr bytes.Buffer
exit := cli.RunWithHandler([]string{
"--json", "--no-input", "--remote", "origin", "check", testStackName,
}, &stdout, &stderr, handler)
if exit != 0 {
t.Fatalf("exit=%d stdout=%s stderr=%s", exit, stdout.String(), stderr.String())
}
for _, call := range calls {
if strings.Contains(strings.Join(call, " "), "/pipelines/9/merge_requests") {
t.Fatalf("check called broken reverse-lookup endpoint: %q", call)
}
}
var envelope struct {
Disposition string `json:"disposition"`
Stack struct {
SnapshotID string `json:"snapshot_id"`
} `json:"stack"`
}
if err := json.Unmarshal(stdout.Bytes(), &envelope); err != nil {
t.Fatalf("decode output: %v\n%s", err, stdout.String())
}
if envelope.Disposition != "ready" || envelope.Stack.SnapshotID == "" {
t.Fatalf("check did not complete with snapshot: %+v\n%s", envelope, stdout.String())
}
}

func TestPipelineAssociationRequiresHeadPipelineInMRList(t *testing.T) {
if !pipelineAssociatedWithMR([]gitlab.Pipeline{{ID: json.Number("9")}}, "9") {
t.Fatal("matching pipeline was rejected")
}
for name, associated := range map[string][]gitlab.PipelineMergeRequest{
"missing": nil,
"mismatch": {{IID: 8}},
"ambiguous": {{IID: 7}, {IID: 8}},
"duplicated": {{IID: 7}, {IID: 7}},
for name, pipelines := range map[string][]gitlab.Pipeline{
"missing": nil,
"mismatch": {{ID: json.Number("10")}},
} {
t.Run(name, func(t *testing.T) {
if pipelineAssociatedExactly(associated, 7) {
t.Fatalf("invalid association accepted: %#v", associated)
if pipelineAssociatedWithMR(pipelines, "9") {
t.Fatalf("invalid association accepted: %#v", pipelines)
}
})
}
Expand Down
17 changes: 4 additions & 13 deletions internal/gitlab/client.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -261,13 +261,6 @@ type Pipeline struct {
Source string `json:"source"`
}

// PipelineMergeRequest is the deliberately small association shape returned
// by GitLab's pipeline-to-merge-request endpoint. The complete MR payload is
// not needed to prove the association.
type PipelineMergeRequest struct {
IID int `json:"iid"`
}

// Commit contains only immutable commit evidence needed to validate a merged
// results pipeline. ParentIDs must be checked exactly by the caller.
type Commit struct {
Expand Down Expand Up @@ -319,17 +312,15 @@ func (c Client) Pipeline(ctx context.Context, projectID, pipelineID string) (Pip
return pipeline, err
}

func (c Client) PipelineMergeRequests(ctx context.Context, projectID, pipelineID string) ([]PipelineMergeRequest, error) {
func (c Client) MergeRequestPipelines(ctx context.Context, projectID string, iid int) ([]Pipeline, error) {
prefix, err := projectPrefix(projectID)
if err != nil {
return nil, err
}
if !validDecimalID(pipelineID) {
return nil, errors.New("pipeline ID must be decimal")
if iid <= 0 {
return nil, errors.New("MR IID must be positive")
}
var requests []PipelineMergeRequest
err = c.api(ctx, prefix+"/pipelines/"+pipelineID+"/merge_requests", &requests)
return requests, err
return apiArray[Pipeline](c, ctx, fmt.Sprintf("%s/merge_requests/%d/pipelines?per_page=100", prefix, iid))
}

func (c Client) Commit(ctx context.Context, projectID, oid string) (Commit, error) {
Expand Down
19 changes: 10 additions & 9 deletions internal/gitlab/client_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,8 +101,8 @@ func TestIDsAreValidatedBeforeGlab(t *testing.T) {
if _, err := client.Pipeline(context.Background(), "1", "bad"); err == nil {
t.Fatal("unsafe pipeline ID accepted")
}
if _, err := client.PipelineMergeRequests(context.Background(), "1", "-2"); err == nil {
t.Fatal("unsafe pipeline association ID accepted")
if _, err := client.MergeRequestPipelines(context.Background(), "1", -2); err == nil {
t.Fatal("unsafe merge request IID accepted")
}
if _, err := client.Commit(context.Background(), "1", "not-an-object-id"); err == nil {
t.Fatal("unsafe commit ID accepted")
Expand All @@ -120,13 +120,13 @@ func TestPipelineEvidenceTransportUsesTypedEndpoints(t *testing.T) {
if pipeline.ID.String() != "9" || pipeline.Source != "merge_request_event" {
t.Fatalf("pipeline evidence lost: %#v", pipeline)
}
runner.stdout = []byte(`[{"iid":7}]`)
associated, err := client.PipelineMergeRequests(context.Background(), "42", "9")
runner.stdout = []byte(`[{"id":9,"ref":"refs/merge-requests/7/merge"}]`)
pipelines, err := client.MergeRequestPipelines(context.Background(), "42", 7)
if err != nil {
t.Fatal(err)
}
if len(associated) != 1 || associated[0].IID != 7 {
t.Fatalf("association evidence lost: %#v", associated)
if len(pipelines) != 1 || pipelines[0].ID.String() != "9" {
t.Fatalf("association evidence lost: %#v", pipelines)
}
runner.stdout = []byte(`{"id":"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa","parent_ids":["bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb","cccccccccccccccccccccccccccccccccccccccc"]}`)
commit, err := client.Commit(context.Background(), "42", "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa")
Expand All @@ -138,12 +138,13 @@ func TestPipelineEvidenceTransportUsesTypedEndpoints(t *testing.T) {
}
wantEndpoints := []string{
"/projects/42/pipelines/9",
"/projects/42/pipelines/9/merge_requests",
"/projects/42/merge_requests/7/pipelines?per_page=100",
"/projects/42/repository/commits/aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa",
}
for i, endpoint := range wantEndpoints {
if !reflect.DeepEqual(runner.calls[i], []string{"glab", "api", endpoint}) {
t.Fatalf("call %d=%#v want endpoint %q", i, runner.calls[i], endpoint)
got := runner.calls[i]
if len(got) < 3 || got[0] != "glab" || got[1] != "api" || got[2] != endpoint {
t.Fatalf("call %d=%#v want endpoint %q", i, got, endpoint)
}
}
}
Expand Down
Loading