fix(attest): always serialise commits in pull request attestations

cmd/kosli/attestPRGitlab.go

@@ -100,8 +100,8 @@ func newAttestGitlabPRCmd(out io.Writer) *cobra.Command {
 		payload: PRAttestationPayload{
 			CommonAttestationPayload: &CommonAttestationPayload{},
 		},
-		retriever: new(gitlabUtils.GitlabConfig),
 	}
+	gitlabFlagsValues := new(gitlabUtils.GitlabConfig)
 	cmd := &cobra.Command{
 		// Args:    cobra.MaximumNArgs(1),  // See CustomMaximumNArgs() below
 		Use:         "gitlab [IMAGE-NAME | FILE-PATH | DIR-PATH]",
@@ -146,14 +146,15 @@ func newAttestGitlabPRCmd(out io.Writer) *cobra.Command {
 			// GitlabConfig.Repository is the short project name (CI_PROJECT_NAME);
 			// combined with Org (CI_PROJECT_NAMESPACE) it forms the API ProjectID.
 			// This is separate from repo_info.name, which uses the full CI_PROJECT_PATH.
-			o.getRetriever().(*gitlabUtils.GitlabConfig).Repository = o.repoName
+			o.retriever = gitlabUtils.NewGitlabRetrieverFunc(gitlabFlagsValues.Token,
+				gitlabFlagsValues.BaseURL, gitlabFlagsValues.Org, o.repoName)
 			return o.run(args)
 		},
 	}
 
 	ci := WhichCI()
 	addAttestationFlags(cmd, o.CommonAttestationOptions, o.payload.CommonAttestationPayload, ci)
