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.rb documents extended_trailers as a hash of each trailer to the list of its values, and Gitlab::Git::Commit#parse_commit_trailers builds it that way: (hash[trailer.key] ||= []) << encode!(trailer.value).
  • commits.go: Commit gains ExtendedTrailerValues map[string][]string with the extended_trailers tag. ExtendedTrailers is 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 an UnmarshalJSON that fills it with the last value of each trailer, but Go would promote that method to every type that embeds Commit and decode such a type as a Commit alone, which is a worse surprise than an empty deprecated field.
  • Retyping ExtendedTrailers in 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_ExtendedTrailers answers a page shaped like GitLab's own response: a commit with no trailers and one with two Cc: trailers. It asserts the page decodes, ExtendedTrailerValues holds both values, Trailers holds the last one, and the deprecated field stays nil.
  • TestCommit_DecodeWithoutExtendedTrailers: a commit whose answer has no extended_trailers key 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, and go test ./... with GOWORK=off, pass.
  • golangci-lint run ./... with golangci-lint 2.13.2, the version .tool-versions pins, reports 0 issues, and gofumpt -l reports nothing; the struct block is realigned by gofumpt because the new field comments split it.

Related to #2300

Merge request reports

Loading
Loading