From 072fb2fb19caae803bfd50f07b003c2d30d36a10 Mon Sep 17 00:00:00 2001
From: Alex Kantor <alex@kosli.com>
Date: Wed, 21 Oct 2026 17:41:01 +0100
Subject: [PATCH 01/10] test: add helpers for feeding PR evidence code replies
 with null fields

Part of #1283. Provider tests use these to send their evidence code a
realistic reply once per field with that field made null, or a list
emptied, and to name the outcome: ok, error or panic, plus which
evidence fields came back changed or missing.
---
 internal/testHelpers/oddReplies.go      | 291 ++++++++++++++++++++++++
 internal/testHelpers/oddReplies_test.go | 120 ++++++++++
 2 files changed, 411 insertions(+)
 create mode 100644 internal/testHelpers/oddReplies.go
 create mode 100644 internal/testHelpers/oddReplies_test.go

diff --git a/internal/testHelpers/oddReplies.go b/internal/testHelpers/oddReplies.go
new file mode 100644
index 000000000..dd9cd29fa
--- /dev/null
+++ b/internal/testHelpers/oddReplies.go
@@ -0,0 +1,291 @@
+package testHelpers
+
+import (
+	"bytes"
+	"encoding/json"
+	"fmt"
+	"io"
+	"reflect"
+	"sort"
+
+	"github.com/kosli-dev/cli/internal/types"
+)
+
+// NullVariant is a copy of a JSON document with the value at Path set to null.
+// A Path ending in "[]" names a list emptied instead.
+type NullVariant struct {
+	Path string
+	Body []byte
+}
+
+// NullVariants returns one copy of doc per value it holds, with that value
+// made null, plus an emptied copy of each non-empty list. The document itself
+// is Path "."; its fields have no leading dot. Key order is kept; a variant
+// means the same as doc except where its Path says.
+func NullVariants(doc []byte) ([]NullVariant, error) {
+	root, err := parseOrdered(doc)
+	if err != nil {
+		return nil, err
+	}
+	variants := nullAt(root, root, ".")
+	var walk func(n *jsonNode, path string)
+	walk = func(n *jsonNode, path string) {
+		switch n.kind {
+		case kindObject:
+			for _, f := range n.fields {
+				child := joinPath(path, f.key)
+				variants = append(variants, nullAt(root, f.value, child)...)
+				walk(f.value, child)
+			}
+		case kindArray:
+			for i, item := range n.items {
+				child := fmt.Sprintf("%s[%d]", path, i)
+				variants = append(variants, nullAt(root, item, child)...)
+				walk(item, child)
+			}
+		}
+	}
+	walk(root, "")
+	return variants, nil
+}
+
+// nullAt gives root rendered with n made null and, for a non-empty list,
+// rendered with it emptied. An already-null n gives nothing.
+func nullAt(root, n *jsonNode, path string) []NullVariant {
+	if n.kind == kindNull {
+		return nil
+	}
+	saved := *n
+	defer func() { *n = saved }()
+
+	*n = jsonNode{kind: kindNull}
+	out := []NullVariant{{Path: path, Body: root.render()}}
+	if saved.kind == kindArray && len(saved.items) > 0 {
+		*n = jsonNode{kind: kindArray}
+		out = append(out, NullVariant{Path: path + "[]", Body: root.render()})
+	}
+	return out
+}
+
+func joinPath(parent, key string) string {
+	if parent == "" {
+		return key
+	}
+	return parent + "." + key
+}
+
+// Outcome runs fn and says what happened: "ok", "error", or "panic: " and the
+// panic value. Error text is left out because it carries test server ports.
+func Outcome(fn func() error) (out string) {
+	defer func() {
+		if r := recover(); r != nil {
+			out = fmt.Sprintf("panic: %v", r)
+		}
+	}()
+	if err := fn(); err != nil {
+		return "error"
+	}
+	return "ok"
+}
+
+// EvidenceOutcome runs fetch and says what happened, as Outcome does. When it
+// succeeds it compares each pull request's API payload with baseline's, key by
+// key, so it sees what the server would. "lost:" when a value the baseline had
+// came back missing or null, or content it had came back empty or shorter;
+// "changed:" for any other difference.
+func EvidenceOutcome(baseline []*types.PREvidence, fetch func() ([]*types.PREvidence, error)) string {
+	var got []*types.PREvidence
+	out := Outcome(func() error {
+		var err error
+		got, err = fetch()
+		return err
+	})
+	if out != "ok" {
+		return out
+	}
+	if len(got) != len(baseline) {
+		return fmt.Sprintf("ok lost:PullRequests(%d->%d)", len(baseline), len(got))
+	}
+	for i := range baseline {
+		if got[i] == nil {
+			out += fmt.Sprintf(" lost:PullRequest%d", i)
+			continue
+		}
+		want, have := payloadFields(baseline[i]), payloadFields(got[i])
+		keys := []string{}
+		for k := range want {
+			keys = append(keys, k)
+		}
+		for k := range have {
+			if _, ok := want[k]; !ok {
+				keys = append(keys, k)
+			}
+		}
+		sort.Strings(keys)
+		for _, k := range keys {
+			w, h := want[k], have[k]
+			if reflect.DeepEqual(w, h) {
+				continue
+			}
+			kind := "changed:"
+			if (w != nil && h == nil) || (!isEmptyValue(w) && (isEmptyValue(h) || shorter(h, w))) {
+				kind = "lost:"
+			}
+			out += " " + kind + k
+		}
+	}
+	return out
+}
+
+// payloadFields is e as the API receives it, one entry per top-level key.
+func payloadFields(e *types.PREvidence) map[string]any {
+	raw, err := json.Marshal(e)
+	if err != nil {
+		panic(err)
+	}
+	fields := map[string]any{}
+	dec := json.NewDecoder(bytes.NewReader(raw))
+	dec.UseNumber()
+	if err := dec.Decode(&fields); err != nil {
+		panic(err)
+	}
+	return fields
+}
+
+func isEmptyValue(v any) bool {
+	switch x := v.(type) {
+	case nil:
+		return true
+	case string:
+		return x == ""
+	case json.Number:
+		return x == "0"
+	case []any:
+		return len(x) == 0
+	}
+	return false
+}
+
+func shorter(have, want any) bool {
+	h, ok1 := have.([]any)
+	w, ok2 := want.([]any)
+	return ok1 && ok2 && len(h) < len(w)
+}
+
+type jsonKind int
+
+const (
+	kindScalar jsonKind = iota
+	kindNull
+	kindObject
+	kindArray
+)
+
+// jsonNode keeps object keys in document order, which encoding/json maps lose.
+type jsonNode struct {
+	kind   jsonKind
+	scalar json.RawMessage
+	fields []jsonField
+	items  []*jsonNode
+}
+
+type jsonField struct {
+	key   string
+	value *jsonNode
+}
+
+func parseOrdered(doc []byte) (*jsonNode, error) {
+	dec := json.NewDecoder(bytes.NewReader(doc))
+	dec.UseNumber()
+	n, err := decodeNode(dec)
+	if err != nil {
+		return nil, err
+	}
+	if _, err := dec.Token(); err != io.EOF {
+		return nil, fmt.Errorf("trailing data after JSON document")
+	}
+	return n, nil
+}
+
+func decodeNode(dec *json.Decoder) (*jsonNode, error) {
+	tok, err := dec.Token()
+	if err != nil {
+		return nil, err
+	}
+	switch v := tok.(type) {
+	case json.Delim:
+		switch v {
+		case '{':
+			n := &jsonNode{kind: kindObject}
+			for dec.More() {
+				keyTok, err := dec.Token()
+				if err != nil {
+					return nil, err
+				}
+				value, err := decodeNode(dec)
+				if err != nil {
+					return nil, err
+				}
+				n.fields = append(n.fields, jsonField{key: keyTok.(string), value: value})
+			}
+			_, err := dec.Token()
+			return n, err
+		case '[':
+			n := &jsonNode{kind: kindArray}
+			for dec.More() {
+				item, err := decodeNode(dec)
+				if err != nil {
+					return nil, err
+				}
+				n.items = append(n.items, item)
+			}
+			_, err := dec.Token()
+			return n, err
+		}
+		return nil, fmt.Errorf("unexpected delimiter %v", v)
+	case nil:
+		return &jsonNode{kind: kindNull}, nil
+	default:
+		raw, err := json.Marshal(v)
+		if err != nil {
+			return nil, err
+		}
+		return &jsonNode{kind: kindScalar, scalar: raw}, nil
+	}
+}
+
+func (n *jsonNode) render() []byte {
+	var b bytes.Buffer
+	n.writeTo(&b)
+	return b.Bytes()
+}
+
+func (n *jsonNode) writeTo(b *bytes.Buffer) {
+	switch n.kind {
+	case kindNull:
+		b.WriteString("null")
+	case kindScalar:
+		b.Write(n.scalar)
+	case kindObject:
+		b.WriteByte('{')
+		for i, f := range n.fields {
+			if i > 0 {
+				b.WriteByte(',')
+			}
+			key, _ := json.Marshal(f.key)
+			b.Write(key)
+			b.WriteByte(':')
+			f.value.writeTo(b)
+		}
+		b.WriteByte('}')
+	case kindArray:
+		b.WriteByte('[')
+		for i, item := range n.items {
+			if i > 0 {
+				b.WriteByte(',')
+			}
+			item.writeTo(b)
+		}
+		b.WriteByte(']')
+	}
+}
diff --git a/internal/testHelpers/oddReplies_test.go b/internal/testHelpers/oddReplies_test.go
new file mode 100644
index 000000000..3fd5672b3
--- /dev/null
+++ b/internal/testHelpers/oddReplies_test.go
@@ -0,0 +1,120 @@
+package testHelpers
+
+import (
+	"errors"
+	"testing"
+
+	"github.com/kosli-dev/cli/internal/types"
+	"github.com/stretchr/testify/require"
+)
+
+func TestNullVariants_NullsEachValueInTurn(t *testing.T) {
+	variants, err := NullVariants([]byte(`{"a":{"b":1,"c":"x"},"d":[{"e":true},{"e":false}],"f":[]}`))
+	require.NoError(t, err)
+
+	got := map[string]string{}
+	for _, v := range variants {
+		got[v.Path] = string(v.Body)
+	}
+	require.Equal(t, map[string]string{
+		".":      `null`,
+		"a":      `{"a":null,"d":[{"e":true},{"e":false}],"f":[]}`,
+		"a.b":    `{"a":{"b":null,"c":"x"},"d":[{"e":true},{"e":false}],"f":[]}`,
+		"a.c":    `{"a":{"b":1,"c":null},"d":[{"e":true},{"e":false}],"f":[]}`,
+		"d":      `{"a":{"b":1,"c":"x"},"d":null,"f":[]}`,
+		"d[]":    `{"a":{"b":1,"c":"x"},"d":[],"f":[]}`,
+		"d[0]":   `{"a":{"b":1,"c":"x"},"d":[null,{"e":false}],"f":[]}`,
+		"d[0].e": `{"a":{"b":1,"c":"x"},"d":[{"e":null},{"e":false}],"f":[]}`,
+		"d[1]":   `{"a":{"b":1,"c":"x"},"d":[{"e":true},null],"f":[]}`,
+		"d[1].e": `{"a":{"b":1,"c":"x"},"d":[{"e":true},{"e":null}],"f":[]}`,
+		"f":      `{"a":{"b":1,"c":"x"},"d":[{"e":true},{"e":false}],"f":null}`,
+	}, got)
+}
+
+func TestNullVariants_LeavesAnAlreadyNullValueAlone(t *testing.T) {
+	variants, err := NullVariants([]byte(`{"a":null,"b":1}`))
+	require.NoError(t, err)
+	paths := []string{}
+	for _, v := range variants {
+		paths = append(paths, v.Path)
+	}
+	require.Equal(t, []string{".", "b"}, paths)
+}
+
+func TestNullVariants_RejectsInvalidJSON(t *testing.T) {
+	_, err := NullVariants([]byte(`{`))
+	require.Error(t, err)
+}
+
+func TestOutcome(t *testing.T) {
+	require.Equal(t, "ok", Outcome(func() error { return nil }))
+	require.Equal(t, "error", Outcome(func() error { return errors.New("boom") }))
+	require.Equal(t, "panic: runtime error: invalid memory address or nil pointer dereference",
+		Outcome(func() error {
+			var p *struct{ x int }
+			return errors.New(string(rune(p.x)))
+		}))
+}
+
+func TestEvidenceOutcome(t *testing.T) {
+	baseline := []*types.PREvidence{{URL: "u", Title: "t", Commits: []types.Commit{{SHA: "a"}, {SHA: "b"}}}}
+	fetch := func(e ...*types.PREvidence) func() ([]*types.PREvidence, error) {
+		return func() ([]*types.PREvidence, error) { return e, nil }
+	}
+
+	require.Equal(t, "ok", EvidenceOutcome(baseline, fetch(baseline[0])))
+	require.Equal(t, "ok lost:commits lost:title", EvidenceOutcome(baseline,
+		fetch(&types.PREvidence{URL: "u", Commits: []types.Commit{{SHA: "a"}}})))
+	require.Equal(t, "ok changed:commits changed:title", EvidenceOutcome(baseline,
+		fetch(&types.PREvidence{URL: "u", Title: "x", Commits: []types.Commit{{SHA: "a"}, {SHA: ""}}})))
+	require.Equal(t, "ok lost:PullRequests(1->0)", EvidenceOutcome(baseline, fetch()))
+	require.Equal(t, "error", EvidenceOutcome(baseline, func() ([]*types.PREvidence, error) {
+		return nil, errors.New("boom")
+	}))
+}
+
+func TestNullVariants_NullsAndEmptiesARootList(t *testing.T) {
+	variants, err := NullVariants([]byte(`[{"a":1}]`))
+	require.NoError(t, err)
+	got := map[string]string{}
+	for _, v := range variants {
+		got[v.Path] = string(v.Body)
+	}
+	require.Equal(t, map[string]string{
+		".":     `null`,
+		".[]":   `[]`,
+		"[0]":   `[null]`,
+		"[0].a": `[{"a":null}]`,
+	}, got)
+}
+
+func TestNullVariants_RejectsTrailingData(t *testing.T) {
+	_, err := NullVariants([]byte(`{"a":1} garbage`))
+	require.Error(t, err)
+}
+
+func TestEvidenceOutcome_ReportsANilEntryWithoutPanicking(t *testing.T) {
+	baseline := []*types.PREvidence{{URL: "u"}}
+	require.Equal(t, "ok lost:PullRequest0", EvidenceOutcome(baseline, func() ([]*types.PREvidence, error) {
+		return []*types.PREvidence{nil}, nil
+	}))
+}
+
+func TestEvidenceOutcome_ComparesWhatTheAPIReceives(t *testing.T) {
+	baseline := []*types.PREvidence{{URL: "u", Reviews: []types.PRApprovals{}}}
+
+	// Commits are always sent as a list, so nil and empty are the same payload.
+	require.Equal(t, "ok", EvidenceOutcome([]*types.PREvidence{{URL: "u", Commits: []types.Commit{}}},
+		func() ([]*types.PREvidence, error) { return []*types.PREvidence{{URL: "u"}}, nil }))
+	// No reviews key means reviews were not collected, unlike an empty list.
+	require.Equal(t, "ok lost:reviews", EvidenceOutcome(baseline,
+		func() ([]*types.PREvidence, error) { return []*types.PREvidence{{URL: "u"}}, nil }))
+	// A key only the result has is still compared.
+	require.Equal(t, "ok changed:reviews changed:title", EvidenceOutcome([]*types.PREvidence{{URL: "u"}},
+		func() ([]*types.PREvidence, error) {
+			return []*types.PREvidence{{URL: "u", Title: "t", Reviews: []types.PRApprovals{}}}, nil
+		}))
+	// Empty in both is not a loss.
+	require.Equal(t, "ok changed:approvers", EvidenceOutcome([]*types.PREvidence{{URL: "u"}},
+		func() ([]*types.PREvidence, error) { return []*types.PREvidence{{URL: "u", Approvers: []any{}}}, nil }))
+}