-	addGitlabFlags(cmd, o.getRetriever().(*gitlabUtils.GitlabConfig), ci)
+	addGitlabFlags(cmd, gitlabFlagsValues, ci)
 	cmd.Flags().BoolVar(&o.assert, "assert", false, assertPREvidenceFlag)
 
 	err := RequireFlags(cmd, []string{"flow", "trail", "name",

cmd/kosli/attestPRGitlab_test.go

@@ -5,7 +5,9 @@ import (
 	"os"
 	"testing"
 
+	gitlabUtils "github.com/kosli-dev/cli/internal/gitlab"
 	"github.com/kosli-dev/cli/internal/testHelpers"
+	"github.com/kosli-dev/cli/internal/types"
 	"github.com/stretchr/testify/require"
 	"github.com/stretchr/testify/suite"
 )
@@ -25,8 +27,6 @@ type AttestGitlabPRCommandTestSuite struct {
 }
 
 func (suite *AttestGitlabPRCommandTestSuite) SetupTest() {
-	testHelpers.SkipIfEnvVarUnset(suite.T(), []string{"KOSLI_GITLAB_TOKEN"})
-
 	suite.flowName = "attest-gitlab-pr"
 	suite.trailName = "test-123"
 	suite.commitWithPR = "f6d2c1a288f2c400c04e8451f4fdddb1f3b4ce01"
@@ -44,12 +44,46 @@ func (suite *AttestGitlabPRCommandTestSuite) SetupTest() {
 	_, err = testHelpers.CloneGitRepo("https://gitlab.com/kosli-dev/merkely-gitlab-demo.git", suite.tmpDir)
 	require.NoError(suite.T(), err)
 
-	suite.defaultKosliArguments = fmt.Sprintf(" --flow %s --trail %s --repo-root %s --host %s --org %s --api-token %s", suite.flowName, suite.trailName, suite.tmpDir, global.Host, global.Org, global.ApiToken)
+	// The merge request API is faked: GitLab stopped returning commits for
+	// merge requests whose diff predates ~2025-11-26, which silently broke this
+	// suite against the live API (#1081). The git repo above is still cloned for
+	// real, so commit resolution stays honest.
+	gitlabUtils.NewGitlabRetrieverFunc = func(token, baseURL, org, repository string) types.PRRetriever {
+		return &gitlabUtils.FakeGitlabClient{
+			MRsByCommit: map[string][]*types.PREvidence{
+				suite.commitWithPR: {{
+					URL:         "https://gitlab.com/kosli-dev/merkely-gitlab-demo/-/merge_requests/1",
+					State:       "merged",
+					Author:      "Test User (@test-user)",
+					Title:       "Changed readme to kosli-dev",
+					CreatedAt:   1728562890,
+					MergedAt:    1728563001,
+					HeadRef:     "update-readme",
+					BaseRef:     "main",
+					MergeCommit: suite.commitWithPR,
+					Approvers:   []any{},
+					Commits: []types.Commit{{
+						SHA:       "77fe4082df8549385462cbed0d28610f3cb59eec",
+						Message:   "Changed readme to kosli-dev",
+						Author:    "Tore Martin Hagen <tore@kosli.com>",
+						Timestamp: 1728562776,
+						Branch:    "update-readme",
+					}},
+				}},
+			},
+		}
+	}
+
+	suite.defaultKosliArguments = fmt.Sprintf(" --gitlab-token fake --flow %s --trail %s --repo-root %s --host %s --org %s --api-token %s", suite.flowName, suite.trailName, suite.tmpDir, global.Host, global.Org, global.ApiToken)
 	CreateFlowWithTemplate(suite.flowName, "testdata/valid_template.yml", suite.T())
 	BeginTrail(suite.trailName, suite.flowName, "", suite.T())
 	CreateArtifactOnTrail(suite.flowName, suite.trailName, "cli", suite.artifactFingerprint, "file1", suite.T())
 }
 
+func (suite *AttestGitlabPRCommandTestSuite) TearDownTest() {
+	gitlabUtils.ResetGitlabRetrieverFunc()
+}
+
 func (suite *AttestGitlabPRCommandTestSuite) TearDownSuite() {
 	if err := os.RemoveAll(suite.tmpDir); err != nil {
 		require.NoError(suite.T(), err, "failed to remove temp dir %s", suite.tmpDir)

internal/gitlab/fake_gitlab.go

@@ -0,0 +1,49 @@
+package gitlab
+
+import (
+	"github.com/kosli-dev/cli/internal/types"
+)
+
+// FakeGitlabClient is an in-memory implementation of types.PRRetriever for
+// testing. Seed MRsByCommit with the commits and merge request evidence you
+// want returned. Set Err to simulate a network or API failure.
+type FakeGitlabClient struct {
+	// MRsByCommit maps a commit SHA to the merge request evidence returned
+	// for that commit.
+	MRsByCommit map[string][]*types.PREvidence
+	// Err, if set, is returned by all calls regardless of commit.
+	Err error
+}
+
+func (f *FakeGitlabClient) ProviderAndLabel() (string, string) {
+	return "gitlab", "merge request"
+}
+
+// PREvidenceForCommitV2 mirrors the real client: a commit with no merge
+// requests yields an empty result and no error, because GitLab's
+// ListMergeRequestsByCommit returns an empty list rather than an error.
+//
+// The result is always non-nil. The real client builds its slice up front, and
+// a nil slice would serialise as null, which the API rejects for pull_requests.
+func (f *FakeGitlabClient) PREvidenceForCommitV2(commit string) ([]*types.PREvidence, error) {
+	if f.Err != nil {
+		return nil, f.Err
+	}
+	mrs := f.MRsByCommit[commit]
+	if mrs == nil {
+		return []*types.PREvidence{}, nil
+	}
+	return mrs, nil
+}
+
+// PREvidenceForCommitV1 mirrors the real client, which serves V1 from the same
+// merge request lookup as V2.
+func (f *FakeGitlabClient) PREvidenceForCommitV1(commit string) ([]*types.PREvidence, error) {
+	return f.PREvidenceForCommitV2(commit)
+}
+
+// PREvidenceForCommitHybrid mirrors the real client, which has no V1 fallback
+// for GitLab and always serves V2.
+func (f *FakeGitlabClient) PREvidenceForCommitHybrid(commit string) ([]*types.PREvidence, error) {
+	return f.PREvidenceForCommitV2(commit)
+}

internal/gitlab/fake_gitlab_test.go

@@ -0,0 +1,39 @@
+package gitlab
+
+import (
+	"testing"
+
+	"github.com/kosli-dev/cli/internal/types"
+	"github.com/stretchr/testify/require"
+)
+
+// The real client builds its result slice up front, so a commit with no merge
+// requests yields an empty list rather than nil. The fake must match: a nil
+// slice serialises as null, and the API rejects null for pull_requests.
+func TestFakeGitlabClientReturnsEmptyNotNilForUnknownCommit(t *testing.T) {
+	client := &FakeGitlabClient{
+		MRsByCommit: map[string][]*types.PREvidence{
+			"known": {{URL: "https://gitlab.com/org/repo/-/merge_requests/1"}},
+			// an explicitly seeded nil must be normalised too
+			"seeded-nil": nil,
+		},
+	}
+
+	for _, retrieve := range []struct {
+		name string
+		call func(string) ([]*types.PREvidence, error)
+	}{
+		{"V2", client.PREvidenceForCommitV2},
+		{"V1", client.PREvidenceForCommitV1},
+		{"Hybrid", client.PREvidenceForCommitHybrid},
+	} {
+		for _, commit := range []string{"unknown", "seeded-nil"} {
+			t.Run(retrieve.name+"/"+commit, func(t *testing.T) {
+				mrs, err := retrieve.call(commit)
+				require.NoError(t, err)
+				require.NotNil(t, mrs, "must be an empty slice, not nil")
+				require.Empty(t, mrs)
+			})
+		}
+	}
+}

internal/gitlab/gitlab.go

@@ -43,6 +43,24 @@ func (c *GitlabConfig) ProviderAndLabel() (string, string) {
 	return "gitlab", "merge request"
 }
 
+// NewGitlabRetrieverFunc creates a types.PRRetriever from GitLab config
+// parameters. It can be replaced in tests to inject a FakeGitlabClient.
+var NewGitlabRetrieverFunc = defaultNewGitlabRetriever
+
+func defaultNewGitlabRetriever(token, baseURL, org, repository string) types.PRRetriever {
+	return &GitlabConfig{
+		Token:      token,
+		BaseURL:    baseURL,
+		Org:        org,
+		Repository: repository,
+	}
+}
+
+// ResetGitlabRetrieverFunc restores NewGitlabRetrieverFunc to its default.
+func ResetGitlabRetrieverFunc() {
+	NewGitlabRetrieverFunc = defaultNewGitlabRetriever
+}
+
 // This is the old implementation, it will be removed after the PR payload is enhanced for all VCS providers
 func (c *GitlabConfig) PREvidenceForCommitV1(commit string) ([]*types.PREvidence, error) {
 	pullRequestsEvidence := []*types.PREvidence{}

internal/types/types.go

@@ -1,5 +1,7 @@
 package types
 
+import "encoding/json"
+
 type PREvidence struct {
 	MergeCommit string   `json:"merge_commit"`
 	URL         string   `json:"url"`
@@ -11,7 +13,20 @@ type PREvidence struct {
 	Title       string   `json:"title,omitempty"`
 	HeadRef     string   `json:"head_ref,omitempty"`
 	BaseRef     string   `json:"base_ref,omitempty"`
-	Commits     []Commit `json:"commits,omitempty"`
+	Commits     []Commit `json:"commits"`
+}
+
+// MarshalJSON keeps "commits" in the payload even when a provider returns no
+// commits. The API requires the field on FoundPullRequestV2, so omitting it —
+// or sending null for a nil slice — produces a payload matching neither
+// attestation schema (#1081).
+func (e PREvidence) MarshalJSON() ([]byte, error) {
+	type prEvidence PREvidence // sheds the method set, so this does not recurse
+	out := prEvidence(e)
+	if out.Commits == nil {
+		out.Commits = []Commit{}
+	}
+	return json.Marshal(out)
 }
 
 type PRApprovals struct {

internal/types/types_test.go

@@ -0,0 +1,42 @@
+package types
+
+import (
+	"encoding/json"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+// The API validates pull request attestations against FoundPullRequestV2, which
+// requires "commits". Dropping the field when a provider returns no commits
+// produces a payload that matches neither V1 nor V2 and is rejected (#1081).
+func TestPREvidenceAlwaysSerialisesCommits(t *testing.T) {
+	for _, tc := range []struct {
+		name     string
+		evidence PREvidence
+		want     string
+	}{
+		{
+			name:     "empty commits serialise as an empty array",
+			evidence: PREvidence{Commits: []Commit{}},
+			want:     `[]`,
+		},
+		{
+			name:     "nil commits serialise as an empty array",
+			evidence: PREvidence{},
+			want:     `[]`,
+		},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			payload, err := json.Marshal(tc.evidence)
+			require.NoError(t, err)
+
+			var decoded map[string]json.RawMessage
+			require.NoError(t, json.Unmarshal(payload, &decoded))
+
+			raw, present := decoded["commits"]
+			require.True(t, present, "commits must always be present in the payload")
+			require.JSONEq(t, tc.want, string(raw))
+		})
+	}
+}