fix(types): decode the user ID values that MarshalJSON writes

What does this MR do?

AssigneeIDValue, ReviewerIDValue and ApproverIDsValue implement UnmarshalJSON as json.Unmarshal(bytes, a.value) (ApproverIDsValue, AssigneeIDValue, ReviewerIDValue). a.value is an any that is nil on a zero value and holds a non-pointer (int64, []int64 or UserIDValue) once constructed, so json.Unmarshal always returns json: Unmarshal(nil) or json: Unmarshal(non-pointer ...) and stores nothing.

As a result, JSON that carries assignee_id, not[assignee_id], reviewer_id, approver_ids or approved_by_ids cannot be decoded into ListMergeRequestsOptions, ListProjectMergeRequestsOptions, ListGroupMergeRequestsOptions, ListIssuesOptions, ListGroupIssuesOptions or ListProjectIssuesOptions, even when the library wrote that JSON itself with json.Marshal.

The case this breaks is any program that holds list options as JSON and reads them back: a tool server that receives a list's filters as JSON arguments and decodes them into these options or into fields of these types, a saved filter, a configuration file, or a job queued for later. Each of them gets json: Unmarshal(nil) at the first user filter and has to parse None, Any and the IDs itself. That is what gitlab-mcp-server, an MCP server built on this library whose tools take their arguments as JSON, does today, and maintaining it is how I found the defect.

This MR decodes into the forms MarshalJSON writes: a number becomes an int64 (a list of numbers becomes a []int64 for ApproverIDsValue), and a string becomes a UserIDValue such as None or Any. JSON null leaves the value unchanged, as ISOTime.UnmarshalJSON does, and any other input (a boolean, a fractional number, a list where a single ID is expected and the reverse) returns an error. The two decode attempts live in one small generic helper shared by the three types.

Related: !3127 (merged) added NotAssigneeID *AssigneeIDValue to the three merge request list options, so the new field had the same problem; it decodes with this change, and the round-trip test covers it. The only open MR that edits types.go is !3124, in the build state constants further down the file, and its change still applies cleanly on top of this one.

Is this a breaking change?

No. No exported identifier or signature changes. The observable change is that UnmarshalJSON on these three types now succeeds and fills the value where it used to fail on every input, null included. One detail to be aware of: an ID built from an int (AssigneeID(5)) decodes back as an int64, which encodes to the same query parameter. Any string is accepted as a UserIDValue, matching what the constructors already accept. The inputs that still fail now fail with an error that names the input and the type, such as json: cannot unmarshal true into Go value of type gitlab.AssigneeIDValue.

How was this tested?

  • Table-driven tests in types_test.go for each type (a number or a list, None, Any, replacing an already set value, null, and inputs that must be rejected), and a round trip through json.Marshal and json.Unmarshal of ListMergeRequestsOptions, with every field of these types in both forms, and of ListIssuesOptions. On main at 85bf7810 with only the new tests added, all 16 decoding cases and the round trip fail with json: Unmarshal(nil) or json: Unmarshal(non-pointer ...), on Go 1.26.10 and on Go 1.27.3; the 8 rejection cases pass there too, since everything errors. With the fix, all of them pass on both.
  • The steps of make reviewable with the versions .tool-versions pins (gofumpt 0.8.0, buf 1.55.1, golangci-lint 2.15.0): go mod tidy, make generate and make fmt change no file, golangci-lint reports 0 issues, buf format --diff --exit-code and buf lint pass, go vet is clean, and go test -race work passes on Go 1.26.10 and on Go 1.27.3, the two versions tests:unit runs. No signature changed, so make generate has nothing to regenerate.
  • To check that the tests pin the behaviour, I removed each null check, removed the string fallback, made ApproverIDsValue decode a single int64, and swallowed the decode error; each of those six changes made at least one of the new tests fail.
  • There is no integration test, because no request to a GitLab instance reaches this code. These types appear only in request options, which client-go encodes into the query string through EncodeValues and never decodes, and no response or webhook struct uses them. The defect is in decoding JSON that the caller holds, so the unit tests drive the same path a caller does: json.Unmarshal into the list options.
Test output and a reproduction, without and with the fix

The new tests on main at 85bf7810 (Go 1.26.10, top-level results sorted and times left out; Go 1.27.3 prints the same):

--- FAIL: TestApproverIDsValue_UnmarshalJSON
--- FAIL: TestAssigneeIDValue_UnmarshalJSON
--- FAIL: TestReviewerIDValue_UnmarshalJSON
--- FAIL: TestUserIDValueOptions_JSONRoundTrip
FAIL	gitlab.com/gitlab-org/api/client-go/v3

The 16 decoding subtests and the round trip fail with json: Unmarshal(nil) (11 times), json: Unmarshal(non-pointer gitlab.UserIDValue) (3 times), json: Unmarshal(non-pointer int64) (twice) and json: Unmarshal(non-pointer []int64) (once).

With the fix, on both versions:

--- PASS: TestApproverIDsValue_UnmarshalJSON
--- PASS: TestAssigneeIDValue_UnmarshalJSON
--- PASS: TestReviewerIDValue_UnmarshalJSON
--- PASS: TestUserIDValueOptions_JSONRoundTrip
PASS
ok  	gitlab.com/gitlab-org/api/client-go/v3

A program that encodes list options and decodes them again:

package main

import (
	"encoding/json"
	"fmt"

	gitlab "gitlab.com/gitlab-org/api/client-go/v3"
)

func main() {
	in := gitlab.ListMergeRequestsOptions{
		AssigneeID: gitlab.AssigneeID(gitlab.UserIDNone),
		ReviewerID: gitlab.ReviewerID(int64(5)),
	}
	data, err := json.Marshal(in)
	fmt.Printf("marshal:   %s err=%v\n", data, err)

	var out gitlab.ListMergeRequestsOptions
	err = json.Unmarshal(data, &out)
	again, _ := json.Marshal(out)
	fmt.Printf("unmarshal: %s err=%v\n", again, err)
}

On main:

marshal:   {"assignee_id":"None","reviewer_id":5} err=<nil>
unmarshal: {"assignee_id":null} err=json: Unmarshal(nil)

With the fix:

marshal:   {"assignee_id":"None","reviewer_id":5} err=<nil>
unmarshal: {"assignee_id":"None","reviewer_id":5} err=<nil>

gitlab-mcp-server keeps a record of what it finds and contributes in the projects it depends on, and in sibling projects, in upstream-bugs.md.

Merge request reports

Loading
Loading