From 8cbda4f097d8cc92438200dabd5488a96abb7420 Mon Sep 17 00:00:00 2001
From: Alex Kantor <alex@kosli.com>
Date: Wed, 21 Oct 2026 18:05:03 +0100
Subject: [PATCH 02/10] fix(gitlab): never panic on null fields in merge
 request replies

The evidence code read times, users and list entries from GitLab
replies without nil checks, so a null merged_at (an MR that has not
merged) panicked assert pullrequest gitlab and attest pullrequest
gitlab.

Null times now give 0 and null users an empty name: that part of the
evidence is empty and the rest is kept. A null entry in a list (MRs,
approved_by, reviewers, commits) is an error naming the list and
index, because it is missing data, not an empty entry. A null
approvals or signature reply reads as none.

The new test feeds both command entry points a realistic reply with
each value made null, or each list emptied, and asserts that rule.

Part of #1283

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
---
 internal/gitlab/gitlab.go           |  76 +++++++--
 internal/gitlab/odd_replies_test.go | 237 ++++++++++++++++++++++++++++
 2 files changed, 299 insertions(+), 14 deletions(-)
 create mode 100644 internal/gitlab/odd_replies_test.go

diff --git a/internal/gitlab/gitlab.go b/internal/gitlab/gitlab.go
index 4e886747c..d3bcd1602 100644
--- a/internal/gitlab/gitlab.go
+++ b/internal/gitlab/gitlab.go
@@ -3,6 +3,7 @@ package gitlab
 import (
 	"fmt"
 	"net/http"
+	"time"
 
 	"github.com/kosli-dev/cli/internal/types"
 	gitlab "gitlab.com/gitlab-org/api/client-go"
@@ -121,9 +122,9 @@ func (c *GitlabConfig) newPRGitlabEvidenceV2(mr *gitlab.BasicMergeRequest) (*typ
 		URL:         mr.WebURL,
 		MergeCommit: mr.MergeCommitSHA,
 		State:       mr.State,
-		Author:      fmt.Sprintf("%s (@%s)", mr.Author.Name, mr.Author.Username),
-		CreatedAt:   mr.CreatedAt.Unix(),
-		MergedAt:    mr.MergedAt.Unix(),
+		Author:      gitlabUserName(mr.Author),
+		CreatedAt:   unixOrZero(mr.CreatedAt),
+		MergedAt:    unixOrZero(mr.MergedAt),
 		Title:       mr.Title,
 	}
 	evidence.HeadRef, evidence.BaseRef = gitlabMRRefs(mr)
@@ -153,7 +154,7 @@ func (c *GitlabConfig) MergeRequestsForCommit(commit string) ([]*gitlab.BasicMer
 	if err != nil {
 		return mrs, fmt.Errorf("failed to list merge requests for commit %s: %w", commit, err)
 	}
-	return mrs, nil
+	return mrs, noNullItems("merge requests", mrs)
 }
 
 // GetMergeRequestApprovers returns a list of users (name and username) who approved an MR
@@ -163,11 +164,20 @@ func (c *GitlabConfig) GetMergeRequestApprovers(mrIID, version int64) ([]any, er
 	if err != nil {
 		return approvers, err
 	}
-	approvals, _, err := client.MergeRequestApprovals.GetConfiguration(c.ProjectID(), mrIID)
+	approvedBy, err := c.approvedBy(client, mrIID)
 	if err != nil {
 		return approvers, err
 	}
-	return gitlabApprovers(approvals.ApprovedBy, version), nil
+	return gitlabApprovers(approvedBy, version), nil
+}
+
+// approvedBy returns who approved an MR.
+func (c *GitlabConfig) approvedBy(client *gitlab.Client, mrIID int64) ([]*gitlab.MergeRequestApproverUser, error) {
+	approvals, _, err := client.MergeRequestApprovals.GetConfiguration(c.ProjectID(), mrIID)
+	if err != nil || approvals == nil {
+		return nil, err
+	}
+	return approvals.ApprovedBy, noNullItems("approved_by", approvals.ApprovedBy)
 }
 
 // GetMergeRequestReviews returns an MR's approvers and its reviews: each
@@ -177,11 +187,11 @@ func (c *GitlabConfig) GetMergeRequestReviews(mrIID int64) ([]any, []types.PRApp
 	if err != nil {
 		return nil, nil, err
 	}
-	approvals, _, err := client.MergeRequestApprovals.GetConfiguration(c.ProjectID(), mrIID)
+	approvedBy, err := c.approvedBy(client, mrIID)
 	if err != nil {
 		return nil, nil, err
 	}
-	approvers := gitlabApprovers(approvals.ApprovedBy, 2)
+	approvers := gitlabApprovers(approvedBy, 2)
 	reviewers, resp, err := client.MergeRequests.GetMergeRequestReviewers(c.ProjectID(), mrIID)
 	if err != nil {
 		// Older GitLab has no reviewers endpoint. Reviews without its change
@@ -191,13 +201,45 @@ func (c *GitlabConfig) GetMergeRequestReviews(mrIID int64) ([]any, []types.PRApp
 		}
 		return nil, nil, err
 	}
-	return approvers, gitlabReviews(approvals.ApprovedBy, reviewers), nil
+	if err := noNullItems("reviewers", reviewers); err != nil {
+		return nil, nil, err
+	}
+	return approvers, gitlabReviews(approvedBy, reviewers), nil
 }
 
 func gitlabUserName(u *gitlab.BasicUser) string {
+	if u == nil {
+		return ""
+	}
 	return fmt.Sprintf("%s (@%s)", u.Name, u.Username)
 }
 
+func gitlabUsername(u *gitlab.BasicUser) string {
+	if u == nil {
+		return ""
+	}
+	return u.Username
+}
+
+// unixOrZero is t in Unix seconds, or 0 when GitLab sent no time.
+func unixOrZero(t *time.Time) int64 {
+	if t == nil {
+		return 0
+	}
+	return t.Unix()
+}
+
+// noNullItems errors on a null entry in a list GitLab sent: it is missing
+// data, not an empty entry.
+func noNullItems[T any](list string, items []*T) error {
+	for i, item := range items {
+		if item == nil {
+			return fmt.Errorf("GitLab sent %s[%d] as null", list, i)
+		}
+	}
+	return nil
+}
+
 func gitlabApprovers(approvedBy []*gitlab.MergeRequestApproverUser, version int64) []any {
 	var approvers []any
 	for _, approver := range approvedBy {
@@ -219,13 +261,13 @@ func gitlabApprovers(approvedBy []*gitlab.MergeRequestApproverUser, version int6
 func gitlabReviews(approvedBy []*gitlab.MergeRequestApproverUser, reviewers []*gitlab.MergeRequestReviewer) []types.PRApprovals {
 	reviewerStates := map[string]string{}
 	for _, r := range reviewers {
-		reviewerStates[r.User.Username] = r.State
+		reviewerStates[gitlabUsername(r.User)] = r.State
 	}
 	reviews := []types.PRApprovals{}
 	approved := map[string]bool{}
 	for _, a := range approvedBy {
-		approved[a.User.Username] = true
-		if reviewerStates[a.User.Username] != "requested_changes" {
+		approved[gitlabUsername(a.User)] = true
+		if reviewerStates[gitlabUsername(a.User)] != "requested_changes" {
 			reviews = append(reviews, types.PRApprovals{Username: gitlabUserName(a.User), State: types.ReviewApproved})
 		}
 	}
@@ -234,7 +276,7 @@ func gitlabReviews(approvedBy []*gitlab.MergeRequestApproverUser, reviewers []*g
 		switch {
 		case r.State == "requested_changes":
 			reviews = append(reviews, types.PRApprovals{Username: name, State: types.ReviewChangesRequested})
-		case r.State == "reviewed" && !approved[r.User.Username]:
+		case r.State == "reviewed" && !approved[gitlabUsername(r.User)]:
 			reviews = append(reviews, types.PRApprovals{Username: name, State: types.ReviewCommented})
 		}
 	}
@@ -252,6 +294,9 @@ func (c *GitlabConfig) GetMergeRequestCommits(mr *gitlab.BasicMergeRequest) ([]t
 	if err != nil {
 		return commits, err
 	}
+	if err := noNullItems("commits", glCommits); err != nil {
+		return commits, err
+	}
 	// One signature lookup per commit: pagination multiplies this, but GitLab
 	// has no batch signature endpoint.
 	for _, commit := range glCommits {
@@ -273,7 +318,7 @@ func (c *GitlabConfig) GetMergeRequestCommits(mr *gitlab.BasicMergeRequest) ([]t
 func commitFromGitlabCommit(commit *gitlab.Commit, branch string) types.Commit {
 	// Use the authored date to match the recorded author identity; fall back to
 	// created_at if the API omits it (server#5479).
-	timestamp := commit.CreatedAt.Unix()
+	timestamp := unixOrZero(commit.CreatedAt)
 	if commit.AuthoredDate != nil {
 		timestamp = commit.AuthoredDate.Unix()
 	}
@@ -311,6 +356,9 @@ func resolveGitlabSignature(sig *gitlab.GPGSignature, resp *gitlab.Response, err
 		}
 		return nil, nil, err
 	}
+	if sig == nil {
+		return nil, nil, nil
+	}
 	verified, signatureState := gitlabCommitVerification(sig.VerificationStatus)
 	return verified, signatureState, nil
 }
diff --git a/internal/gitlab/odd_replies_test.go b/internal/gitlab/odd_replies_test.go
new file mode 100644
index 000000000..a55b3238f
--- /dev/null
+++ b/internal/gitlab/odd_replies_test.go
@@ -0,0 +1,237 @@
+package gitlab
+
+import (
+	"fmt"
+	"net/http"
+	"net/http/httptest"
+	"regexp"
+	"strings"
+	"testing"
+
+	"github.com/kosli-dev/cli/internal/testHelpers"
+	"github.com/kosli-dev/cli/internal/types"
+	"github.com/stretchr/testify/require"
+)
+
+// Baseline replies for a merged MR (iid 101) with 2 commits, 2 reviewers and
+// 1 approval, so every field PR evidence reads is present. Shapes follow
+// GitLab's documented examples; values are made up.
+
+// https://docs.gitlab.com/api/commits/#list-merge-requests-associated-with-a-commit
+const oddMRsForCommit = `[{
+  "id": 4501, "iid": 101, "project_id": 35,
+  "title": "Add audit log", "description": "Adds an audit log.",
+  "state": "merged",
+  "created_at": "2026-03-26T17:26:30.916Z", "updated_at": "2026-03-27T09:12:00.000Z",
+  "merged_at": "2026-03-27T09:12:00.000Z",
+  "merge_user": {"id": 28, "username": "janedoe", "name": "Jane Doe", "state": "active",
+    "avatar_url": "https://gitlab.example.com/uploads/user/avatar/28/a.png", "web_url": "https://gitlab.example.com/janedoe"},
+  "closed_by": null, "closed_at": null,
+  "target_branch": "main", "source_branch": "audit-log",
+  "upvotes": 0, "downvotes": 0,
+  "author": {"id": 28, "username": "janedoe", "name": "Jane Doe", "state": "active",
+    "avatar_url": "https://gitlab.example.com/uploads/user/avatar/28/a.png", "web_url": "https://gitlab.example.com/janedoe"},
+  "assignee": null,
+  "source_project_id": 35, "target_project_id": 35,
+  "labels": ["audit"], "draft": false, "milestone": null,
+  "merge_when_pipeline_succeeds": false, "detailed_merge_status": "not_open",
+  "sha": "c2a6f2b1e0d94e3f8a7b6c5d4e3f2a1b0c9d8e7f",
+  "merge_commit_sha": "9f8e7d6c5b4a39281706f5e4d3c2b1a098765432",
+  "squash_commit_sha": null,
+  "user_notes_count": 2, "discussion_locked": null,
+  "should_remove_source_branch": true, "force_remove_source_branch": false,
+  "web_url": "https://gitlab.example.com/acme/app/-/merge_requests/101",
+  "time_stats": {"time_estimate": 0, "total_time_spent": 0, "human_time_estimate": null, "human_total_time_spent": null}
+}]`
+
+// https://docs.gitlab.com/api/merge_request_approvals/#retrieve-approval-state-for-a-merge-request
+const oddApprovals = `{
+  "id": 4501, "iid": 101, "project_id": 35,
+  "title": "Add audit log", "description": "Adds an audit log.",
+  "state": "merged",
+  "created_at": "2026-03-26T17:26:30.916Z", "updated_at": "2026-03-27T09:12:00.000Z",
+  "merge_status": "can_be_merged",
+  "approved": true, "approvals_required": 1, "approvals_left": 0,
+  "approved_by": [{"user": {"id": 1, "username": "user1", "name": "John Doe1", "state": "active",
+    "avatar_url": "https://gitlab.example.com/uploads/user/avatar/1/a.png", "web_url": "https://gitlab.example.com/user1"}}]
+}`
+
+// https://docs.gitlab.com/api/merge_requests/#get-single-merge-request-reviewers
+const oddReviewers = `[
+  {"user": {"id": 1, "username": "user1", "name": "John Doe1", "state": "active",
+    "avatar_url": "https://gitlab.example.com/uploads/user/avatar/1/a.png", "web_url": "https://gitlab.example.com/user1"},
+   "state": "approved", "created_at": "2026-03-26T18:03:27.684Z"},
+  {"user": {"id": 2, "username": "user2", "name": "John Doe2", "state": "active",
+    "avatar_url": "https://gitlab.example.com/uploads/user/avatar/2/a.png", "web_url": "https://gitlab.example.com/user2"},
+   "state": "reviewed", "created_at": "2026-03-26T18:03:27.684Z"}
+]`
+
+// https://docs.gitlab.com/api/merge_requests/#get-single-merge-request-commits
+const oddMRCommits = `[
+  {"id": "ed899a2f4b50b4370feeea94676502b42383c746", "short_id": "ed899a2f4b5",
+   "title": "Add audit log table", "message": "Add audit log table\n",
+   "author_name": "Example User", "author_email": "user@example.com", "authored_date": "2026-03-26T11:50:22+03:00",
+   "committer_name": "Example User", "committer_email": "user@example.com", "committed_date": "2026-03-26T11:50:22+03:00",
+   "created_at": "2026-03-26T11:50:22+03:00",
+   "parent_ids": ["ae1d9fb46aa2b07ee9836d49862ec4e2c46fbbba"],
+   "trailers": {}, "extended_trailers": {},
+   "web_url": "https://gitlab.example.com/acme/app/-/commit/ed899a2f4b50b4370feeea94676502b42383c746"},
+  {"id": "6104942438c14ec7bd21c6cd5bd995272b3faff6", "short_id": "6104942438c",
+   "title": "Write audit entries", "message": "Write audit entries\n",
+   "author_name": "Example User", "author_email": "user@example.com", "authored_date": "2026-03-26T12:10:01+03:00",
+   "committer_name": "Example User", "committer_email": "user@example.com", "committed_date": "2026-03-26T12:10:01+03:00",
+   "created_at": "2026-03-26T12:10:01+03:00",
+   "parent_ids": ["ed899a2f4b50b4370feeea94676502b42383c746"],
+   "trailers": {}, "extended_trailers": {},
+   "web_url": "https://gitlab.example.com/acme/app/-/commit/6104942438c14ec7bd21c6cd5bd995272b3faff6"}
+]`
+
+// https://docs.gitlab.com/api/commits/#retrieve-commit-signature
+// Served for every commit in oddMRCommits; a variant changes both.
+const oddSignature = `{
+  "signature_type": "PGP", "verification_status": "verified",
+  "gpg_key_id": 1, "gpg_key_primary_keyid": "8254AAB3FBD54AC9",
+  "gpg_key_user_name": "John Doe", "gpg_key_user_email": "johndoe@example.com",
+  "gpg_key_subkey_id": null, "commit_source": "gitaly"
+}`
+
+const oddCommit = "c2a6f2b1e0d94e3f8a7b6c5d4e3f2a1b0c9d8e7f"
+
+// oddReplyNames are the replies in the order PREvidenceForCommitV2 asks.
+var oddReplyNames = []string{"mrs_for_commit", "approvals", "reviewers", "mr_commits", "signature"}
+
+func oddBaseline() map[string]string {
+	return map[string]string{
+		"mrs_for_commit": oddMRsForCommit,
+		"approvals":      oddApprovals,
+		"reviewers":      oddReviewers,
+		"mr_commits":     oddMRCommits,
+		"signature":      oddSignature,
+	}
+}
+
+// oddRepliesServer answers like GitLab: only MR 101 and the two baseline
+// commits exist, anything else is a 404.
+func oddRepliesServer(t *testing.T, replies map[string]string) *httptest.Server {
+	t.Helper()
+	knownSHA := map[string]bool{
+		"ed899a2f4b50b4370feeea94676502b42383c746": true,
+		"6104942438c14ec7bd21c6cd5bd995272b3faff6": true,
+	}
+	ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.Header().Set("Content-Type", "application/json")
+		p := r.URL.Path
+		reply := ""
+		switch {
+		case strings.HasSuffix(p, "/repository/commits/"+oddCommit+"/merge_requests"):
+			reply = "mrs_for_commit"
+		case strings.HasSuffix(p, "/merge_requests/101/approvals"):
+			reply = "approvals"
+		case strings.HasSuffix(p, "/merge_requests/101/reviewers"):
+			reply = "reviewers"
+		case strings.HasSuffix(p, "/merge_requests/101/commits"):
+			reply = "mr_commits"
+		case strings.HasSuffix(p, "/signature"):
+			sha := strings.TrimSuffix(p[strings.LastIndex(p, "/commits/")+len("/commits/"):], "/signature")
+			if knownSHA[sha] {
+				reply = "signature"
+			}
+		}
+		if reply == "" {
+			w.WriteHeader(http.StatusNotFound)
+			_, _ = fmt.Fprint(w, `{"message":"404 Not Found"}`)
+			return
+		}
+		_, _ = fmt.Fprint(w, replies[reply])
+	}))
+	t.Cleanup(ts.Close)
+	return ts
+}
+
+func oddEvidence(t *testing.T, replies map[string]string) ([]*types.PREvidence, error) {
+	ts := oddRepliesServer(t, replies)
+	return newPaginationConfig(ts.URL).PREvidenceForCommitV2(oddCommit)
+}
+
+func TestOddReplies_BaselineGivesFullEvidence(t *testing.T) {
+	evidence, err := oddEvidence(t, oddBaseline())
+	require.NoError(t, err)
+	require.Len(t, evidence, 1)
+	e := evidence[0]
+	require.Equal(t, "https://gitlab.example.com/acme/app/-/merge_requests/101", e.URL)
+	require.Equal(t, "9f8e7d6c5b4a39281706f5e4d3c2b1a098765432", e.MergeCommit)
+	require.Equal(t, "merged", e.State)
+	require.Equal(t, "Jane Doe (@janedoe)", e.Author)
+	require.NotZero(t, e.CreatedAt)
+	require.NotZero(t, e.MergedAt)
+	require.Equal(t, "Add audit log", e.Title)
+	require.Equal(t, "audit-log", e.HeadRef)
+	require.Equal(t, "main", e.BaseRef)
+	require.Equal(t, []any{types.PRApprovals{Username: "John Doe1 (@user1)"}}, e.Approvers)
+	require.Equal(t, []types.PRApprovals{
+		{Username: "John Doe1 (@user1)", State: types.ReviewApproved},
+		{Username: "John Doe2 (@user2)", State: types.ReviewCommented},
+	}, e.Reviews)
+	require.Len(t, e.Commits, 2)
+	for _, c := range e.Commits {
+		require.NotEmpty(t, c.SHA)
+		require.NotZero(t, c.Timestamp)
+		require.NotNil(t, c.Verified)
+		require.True(t, *c.Verified)
+	}
+}
+
+// oddRuleExceptions are the replies whose outcome is the opposite of the
+// rule's: an error for a null field, or no error for a null list item.
+var oddRuleExceptions = map[string]string{
+	"mrs_for_commit [0].iid":       "error: the iid names the MR in every later request; null asks for MR 0, which 404s",
+	"mrs_for_commit [0].labels[0]": "ok: labels are not read, and the client reads a null string as empty",
+	"mr_commits [0].parent_ids[0]": "ok: parent ids are not read, and the client reads a null string as empty",
+	"mr_commits [1].parent_ids[0]": "ok: parent ids are not read, and the client reads a null string as empty",
+}
+
+var listItemPath = regexp.MustCompile(`([A-Za-z_]*\[\d+\])$`)
+
+// TestOddReplies feeds every entry point that commands use each reply with one
+// value made null, or one list emptied. None may panic. A null field or an
+// emptied list gives no error; a null list item gives an error naming it.
+func TestOddReplies(t *testing.T) {
+	entryPoints := map[string]func(*GitlabConfig) func(string) ([]*types.PREvidence, error){
+		"V2":     func(c *GitlabConfig) func(string) ([]*types.PREvidence, error) { return c.PREvidenceForCommitV2 },
+		"Hybrid": func(c *GitlabConfig) func(string) ([]*types.PREvidence, error) { return c.PREvidenceForCommitHybrid },
+	}
+	for entry, fetchFor := range entryPoints {
+		for _, name := range oddReplyNames {
+			variants, err := testHelpers.NullVariants([]byte(oddBaseline()[name]))
+			require.NoError(t, err)
+			for _, v := range variants {
+				key := name + " " + v.Path
+				t.Run(entry+"/"+key, func(t *testing.T) {
+					replies := oddBaseline()
+					replies[name] = string(v.Body)
+					ts := oddRepliesServer(t, replies)
+					var fetchErr error
+					outcome := testHelpers.Outcome(func() error {
+						_, fetchErr = fetchFor(newPaginationConfig(ts.URL))(oddCommit)
+						return fetchErr
+					})
+					require.NotContains(t, outcome, "panic")
+
+					item := listItemPath.FindString(v.Path)
+					want := "ok"
+					if item != "" {
+						want = "error"
+					}
+					if reason, ok := oddRuleExceptions[key]; ok {
+						want = map[string]string{"ok": "error", "error": "ok"}[want]
+						t.Log(reason)
+					}
+					require.Equal(t, want, outcome)
+					if item != "" && want == "error" {
+						require.ErrorContains(t, fetchErr, item)
+					}
+				})
+			}
+		}
+	}
+}

From d7381b0af79fc6ec2671268bb18919ea1f15975e Mon Sep 17 00:00:00 2001
From: Alex Kantor <alex@kosli.com>
Date: Wed, 21 Oct 2026 18:15:07 +0100
Subject: [PATCH 03/10] docs(gitlab): correct the signature comment after the
 null-reply change

A null signature reply now reads as unsigned, like a 404, so the comment
saying only a 404 does was wrong. Drops a comment that restated its
function name. Part of #1283.
---
 internal/gitlab/gitlab.go | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/internal/gitlab/gitlab.go b/internal/gitlab/gitlab.go
index d3bcd1602..cd9208492 100644
--- a/internal/gitlab/gitlab.go
+++ b/internal/gitlab/gitlab.go
@@ -171,7 +171,6 @@ func (c *GitlabConfig) GetMergeRequestApprovers(mrIID, version int64) ([]any, er
 	return gitlabApprovers(approvedBy, version), nil
 }
 
-// approvedBy returns who approved an MR.
 func (c *GitlabConfig) approvedBy(client *gitlab.Client, mrIID int64) ([]*gitlab.MergeRequestApproverUser, error) {
 	approvals, _, err := client.MergeRequestApprovals.GetConfiguration(c.ProjectID(), mrIID)
 	if err != nil || approvals == nil {
@@ -346,8 +345,8 @@ func gitlabCommitVerification(status string) (*bool, *string) {
 
 // resolveGitlabSignature maps the result of a GetGPGSignature call to the
 // neutral verified/signature_state fields. GitLab returns 404 for unsigned
-// commits, which is treated as unsigned (nil fields) rather than a fatal error
-// (server#5892); other errors propagate so incomplete signature data is never
+// commits; that and a null reply are unsigned (nil fields), not a fatal error
+// (server#5892). Other errors propagate so incomplete signature data is never
 // silently recorded.
 func resolveGitlabSignature(sig *gitlab.GPGSignature, resp *gitlab.Response, err error) (*bool, *string, error) {
 	if err != nil {

From 95569e73b71bc286d67a995f9de8b7c08f8639c2 Mon Sep 17 00:00:00 2001
From: Alex Kantor <alex@kosli.com>
Date: Thu, 22 Oct 2026 17:17:00 +0100
Subject: [PATCH 04/10] fix(deps): build with Go 1.26.10 and pin the builder
 image by digest

Snyk fails every branch on CVE-2026-111943, a high-severity flaw in Go's
crypto/x509 fixed in 1.26.10; go.mod declared 1.26.7.

The Dockerfile built from golang:1.26-alpine3.23, so each build could
pull a different toolchain. It now names golang:1.26.10-alpine3.23 by
digest. Dependabot can't update a FROM built from ARGs, so the line is
literal, and a docker ecosystem entry (golang only) keeps the digest
current.
---
 .github/dependabot.yml | 18 ++++++++++++++++++
 Dockerfile             |  6 ++----
 go.mod                 |  2 +-
 3 files changed, 21 insertions(+), 5 deletions(-)

diff --git a/.github/dependabot.yml b/.github/dependabot.yml
index d9578d29a..8e9a77e83 100644
--- a/.github/dependabot.yml
+++ b/.github/dependabot.yml
@@ -46,6 +46,24 @@ updates:
     labels:
       - "dependencies"
       - "github-actions"
+  # The Go builder image in Dockerfile, pinned by digest
+  - package-ecosystem: "docker"
+    directory: "/"
+    schedule:
+      interval: "weekly"
+      day: "tuesday"
+      time: "11:00"
+    commit-message:
+      prefix: 'chore(deps)'
+    allow:
+      - dependency-name: "golang"
+    # Minor bumps change the Go the image builds with; go.mod moves first.
+    ignore:
+      - dependency-name: "golang"
+        update-types: ["version-update:semver-major", "version-update:semver-minor"]
+    labels:
+      - "dependencies"
+      - "docker"
   # Docker Compose images
   - package-ecosystem: "docker-compose"
     directory: "/"
diff --git a/Dockerfile b/Dockerfile
index 83fa1cb44..4ddecbc0f 100644
--- a/Dockerfile
+++ b/Dockerfile
@@ -1,15 +1,13 @@
 # syntax=docker/dockerfile:1
 
-ARG GO_VERSION="1.26"
-ARG ALPINE_VERSION="3.23"
-
 # The version the binary reports; empty builds dev+<sha>. Passed in rather than
 # read from git tags, so the image cannot depend on the checkout's tags (#1133).
 ARG VERSION=""
 
 
 ### Go Builder ###
-FROM golang:${GO_VERSION}-alpine${ALPINE_VERSION} AS builder
+# Pinned by digest so a rebuild gets the same toolchain; Dependabot moves it.
+FROM golang:1.26.10-alpine3.23@sha256:107301490755aa2ea1b0b43788b6e62cff2faae45a538028b193becc06aaffba AS builder
 
 RUN apk add --update --no-cache git bash make ca-certificates
 
diff --git a/go.mod b/go.mod
index a79fd4797..a9ebbb66c 100644
--- a/go.mod
+++ b/go.mod
@@ -1,6 +1,6 @@
 module github.com/kosli-dev/cli
 
-go 1.26.7
+go 1.26.10
 
 require (
 	cloud.google.com/go/asset v1.29.0

From 9b4460c12de7d0c8218636590abd39d509f5d951 Mon Sep 17 00:00:00 2001
From: Alex Kantor <alex@kosli.com>
Date: Thu, 22 Oct 2026 17:24:12 +0100
Subject: [PATCH 05/10] fix(gitlab): error on approvals, reviews and commits we
 can't identify

An approved_by entry with no user was recorded as an approval by nobody,
which a policy counting approvers would accept: that fails open. A
reviewer with no user can't be matched to its approval either, and a
commit with no id was looked up at commits//signature and recorded as
unsigned with no SHA. Each is now an error naming the list entry, the
same rule as a null entry, checked in one place (noUnreadableItems).
A user without a username counts as no user, since the username is what
attributes and matches the entry.

Part of #1283

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
---
 internal/gitlab/gitlab.go           | 47 ++++++++++++++++++++---------
 internal/gitlab/odd_replies_test.go | 27 ++++++++++++-----
 2 files changed, 51 insertions(+), 23 deletions(-)

diff --git a/internal/gitlab/gitlab.go b/internal/gitlab/gitlab.go
index cd9208492..7e24f9885 100644
--- a/internal/gitlab/gitlab.go
+++ b/internal/gitlab/gitlab.go
@@ -154,7 +154,7 @@ func (c *GitlabConfig) MergeRequestsForCommit(commit string) ([]*gitlab.BasicMer
 	if err != nil {
 		return mrs, fmt.Errorf("failed to list merge requests for commit %s: %w", commit, err)
 	}
-	return mrs, noNullItems("merge requests", mrs)
+	return mrs, noUnreadableItems("merge requests", mrs, nil)
 }
 
 // GetMergeRequestApprovers returns a list of users (name and username) who approved an MR
@@ -176,7 +176,8 @@ func (c *GitlabConfig) approvedBy(client *gitlab.Client, mrIID int64) ([]*gitlab
 	if err != nil || approvals == nil {
 		return nil, err
 	}
-	return approvals.ApprovedBy, noNullItems("approved_by", approvals.ApprovedBy)
+	return approvals.ApprovedBy, noUnreadableItems("approved_by", approvals.ApprovedBy,
+		func(a *gitlab.MergeRequestApproverUser) string { return missingUser(a.User) })
 }
 
 // GetMergeRequestReviews returns an MR's approvers and its reviews: each
@@ -200,7 +201,8 @@ func (c *GitlabConfig) GetMergeRequestReviews(mrIID int64) ([]any, []types.PRApp
 		}
 		return nil, nil, err
 	}
-	if err := noNullItems("reviewers", reviewers); err != nil {
+	if err := noUnreadableItems("reviewers", reviewers,
+		func(r *gitlab.MergeRequestReviewer) string { return missingUser(r.User) }); err != nil {
 		return nil, nil, err
 	}
 	return approvers, gitlabReviews(approvedBy, reviewers), nil
@@ -213,11 +215,13 @@ func gitlabUserName(u *gitlab.BasicUser) string {
 	return fmt.Sprintf("%s (@%s)", u.Name, u.Username)
 }
 
-func gitlabUsername(u *gitlab.BasicUser) string {
-	if u == nil {
-		return ""
+// missingUser says what is missing when an approval or review names no user:
+// without a username it can't be attributed or matched across the two lists.
+func missingUser(u *gitlab.BasicUser) string {
+	if u == nil || u.Username == "" {
+		return "a user"
 	}
-	return u.Username
+	return ""
 }
 
 // unixOrZero is t in Unix seconds, or 0 when GitLab sent no time.
@@ -228,13 +232,20 @@ func unixOrZero(t *time.Time) int64 {
 	return t.Unix()
 }
 
-// noNullItems errors on a null entry in a list GitLab sent: it is missing
-// data, not an empty entry.
-func noNullItems[T any](list string, items []*T) error {
+// noUnreadableItems errors on an entry in a list GitLab sent that is null, or
+// that missing (if given) says lacks a field: it is missing data, not an
+// empty entry.
+func noUnreadableItems[T any](list string, items []*T, missing func(*T) string) error {
 	for i, item := range items {
 		if item == nil {
 			return fmt.Errorf("GitLab sent %s[%d] as null", list, i)
 		}
+		if missing == nil {
+			continue
+		}
+		if field := missing(item); field != "" {
+			return fmt.Errorf("GitLab sent %s[%d] without %s", list, i, field)
+		}
 	}
 	return nil
 }
@@ -260,13 +271,13 @@ func gitlabApprovers(approvedBy []*gitlab.MergeRequestApproverUser, version int6
 func gitlabReviews(approvedBy []*gitlab.MergeRequestApproverUser, reviewers []*gitlab.MergeRequestReviewer) []types.PRApprovals {
 	reviewerStates := map[string]string{}
 	for _, r := range reviewers {
-		reviewerStates[gitlabUsername(r.User)] = r.State
+		reviewerStates[r.User.Username] = r.State
 	}
 	reviews := []types.PRApprovals{}
 	approved := map[string]bool{}
 	for _, a := range approvedBy {
-		approved[gitlabUsername(a.User)] = true
-		if reviewerStates[gitlabUsername(a.User)] != "requested_changes" {
+		approved[a.User.Username] = true
+		if reviewerStates[a.User.Username] != "requested_changes" {
 			reviews = append(reviews, types.PRApprovals{Username: gitlabUserName(a.User), State: types.ReviewApproved})
 		}
 	}
@@ -275,7 +286,7 @@ func gitlabReviews(approvedBy []*gitlab.MergeRequestApproverUser, reviewers []*g
 		switch {
 		case r.State == "requested_changes":
 			reviews = append(reviews, types.PRApprovals{Username: name, State: types.ReviewChangesRequested})
-		case r.State == "reviewed" && !approved[gitlabUsername(r.User)]:
+		case r.State == "reviewed" && !approved[r.User.Username]:
 			reviews = append(reviews, types.PRApprovals{Username: name, State: types.ReviewCommented})
 		}
 	}
@@ -293,7 +304,13 @@ func (c *GitlabConfig) GetMergeRequestCommits(mr *gitlab.BasicMergeRequest) ([]t
 	if err != nil {
 		return commits, err
 	}
-	if err := noNullItems("commits", glCommits); err != nil {
+	err = noUnreadableItems("commits", glCommits, func(c *gitlab.Commit) string {
+		if c.ID == "" {
+			return "an id"
+		}
+		return ""
+	})
+	if err != nil {
 		return commits, err
 	}
 	// One signature lookup per commit: pagination multiplies this, but GitLab
diff --git a/internal/gitlab/odd_replies_test.go b/internal/gitlab/odd_replies_test.go
index a55b3238f..bb27044d8 100644
--- a/internal/gitlab/odd_replies_test.go
+++ b/internal/gitlab/odd_replies_test.go
@@ -182,12 +182,21 @@ func TestOddReplies_BaselineGivesFullEvidence(t *testing.T) {
 }
 
 // oddRuleExceptions are the replies whose outcome is the opposite of the
-// rule's: an error for a null field, or no error for a null list item.
-var oddRuleExceptions = map[string]string{
-	"mrs_for_commit [0].iid":       "error: the iid names the MR in every later request; null asks for MR 0, which 404s",
-	"mrs_for_commit [0].labels[0]": "ok: labels are not read, and the client reads a null string as empty",
-	"mr_commits [0].parent_ids[0]": "ok: parent ids are not read, and the client reads a null string as empty",
-	"mr_commits [1].parent_ids[0]": "ok: parent ids are not read, and the client reads a null string as empty",
+// rule's: an error for a null field, or no error for a null list item. An
+// error for a null field must name the list item it makes unreadable.
+var oddRuleExceptions = map[string]struct{ why, names string }{
+	"mrs_for_commit [0].iid":                 {why: "error: the iid names the MR in every later request; null asks for MR 0, which 404s"},
+	"mrs_for_commit [0].labels[0]":           {why: "ok: labels are not read, and the client reads a null string as empty"},
+	"mr_commits [0].parent_ids[0]":           {why: "ok: parent ids are not read, and the client reads a null string as empty"},
+	"mr_commits [1].parent_ids[0]":           {why: "ok: parent ids are not read, and the client reads a null string as empty"},
+	"approvals approved_by[0].user":          {why: "error: an approval by nobody would count as approval evidence", names: "approved_by[0]"},
+	"approvals approved_by[0].user.username": {why: "error: an approval by nobody would count as approval evidence", names: "approved_by[0]"},
+	"reviewers [0].user":                     {why: "error: a review by nobody can't be matched to its approval", names: "reviewers[0]"},
+	"reviewers [0].user.username":            {why: "error: a review by nobody can't be matched to its approval", names: "reviewers[0]"},
+	"reviewers [1].user":                     {why: "error: a review by nobody can't be matched to its approval", names: "reviewers[1]"},
+	"reviewers [1].user.username":            {why: "error: a review by nobody can't be matched to its approval", names: "reviewers[1]"},
+	"mr_commits [0].id":                      {why: "error: a commit without a SHA can't be looked up or attested", names: "commits[0]"},
+	"mr_commits [1].id":                      {why: "error: a commit without a SHA can't be looked up or attested", names: "commits[1]"},
 }
 
 var listItemPath = regexp.MustCompile(`([A-Za-z_]*\[\d+\])$`)
@@ -196,6 +205,7 @@ var listItemPath = regexp.MustCompile(`([A-Za-z_]*\[\d+\])$`)
 // value made null, or one list emptied. None may panic. A null field or an
 // emptied list gives no error; a null list item gives an error naming it.
 func TestOddReplies(t *testing.T) {
+	// `kosli assert pullrequest gitlab` calls V2 and `attest` calls Hybrid.
 	entryPoints := map[string]func(*GitlabConfig) func(string) ([]*types.PREvidence, error){
 		"V2":     func(c *GitlabConfig) func(string) ([]*types.PREvidence, error) { return c.PREvidenceForCommitV2 },
 		"Hybrid": func(c *GitlabConfig) func(string) ([]*types.PREvidence, error) { return c.PREvidenceForCommitHybrid },
@@ -222,9 +232,10 @@ func TestOddReplies(t *testing.T) {
 					if item != "" {
 						want = "error"
 					}
-					if reason, ok := oddRuleExceptions[key]; ok {
+					if ex, ok := oddRuleExceptions[key]; ok {
 						want = map[string]string{"ok": "error", "error": "ok"}[want]
-						t.Log(reason)
+						item = ex.names
+						t.Log(ex.why)
 					}
 					require.Equal(t, want, outcome)
 					if item != "" && want == "error" {

From bba2e6006e8175c1b12e9d72962a478e2b2dc4f8 Mon Sep 17 00:00:00 2001
From: Alex Kantor <alex@kosli.com>
Date: Thu, 22 Oct 2026 17:28:32 +0100
Subject: [PATCH 06/10] test: fail when the Dockerfile builds with a different
 Go than go.mod

Dependabot bumps the pinned golang image on its own, and none of the
gomod updates touch go.mod's go directive. Snyk reads go.mod, so an
image-only bump would leave the next Go stdlib CVE flagged even though
the image is fixed. The test makes that bump red until go.mod moves too.
---
 .github/dependabot.yml             |  2 +-
 internal/version/toolchain_test.go | 26 ++++++++++++++++++++++++++
 2 files changed, 27 insertions(+), 1 deletion(-)
 create mode 100644 internal/version/toolchain_test.go

diff --git a/.github/dependabot.yml b/.github/dependabot.yml
index 8e9a77e83..8c2a8e9c9 100644
--- a/.github/dependabot.yml
+++ b/.github/dependabot.yml
@@ -57,7 +57,7 @@ updates:
       prefix: 'chore(deps)'
     allow:
       - dependency-name: "golang"
-    # Minor bumps change the Go the image builds with; go.mod moves first.
+    # Only patch bumps; TestDockerfileBuildsWithTheGoInGoMod fails until go.mod moves too.
     ignore:
       - dependency-name: "golang"
         update-types: ["version-update:semver-major", "version-update:semver-minor"]
diff --git a/internal/version/toolchain_test.go b/internal/version/toolchain_test.go
new file mode 100644
index 000000000..6f4dffd44
--- /dev/null
+++ b/internal/version/toolchain_test.go
@@ -0,0 +1,26 @@
+package version
+
+import (
+	"os"
+	"regexp"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+// Dependabot bumps the Dockerfile's golang image on its own, but Snyk reads
+// the Go version from go.mod, so a bump that skips go.mod hides a stdlib CVE.
+func TestDockerfileBuildsWithTheGoInGoMod(t *testing.T) {
+	goMod, err := os.ReadFile("../../go.mod")
+	require.NoError(t, err)
+	dockerfile, err := os.ReadFile("../../Dockerfile")
+	require.NoError(t, err)
+
+	want := regexp.MustCompile(`(?m)^go (\S+)$`).FindSubmatch(goMod)
+	require.NotNil(t, want, "no go directive in go.mod")
+	got := regexp.MustCompile(`(?m)^FROM golang:([0-9.]+)-`).FindSubmatch(dockerfile)
+	require.NotNil(t, got, "no golang builder image in Dockerfile")
+
+	require.Equal(t, string(want[1]), string(got[1]),
+		"Dockerfile builds with a different Go than go.mod declares; bump both together")
+}

From 1002181adf1e1857342bd3c47024fc5bf7ddfa22 Mon Sep 17 00:00:00 2001
From: Alex Kantor <alex@kosli.com>
Date: Thu, 22 Oct 2026 17:30:18 +0100
Subject: [PATCH 07/10] fix(gitlab): leave reviews by deleted accounts out of
 approvers, not error

GitLab sends a review whose account was deleted with no user. Erroring on
it would fail attestation for every MR ever approved by someone who has
since left, with no workaround. Such a review is now kept in Reviews with
no name and its state, but never counted as an approver, so approval
counting still fails closed. Nameless entries are never matched to each
other, so a nameless change request can't hide a nameless approval. A
commit with no id still errors.

Part of #1283

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
---
 internal/gitlab/gitlab.go           | 66 +++++++++++++-------------
 internal/gitlab/odd_replies_test.go | 72 ++++++++++++++++++++++++-----
 2 files changed, 93 insertions(+), 45 deletions(-)

diff --git a/internal/gitlab/gitlab.go b/internal/gitlab/gitlab.go
index 7e24f9885..a3feb0130 100644
--- a/internal/gitlab/gitlab.go
+++ b/internal/gitlab/gitlab.go
@@ -154,7 +154,7 @@ func (c *GitlabConfig) MergeRequestsForCommit(commit string) ([]*gitlab.BasicMer
 	if err != nil {
 		return mrs, fmt.Errorf("failed to list merge requests for commit %s: %w", commit, err)
 	}
-	return mrs, noUnreadableItems("merge requests", mrs, nil)
+	return mrs, noNullItems("merge requests", mrs)
 }
 
 // GetMergeRequestApprovers returns a list of users (name and username) who approved an MR
@@ -176,8 +176,7 @@ func (c *GitlabConfig) approvedBy(client *gitlab.Client, mrIID int64) ([]*gitlab
 	if err != nil || approvals == nil {
 		return nil, err
 	}
-	return approvals.ApprovedBy, noUnreadableItems("approved_by", approvals.ApprovedBy,
-		func(a *gitlab.MergeRequestApproverUser) string { return missingUser(a.User) })
+	return approvals.ApprovedBy, noNullItems("approved_by", approvals.ApprovedBy)
 }
 
 // GetMergeRequestReviews returns an MR's approvers and its reviews: each
@@ -201,27 +200,28 @@ func (c *GitlabConfig) GetMergeRequestReviews(mrIID int64) ([]any, []types.PRApp
 		}
 		return nil, nil, err
 	}
-	if err := noUnreadableItems("reviewers", reviewers,
-		func(r *gitlab.MergeRequestReviewer) string { return missingUser(r.User) }); err != nil {
+	if err := noNullItems("reviewers", reviewers); err != nil {
 		return nil, nil, err
 	}
 	return approvers, gitlabReviews(approvedBy, reviewers), nil
 }
 
+// gitlabUserName is empty when GitLab names no account (a deleted user): such
+// a review can't count as an approval or be matched to another review.
 func gitlabUserName(u *gitlab.BasicUser) string {
-	if u == nil {
+	if u == nil || u.Username == "" {
 		return ""
 	}
 	return fmt.Sprintf("%s (@%s)", u.Name, u.Username)
 }
 
-// missingUser says what is missing when an approval or review names no user:
-// without a username it can't be attributed or matched across the two lists.
-func missingUser(u *gitlab.BasicUser) string {
-	if u == nil || u.Username == "" {
-		return "a user"
+// gitlabUsername matches people across approvals and reviews; empty when
+// GitLab names no account.
+func gitlabUsername(u *gitlab.BasicUser) string {
+	if u == nil {
+		return ""
 	}
-	return ""
+	return u.Username
 }
 
 // unixOrZero is t in Unix seconds, or 0 when GitLab sent no time.
@@ -232,20 +232,13 @@ func unixOrZero(t *time.Time) int64 {
 	return t.Unix()
 }
 
-// noUnreadableItems errors on an entry in a list GitLab sent that is null, or
-// that missing (if given) says lacks a field: it is missing data, not an
-// empty entry.
-func noUnreadableItems[T any](list string, items []*T, missing func(*T) string) error {
+// noNullItems errors on a null entry in a list GitLab sent: it is missing
+// data, not an empty entry.
+func noNullItems[T any](list string, items []*T) error {
 	for i, item := range items {
 		if item == nil {
 			return fmt.Errorf("GitLab sent %s[%d] as null", list, i)
 		}
-		if missing == nil {
-			continue
-		}
-		if field := missing(item); field != "" {
-			return fmt.Errorf("GitLab sent %s[%d] without %s", list, i, field)
-		}
 	}
 	return nil
 }
@@ -254,6 +247,9 @@ func gitlabApprovers(approvedBy []*gitlab.MergeRequestApproverUser, version int6
 	var approvers []any
 	for _, approver := range approvedBy {
 		approverName := gitlabUserName(approver.User)
+		if approverName == "" {
+			continue
+		}
 		if version == 1 {
 			approvers = append(approvers, approverName)
 		} else {
@@ -271,13 +267,18 @@ func gitlabApprovers(approvedBy []*gitlab.MergeRequestApproverUser, version int6
 func gitlabReviews(approvedBy []*gitlab.MergeRequestApproverUser, reviewers []*gitlab.MergeRequestReviewer) []types.PRApprovals {
 	reviewerStates := map[string]string{}
 	for _, r := range reviewers {
-		reviewerStates[r.User.Username] = r.State
+		if username := gitlabUsername(r.User); username != "" {
+			reviewerStates[username] = r.State
+		}
 	}
 	reviews := []types.PRApprovals{}
 	approved := map[string]bool{}
 	for _, a := range approvedBy {
-		approved[a.User.Username] = true
-		if reviewerStates[a.User.Username] != "requested_changes" {
+		username := gitlabUsername(a.User)
+		if username != "" {
+			approved[username] = true
+		}
+		if reviewerStates[username] != "requested_changes" {
 			reviews = append(reviews, types.PRApprovals{Username: gitlabUserName(a.User), State: types.ReviewApproved})
 		}
 	}
@@ -286,7 +287,7 @@ func gitlabReviews(approvedBy []*gitlab.MergeRequestApproverUser, reviewers []*g
 		switch {
 		case r.State == "requested_changes":
 			reviews = append(reviews, types.PRApprovals{Username: name, State: types.ReviewChangesRequested})
-		case r.State == "reviewed" && !approved[r.User.Username]:
+		case r.State == "reviewed" && !approved[gitlabUsername(r.User)]:
 			reviews = append(reviews, types.PRApprovals{Username: name, State: types.ReviewCommented})
 		}
 	}
@@ -304,15 +305,14 @@ func (c *GitlabConfig) GetMergeRequestCommits(mr *gitlab.BasicMergeRequest) ([]t
 	if err != nil {
 		return commits, err
 	}
-	err = noUnreadableItems("commits", glCommits, func(c *gitlab.Commit) string {
-		if c.ID == "" {
-			return "an id"
-		}
-		return ""
-	})
-	if err != nil {
+	if err := noNullItems("commits", glCommits); err != nil {
 		return commits, err
 	}
+	for i, commit := range glCommits {
+		if commit.ID == "" {
+			return commits, fmt.Errorf("GitLab sent commits[%d] without an id", i)
+		}
+	}
 	// One signature lookup per commit: pagination multiplies this, but GitLab
 	// has no batch signature endpoint.
 	for _, commit := range glCommits {
diff --git a/internal/gitlab/odd_replies_test.go b/internal/gitlab/odd_replies_test.go
index bb27044d8..cb588b5a3 100644
--- a/internal/gitlab/odd_replies_test.go
+++ b/internal/gitlab/odd_replies_test.go
@@ -185,18 +185,12 @@ func TestOddReplies_BaselineGivesFullEvidence(t *testing.T) {
 // rule's: an error for a null field, or no error for a null list item. An
 // error for a null field must name the list item it makes unreadable.
 var oddRuleExceptions = map[string]struct{ why, names string }{
-	"mrs_for_commit [0].iid":                 {why: "error: the iid names the MR in every later request; null asks for MR 0, which 404s"},
-	"mrs_for_commit [0].labels[0]":           {why: "ok: labels are not read, and the client reads a null string as empty"},
-	"mr_commits [0].parent_ids[0]":           {why: "ok: parent ids are not read, and the client reads a null string as empty"},
-	"mr_commits [1].parent_ids[0]":           {why: "ok: parent ids are not read, and the client reads a null string as empty"},
-	"approvals approved_by[0].user":          {why: "error: an approval by nobody would count as approval evidence", names: "approved_by[0]"},
-	"approvals approved_by[0].user.username": {why: "error: an approval by nobody would count as approval evidence", names: "approved_by[0]"},
-	"reviewers [0].user":                     {why: "error: a review by nobody can't be matched to its approval", names: "reviewers[0]"},
-	"reviewers [0].user.username":            {why: "error: a review by nobody can't be matched to its approval", names: "reviewers[0]"},
-	"reviewers [1].user":                     {why: "error: a review by nobody can't be matched to its approval", names: "reviewers[1]"},
-	"reviewers [1].user.username":            {why: "error: a review by nobody can't be matched to its approval", names: "reviewers[1]"},
-	"mr_commits [0].id":                      {why: "error: a commit without a SHA can't be looked up or attested", names: "commits[0]"},
-	"mr_commits [1].id":                      {why: "error: a commit without a SHA can't be looked up or attested", names: "commits[1]"},
+	"mrs_for_commit [0].iid":       {why: "error: the iid names the MR in every later request; null asks for MR 0, which 404s"},
+	"mrs_for_commit [0].labels[0]": {why: "ok: labels are not read, and the client reads a null string as empty"},
+	"mr_commits [0].parent_ids[0]": {why: "ok: parent ids are not read, and the client reads a null string as empty"},
+	"mr_commits [1].parent_ids[0]": {why: "ok: parent ids are not read, and the client reads a null string as empty"},
+	"mr_commits [0].id":            {why: "error: a commit without a SHA can't be looked up or attested", names: "commits[0]"},
+	"mr_commits [1].id":            {why: "error: a commit without a SHA can't be looked up or attested", names: "commits[1]"},
 }
 
 var listItemPath = regexp.MustCompile(`([A-Za-z_]*\[\d+\])$`)
@@ -246,3 +240,57 @@ func TestOddReplies(t *testing.T) {
 		}
 	}
 }
+
+// A review whose account GitLab no longer names (a deleted user) is kept in
+// Reviews with no name, but never counts as an approver.
+func TestOddReplies_ReviewByNoOne(t *testing.T) {
+	approver := types.PRApprovals{Username: "John Doe1 (@user1)"}
+	approved := types.PRApprovals{Username: "John Doe1 (@user1)", State: types.ReviewApproved}
+	commented := types.PRApprovals{Username: "John Doe2 (@user2)", State: types.ReviewCommented}
+	tests := []struct {
+		name          string
+		nulls         map[string]string
+		wantApprovers []any
+		wantReviews   []types.PRApprovals
+	}{
+		{"approval with no user", map[string]string{"approvals": "approved_by[0].user"},
+			nil, []types.PRApprovals{{State: types.ReviewApproved}, commented}},
+		{"approval with no username", map[string]string{"approvals": "approved_by[0].user.username"},
+			nil, []types.PRApprovals{{State: types.ReviewApproved}, commented}},
+		{"review with no user", map[string]string{"reviewers": "[1].user"},
+			[]any{approver}, []types.PRApprovals{approved, {State: types.ReviewCommented}}},
+		{"review with no username", map[string]string{"reviewers": "[1].user.username"},
+			[]any{approver}, []types.PRApprovals{approved, {State: types.ReviewCommented}}},
+		{"approval and review by no one are not matched", map[string]string{"approvals": "approved_by[0].user", "reviewers": "[1].user"},
+			nil, []types.PRApprovals{{State: types.ReviewApproved}, {State: types.ReviewCommented}}},
+		{"change request by no one does not hide an approval by no one", map[string]string{"approvals": "approved_by[0].user", "reviewers": "[1].user", "requested_changes": ""},
+			nil, []types.PRApprovals{{State: types.ReviewApproved}, {State: types.ReviewChangesRequested}}},
+	}
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			replies := oddBaseline()
+			if _, ok := tc.nulls["requested_changes"]; ok {
+				replies["reviewers"] = strings.Replace(replies["reviewers"], `"state": "reviewed"`, `"state": "requested_changes"`, 1)
+			}
+			for reply, path := range tc.nulls {
+				if reply == "requested_changes" {
+					continue
+				}
+				variants, err := testHelpers.NullVariants([]byte(replies[reply]))
+				require.NoError(t, err)
+				found := false
+				for _, v := range variants {
+					if v.Path == path {
+						replies[reply], found = string(v.Body), true
+					}
+				}
+				require.True(t, found, path)
+			}
+			evidence, err := oddEvidence(t, replies)
+			require.NoError(t, err)
+			require.Len(t, evidence, 1)
+			require.Equal(t, tc.wantApprovers, evidence[0].Approvers)
+			require.Equal(t, tc.wantReviews, evidence[0].Reviews)
+		})
+	}
+}

From ed4f9b3582fb4bf08708bdbd126ddc4b396807a9 Mon Sep 17 00:00:00 2001
From: Alex Kantor <alex@kosli.com>
Date: Sun, 25 Oct 2026 14:00:51 +0000
Subject: [PATCH 08/10] fix(gitlab): keep only change requests from reviews
 with no account

Policies read Reviews as well as Approvers, so an approval or comment
kept there with no name still counted as evidence from nobody: the same
fail-open as an anonymous approver. A review with no account is now kept
in Reviews only when it requests changes, as dropping that would read as
none given. Same rule as Bitbucket's on main. Never an error.

The empty-username guards on matching are gone: with this rule nothing
an unnamed match could hide is ever kept.

Part of #1283

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
---
 internal/gitlab/gitlab.go           | 29 ++++++++++++++++-------------
 internal/gitlab/odd_replies_test.go | 26 ++++++++++++++------------
 2 files changed, 30 insertions(+), 25 deletions(-)

diff --git a/internal/gitlab/gitlab.go b/internal/gitlab/gitlab.go
index a3feb0130..988151976 100644
--- a/internal/gitlab/gitlab.go
+++ b/internal/gitlab/gitlab.go
@@ -207,7 +207,7 @@ func (c *GitlabConfig) GetMergeRequestReviews(mrIID int64) ([]any, []types.PRApp
 }
 
 // gitlabUserName is empty when GitLab names no account (a deleted user): such
-// a review can't count as an approval or be matched to another review.
+// a review is never an approver, and is in Reviews only if it requests changes.
 func gitlabUserName(u *gitlab.BasicUser) string {
 	if u == nil || u.Username == "" {
 		return ""
@@ -267,28 +267,31 @@ func gitlabApprovers(approvedBy []*gitlab.MergeRequestApproverUser, version int6
 func gitlabReviews(approvedBy []*gitlab.MergeRequestApproverUser, reviewers []*gitlab.MergeRequestReviewer) []types.PRApprovals {
 	reviewerStates := map[string]string{}
 	for _, r := range reviewers {
-		if username := gitlabUsername(r.User); username != "" {
-			reviewerStates[username] = r.State
-		}
+		reviewerStates[gitlabUsername(r.User)] = r.State
 	}
 	reviews := []types.PRApprovals{}
+	add := func(u *gitlab.BasicUser, state string) {
+		name := gitlabUserName(u)
+		// A change request is kept without a name: dropping it would read as
+		// none given.
+		if name == "" && state != types.ReviewChangesRequested {
+			return
+		}
+		reviews = append(reviews, types.PRApprovals{Username: name, State: state})
+	}
 	approved := map[string]bool{}
 	for _, a := range approvedBy {
-		username := gitlabUsername(a.User)
-		if username != "" {
-			approved[username] = true
-		}
-		if reviewerStates[username] != "requested_changes" {
-			reviews = append(reviews, types.PRApprovals{Username: gitlabUserName(a.User), State: types.ReviewApproved})
+		approved[gitlabUsername(a.User)] = true
+		if reviewerStates[gitlabUsername(a.User)] != "requested_changes" {
+			add(a.User, types.ReviewApproved)
 		}
 	}
 	for _, r := range reviewers {
-		name := gitlabUserName(r.User)
 		switch {
 		case r.State == "requested_changes":
-			reviews = append(reviews, types.PRApprovals{Username: name, State: types.ReviewChangesRequested})
+			add(r.User, types.ReviewChangesRequested)
 		case r.State == "reviewed" && !approved[gitlabUsername(r.User)]:
-			reviews = append(reviews, types.PRApprovals{Username: name, State: types.ReviewCommented})
+			add(r.User, types.ReviewCommented)
 		}
 	}
 	return reviews
diff --git a/internal/gitlab/odd_replies_test.go b/internal/gitlab/odd_replies_test.go
index cb588b5a3..32fcf6b9b 100644
--- a/internal/gitlab/odd_replies_test.go
+++ b/internal/gitlab/odd_replies_test.go
@@ -241,8 +241,8 @@ func TestOddReplies(t *testing.T) {
 	}
 }
 
-// A review whose account GitLab no longer names (a deleted user) is kept in
-// Reviews with no name, but never counts as an approver.
+// A review whose account GitLab no longer names (a deleted user) is never an
+// approver, and is kept in Reviews, without a name, only if it requests changes.
 func TestOddReplies_ReviewByNoOne(t *testing.T) {
 	approver := types.PRApprovals{Username: "John Doe1 (@user1)"}
 	approved := types.PRApprovals{Username: "John Doe1 (@user1)", State: types.ReviewApproved}
@@ -254,17 +254,19 @@ func TestOddReplies_ReviewByNoOne(t *testing.T) {
 		wantReviews   []types.PRApprovals
 	}{
 		{"approval with no user", map[string]string{"approvals": "approved_by[0].user"},
-			nil, []types.PRApprovals{{State: types.ReviewApproved}, commented}},
+			nil, []types.PRApprovals{commented}},
 		{"approval with no username", map[string]string{"approvals": "approved_by[0].user.username"},
-			nil, []types.PRApprovals{{State: types.ReviewApproved}, commented}},
-		{"review with no user", map[string]string{"reviewers": "[1].user"},
-			[]any{approver}, []types.PRApprovals{approved, {State: types.ReviewCommented}}},
-		{"review with no username", map[string]string{"reviewers": "[1].user.username"},
-			[]any{approver}, []types.PRApprovals{approved, {State: types.ReviewCommented}}},
-		{"approval and review by no one are not matched", map[string]string{"approvals": "approved_by[0].user", "reviewers": "[1].user"},
-			nil, []types.PRApprovals{{State: types.ReviewApproved}, {State: types.ReviewCommented}}},
-		{"change request by no one does not hide an approval by no one", map[string]string{"approvals": "approved_by[0].user", "reviewers": "[1].user", "requested_changes": ""},
-			nil, []types.PRApprovals{{State: types.ReviewApproved}, {State: types.ReviewChangesRequested}}},
+			nil, []types.PRApprovals{commented}},
+		{"comment with no user", map[string]string{"reviewers": "[1].user"},
+			[]any{approver}, []types.PRApprovals{approved}},
+		{"comment with no username", map[string]string{"reviewers": "[1].user.username"},
+			[]any{approver}, []types.PRApprovals{approved}},
+		{"approval and comment by no one", map[string]string{"approvals": "approved_by[0].user", "reviewers": "[1].user"},
+			nil, []types.PRApprovals{}},
+		{"change request by no one is kept", map[string]string{"reviewers": "[1].user", "requested_changes": ""},
+			[]any{approver}, []types.PRApprovals{approved, {State: types.ReviewChangesRequested}}},
+		{"change request by no one is kept beside an approval by no one", map[string]string{"approvals": "approved_by[0].user", "reviewers": "[1].user", "requested_changes": ""},
+			nil, []types.PRApprovals{{State: types.ReviewChangesRequested}}},
 	}
 	for _, tc := range tests {
 		t.Run(tc.name, func(t *testing.T) {

From c6175154cf7c7ea3218b28adedf8a990a982a023 Mon Sep 17 00:00:00 2001
From: Alex Kantor <alex@kosli.com>
Date: Sun, 25 Oct 2026 14:07:04 +0000
Subject: [PATCH 09/10] docs(gitlab): say why nameless users can share a map
 key

The reviewer and approval maps now key every user with no account as "".
Say that this is safe only because add drops every nameless entry except a
change request, so a later change to add doesn't hide an approval silently.

Part of #1283

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
---
 internal/gitlab/gitlab.go | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/internal/gitlab/gitlab.go b/internal/gitlab/gitlab.go
index 988151976..00316b41b 100644
--- a/internal/gitlab/gitlab.go
+++ b/internal/gitlab/gitlab.go
@@ -265,6 +265,8 @@ func gitlabApprovers(approvedBy []*gitlab.MergeRequestApproverUser, version int6
 // reviewers can approve too. A change request outweighs an approval by the
 // same person, as there are no review times to order them by.
 func gitlabReviews(approvedBy []*gitlab.MergeRequestApproverUser, reviewers []*gitlab.MergeRequestReviewer) []types.PRApprovals {
+	// Users with no account all share the key "". That's safe only because
+	// add keeps no nameless entry other than a change request.
 	reviewerStates := map[string]string{}
 	for _, r := range reviewers {
 		reviewerStates[gitlabUsername(r.User)] = r.State

From ad1b4f6593272ffe8d97cf48b159e35e8f375d48 Mon Sep 17 00:00:00 2001
From: Alex Kantor <alex@kosli.com>
Date: Sun, 25 Oct 2026 17:17:47 +0000
Subject: [PATCH 10/10] fix(gitlab): name the commit or merge request in
 null-entry errors

An error for a null entry named only the list and index, such as
merge requests[0]. The error just before it names the commit, so a user
reading both couldn't tell what the second one was about. Each error now
says what the list belongs to: the commit for its merge requests, or the
merge request for its approvals, reviewers and commits.

Part of #1283

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
---
 internal/gitlab/gitlab.go           | 19 ++++++++++---------
 internal/gitlab/odd_replies_test.go |  5 +++++
 2 files changed, 15 insertions(+), 9 deletions(-)

diff --git a/internal/gitlab/gitlab.go b/internal/gitlab/gitlab.go
index 00316b41b..48af5c52f 100644
--- a/internal/gitlab/gitlab.go
+++ b/internal/gitlab/gitlab.go
@@ -154,7 +154,7 @@ func (c *GitlabConfig) MergeRequestsForCommit(commit string) ([]*gitlab.BasicMer
 	if err != nil {
 		return mrs, fmt.Errorf("failed to list merge requests for commit %s: %w", commit, err)
 	}
-	return mrs, noNullItems("merge requests", mrs)
+	return mrs, noNullItems("merge requests", "commit "+commit, mrs)
 }
 
 // GetMergeRequestApprovers returns a list of users (name and username) who approved an MR
@@ -176,7 +176,7 @@ func (c *GitlabConfig) approvedBy(client *gitlab.Client, mrIID int64) ([]*gitlab
 	if err != nil || approvals == nil {
 		return nil, err
 	}
-	return approvals.ApprovedBy, noNullItems("approved_by", approvals.ApprovedBy)
+	return approvals.ApprovedBy, noNullItems("approved_by", fmt.Sprintf("merge request !%d", mrIID), approvals.ApprovedBy)
 }
 
 // GetMergeRequestReviews returns an MR's approvers and its reviews: each
@@ -200,7 +200,7 @@ func (c *GitlabConfig) GetMergeRequestReviews(mrIID int64) ([]any, []types.PRApp
 		}
 		return nil, nil, err
 	}
-	if err := noNullItems("reviewers", reviewers); err != nil {
+	if err := noNullItems("reviewers", fmt.Sprintf("merge request !%d", mrIID), reviewers); err != nil {
 		return nil, nil, err
 	}
 	return approvers, gitlabReviews(approvedBy, reviewers), nil
@@ -232,12 +232,12 @@ func unixOrZero(t *time.Time) int64 {
 	return t.Unix()
 }
 
-// noNullItems errors on a null entry in a list GitLab sent: it is missing
-// data, not an empty entry.
-func noNullItems[T any](list string, items []*T) error {
+// noNullItems errors on a null entry in a list GitLab sent for owner: it is
+// missing data, not an empty entry.
+func noNullItems[T any](list, owner string, items []*T) error {
 	for i, item := range items {
 		if item == nil {
-			return fmt.Errorf("GitLab sent %s[%d] as null", list, i)
+			return fmt.Errorf("GitLab sent %s[%d] for %s as null", list, i, owner)
 		}
 	}
 	return nil
@@ -310,12 +310,13 @@ func (c *GitlabConfig) GetMergeRequestCommits(mr *gitlab.BasicMergeRequest) ([]t
 	if err != nil {
 		return commits, err
 	}
-	if err := noNullItems("commits", glCommits); err != nil {
+	owner := fmt.Sprintf("merge request !%d", mr.IID)
+	if err := noNullItems("commits", owner, glCommits); err != nil {
 		return commits, err
 	}
 	for i, commit := range glCommits {
 		if commit.ID == "" {
-			return commits, fmt.Errorf("GitLab sent commits[%d] without an id", i)
+			return commits, fmt.Errorf("GitLab sent commits[%d] for %s without an id", i, owner)
 		}
 	}
 	// One signature lookup per commit: pagination multiplies this, but GitLab
diff --git a/internal/gitlab/odd_replies_test.go b/internal/gitlab/odd_replies_test.go
index 32fcf6b9b..286f4c34b 100644
--- a/internal/gitlab/odd_replies_test.go
+++ b/internal/gitlab/odd_replies_test.go
@@ -234,6 +234,11 @@ func TestOddReplies(t *testing.T) {
 					require.Equal(t, want, outcome)
 					if item != "" && want == "error" {
 						require.ErrorContains(t, fetchErr, item)
+						owner := "for merge request !101 "
+						if name == "mrs_for_commit" {
+							owner = "for commit " + oddCommit + " "
+						}
+						require.ErrorContains(t, fetchErr, owner, "the error must say what the list belongs to")
 					}
 				})
 			}