fix(commits): decode the lists GitLab sends in extended_trailers
What does this MR do?
GitLab sends a commit's extended_trailers as a map of each trailer to the list of its values, and Commit declared the field as map[string]string, so any response holding a commit that carries a trailer fails to decode as a whole. ListCommits with Trailers set is where this shows today: one commit with a Cc: or Signed-off-by: line in a page makes the whole page an error. This adds ExtendedTrailerValues map[string][]string, which takes the extended_trailers key, and keeps ExtendedTrailers as a deprecated field, so no caller stops compiling. It comes from maintaining gitlab-mcp-server, an MCP server built on this library, which hit it on its commit list.
The change
lib/api/entities/commit.rbdocumentsextended_trailersas a hash of each trailer to the list of its values, andGitlab::Git::Commit#parse_commit_trailersbuilds it that way:(hash[trailer.key] ||= []) << encode!(trailer.value).commits.go:CommitgainsExtendedTrailerValues map[string][]stringwith theextended_trailerstag.ExtendedTrailersis marked deprecated and no longer bound to a JSON key, since two fields cannot share one. It is no longer filled, and nothing is lost by that: since GitLab added the key it has sent lists, so a response that would have filled the old field failed to decode instead. I considered anUnmarshalJSONthat fills it with the last value of each trailer, but Go would promote that method to every type that embedsCommitand decode such a type as aCommitalone, which is a worse surprise than an empty deprecated field.- Retyping
ExtendedTrailersin place would be the simpler change, but it breaks every caller that reads it, so I left that for a major version; I can add it to the 4.0 list in #2301 if you prefer that route.
gitlab-mcp-server keeps the record of this defect, and of everything else it finds and contributes in the projects it depends on and in sibling projects, in upstream-bugs.md (entry 69). The server works around it by reading commits from the raw response wherever it publishes trailers.
Is this a breaking change?
No for what GitLab sends: every response that decoded before decodes the same way, and responses that failed now decode. ExtendedTrailers still compiles but stays empty, which is what a successfully decoded response gave it before. Two things change for JSON this library did not get from GitLab: marshalling a Commit writes extended_trailers from ExtendedTrailerValues, in the shape GitLab sends, since ExtendedTrailers no longer has a JSON key; and JSON written by an earlier version from a Commit whose ExtendedTrailers held values, a shape GitLab never sends, no longer decodes. No service interface changes, so no mock is regenerated.
How was this tested?
Tests and lint
TestCommitsService_ListCommits_ExtendedTrailersanswers a page shaped like GitLab's own response: a commit with no trailers and one with twoCc:trailers. It asserts the page decodes,ExtendedTrailerValuesholds both values,Trailersholds the last one, and the deprecated field stays nil.TestCommit_DecodeWithoutExtendedTrailers: a commit whose answer has noextended_trailerskey at all decodes with both maps nil. Unless trailers are asked for, GitLab sends{}, which the first commit of the list test covers.- Without the change, the same page fails with
json: cannot unmarshal array into Go struct field .1.extended_trailers.Cc of type string.
On the branch, with Go 1.27.1:
go test -race ./... ./config/...in workspace mode, andgo test ./...withGOWORK=off, pass.golangci-lint run ./...with golangci-lint 2.13.2, the version.tool-versionspins, reports 0 issues, andgofumpt -lreports nothing; the struct block is realigned by gofumpt because the new field comments split it.
Related to #2300