Upstream Contribution — GitLab client-go

SkillDev tools

Contribute bug fixes or features to upstream projects (gitlab.com/gitlab-org/api/client-go). Use when: API gap found, client-go bug, missing endpoint wrapper.

Available today. Use it from your connected AI after setup.

Add ahel to your AI once: Claude, ChatGPT, Cursor, Claude Code or Codex. Then ask it to use this.

Then ask your AI: use the Upstream Contribution skill

What this skill tells your AI

The instructions your AI receives, as published by jmrplens/gitlab-mcp-server in .github/skills/upstream-contribution/SKILL.md and read by ahel’s review.

Contribute bug fixes, missing endpoint wrappers, or missing struct fields to the upstream GitLab API client library this project depends on.

Every rule below was read off the upstream project's own configuration, its triage automation or its merged history, and the source is named beside it. Where two readings disagreed, the one that survived an adversarial pass is the one written here.

Before starting

  1. Identify the specific gap or bug in gitlab.com/gitlab-org/api/client-go/v3, with the GitLab API source or documentation that proves it.
  2. Verify it is not already fixed in the latest release.
  3. Search the upstream project's open merge requests for the same struct, field or file, from any author. If one already covers it, do not open a second: read it, judge whether it is complete (does it carry every key the entity exposes, does it change the options struct where the endpoint accepts the parameter, does its test actually decode the field), and if something is missing, say so in a comment on that merge request with the same evidence a new one would have carried. A duplicate costs a maintainer more than it saves.
  4. Record the finding in docs/development/upstream-bugs.md, the permanent register of upstream defects and gaps found from this codebase: follow its entry schema, link the tracker item with a full URL (never a bare !NNN or #NN, since the repository is mirrored between GitHub and GitLab), and keep the entry when the fix lands, marking it merged with the version that carries it.

Upstream project

Opening a merge request

1. Branch in the community fork

Push to the community fork, never to a personal one. docs/CONTRIBUTING.md recommends it, and GitLab's triage bot posts a "did you know about our community forks" nudge on every merge request opened from a personal fork. The pipeline runs either way, so CI is not the reason.

The fork is shared by many contributors, so prefix branch names with the account: jmrp-<topic>.

git push https://oauth2:$GITLAB_COM_TOKEN@gitlab.com/gitlab-community/gitlab-org/api/client-go.git HEAD:refs/heads/jmrp-<topic>

Then open the merge request from project 65275361 with target_project_id=65271576.

A merge request's source project and branch cannot be changed after it is opened — the update endpoint accepts neither parameter and answers with the list of ones it does accept. Moving one means opening a replacement and closing the original with a note naming it.

2. Target main

Target a release-client-N.0 branch only when the change breaks the public API. A new struct field or a corrected json tag does not.

3. Write a conventional-commit title, and prefix every commit

Title: feat(topics): add OrganizationID to the Topic struct. feat for a new field or endpoint, fix for a wrong tag or a bug.

The scope is the API file, and it is expected even though conventional-commit does not require one. fix: correct the json tag parses as conventional and reaches the release notes, so a check for "has a prefix" passes it, and the project's merged history is overwhelmingly file-scoped (fix(group_boards), fix(feature_flags), fix(issues)). Write the scope.

The release-notes generator strips the prefix and omits a non-conventional title entirely, so an unprefixed title merges and then never appears in the changelog. Of 279 changelog entries, none differs from its merge request title.

AddingAPISupport.md step 9 asks for the prefix on every commit, not only the merge request title.

Never use a (no-release) scope, and never put [delay release] in the description: the first marks work that deliberately cuts no version, the second delays the release job.

4. Use the project's description headings

Three ## headings, in this order: "What does this MR do?", "Is this a breaking change?", "How was this tested?". The repository has issue templates and no merge request template, so this is convention read off merged requests rather than something enforced, which is exactly why it gets skipped: seven merge requests opened from here carried ad-hoc headings or none until they were rewritten.

Answer the breaking-change heading honestly rather than with a flat "no". Correcting a json tag is source-compatible and still changes the value a caller decodes, which is worth one sentence.

Say the gap was found while developing this MCP server and link the repository: the backlink is the point of contributing from here.

While issue 2300 is open, always reference it, and do it in the opening line. Every merge request that comes out of the field-by-field review before the umbrella closes names issue 2300 in the first paragraph of "What does this MR do?", and carries the machine-readable form on a line of its own at the end:

Related to https://gitlab.com/gitlab-org/api/client-go/-/issues/2300

Issue 2300 is the umbrella: it publishes the method, the per-struct lists of fields GitLab sends that the library does not model, and the table of what has been sent so far. Related to is deliberate on every merge request that leaves the umbrella open, because it has to survive that merge rather than be closed by it. The one exception is the merge request that finishes it: !3063, the single merge request the maintainers asked for (see "Batch, do not drip"), ends with Closes #2300, since nothing is left for the umbrella to track once it merges. After that, a field found later is an ordinary on-demand contribution, and CONTRIBUTING.md asks for no issue before one.

Putting the reference only at the bottom is what went wrong on 2026-09-09. A code owner asked, on !3053, for exactly the issue that already existed and that every one of those merge requests linked. A Related to line under the fold is a machine-readable trailer, not a thing a reviewer reads.

Batch, do not drip

One merge request per struct is the wrong shape, and the maintainers said so. The request, in their words on !3053: create an issue with the missing fields, keep updating it until there is a good point to send merge requests that may be a bit larger, so they can approve chunks ahead of time and then only check that the merge request matches the chunk.

What they then settled on in issue 2300 went further than chunks. PatrickRice proposed on 2026-09-09 (note 3811110919) that this server send every addition and struct change it needs in a single merge request, and that the project then go back to people submitting fields on demand, with no list kept; timofurrer agreed the next day, recalling that a list they kept years ago did not age well. That single merge request is !3063. So the rule after it merges is the on-demand one: a field this server needs goes upstream when it is needed, in a merge request of its own sized to that need, and no list or table of pending fields is proposed to them again.

What actually stung was the burst, not the count. Asked directly, the same maintainer said there was no reason to close the ones already in front of reviewers, that they would get through them, and that he was reacting to a second large set of pings arriving on top of an overnight one. So the rule to follow is about the rate at which you demand attention, not a ceiling on open merge requests: batch the work, and above all do not fire a round of @gitlab-bot ready commands in one sitting. Offering to withdraw work already under review was the wrong instinct and was declined.

Say plainly how the work is produced when asked, along with what that does and does not guarantee: the entity, line and condition are read out of a booted GitLab rather than recalled, and each is checked against current master, which narrows what a reviewer must distrust without pretending it removes the review. They will notice on their own, and being told is better than being reassured.

Two separate mechanisms read that line, and confusing them is what produced the wrong rule this replaces.

  • apply_labels_from_related_issue.rb copies the first type:: label off the referenced issue, and only onto a merge request that carries none, so a label already asked for by comment is not overwritten. It re-fires whenever the description is edited. The reference must start a line with one of related to, relates to, relate to, contributes to, contribute to, closes, close, see; fixes, resolves and updates are not in that list, and merge requests using them merged with no type:: label ever. Issue 2300 carries type::maintenance, which the machine-learning labeller put on it fifteen minutes after it was opened (see "Keep issue 2300 current"), so every merge request referencing it is labelled type::maintenance at creation, and that label cuts no release. !3063 was: it carried type::maintenance from the moment it opened. The fix is the label command of "Moving an open merge request", which the author may post: type:: is a scoped label, so ~"type::feature" replaces the copied one rather than sitting beside it, and the script never re-fires over a merge request that already carries a type:: label.
  • The contributor platform applies a linked-issue label of its own a few minutes after creation, when the merge request's GraphQL linked_work_items connection is non-empty. A MENTIONED link is enough; it does not have to be CLOSES. That label is what carries the linked-issue point bonus, which is why one umbrella issue referenced from every merge request scores nearly the same as opening a throwaway issue per merge request, and reads as evidence rather than as noise.

CONTRIBUTING.md asks for no issue before a merge request in as many words: "When you are up for writing a MR to solve the issue you encountered, it's not needed to first open a separate issue." So do not open one per merge request. One umbrella carrying the evidence, referenced from each, is the shape that respects that and still earns the link.

Issue 2300 is ours, opened to answer a question their own issue 2269 left open. What the maintainers agreed to in it is the shape above and nothing more: one merge request, then fields on demand, and no list. Quote that when a description needs it, and do not describe the issue's tables or method as anything a maintainer endorsed.

5. Do not set fields the account cannot set

MergeRequests::BaseService#filter_reviewer deletes reviewer_ids silently unless the caller can administer the merge request, at creation and after. The same holds for assignee, labels and milestone: a non-member account has adminMergeRequest: false and createLabel: false. Setting them through the API looks like it worked and changes nothing. Section "Moving an open merge request" below is how those fields are actually reached.

Do not open as a draft; the ready command issues /ready regardless.

Do not request @GitLabDuo yourself. That request is what produces the DCR4003 warning ("you don't have permission to create a pipeline for Code Review Flow"), and it is not the review that counts: on merged requests gitlab-bot requests Duo after the ready command and Duo then posts a real review.

This one cannot be undone. Removing a reviewer means reviewer_ids, which is exactly the field dropped silently for a non-member, so a Duo request made at creation stays on the merge request with its warning comment for the life of it. Seven merge requests opened from here carry one permanently.

Do not write a Changelog: trailer and do not touch CHANGELOG.md: semantic-release writes it.

Code rules

From .gitlab/duo/mr-review-instructions.yaml, which is what the reviewer checks against:

  • int64 never int, and any never interface{} — inside slices and maps too.
  • Pointer structs for requests, non-pointer for responses.
  • PathEscape() on every path parameter interpolated into a route. Query values are not path segments: encode those with url.Values.Encode(), or url.QueryEscape() for a single value, since url.PathEscape leaves a + alone and form-style decoding then reads it as a space.
  • Project and group ids typed any, parsed with parseID().
  • lowerCamelCase locals.
  • "GitLab", never "Gitlab" or "gitlab", in comments, log messages, test names and any other text.
  • Put a new struct field where the API documentation puts it, not at the end of the struct.

From AGENTS.md, and the single most-repeated demand in the project's own documents: every public function, type and method carries a comment beginning with its own name, in the present tense, wrapped under 80 characters, ending in a // GitLab API docs: <url> line.

Tests:

  • Assert the new field specifically.
  • Start with t.Parallel(), use setup(t), testMethod(t, r, ...) and testify assert/require; never reflect.DeepEqual.
  • Inline JSON for a small response, mustWriteHTTPResponse(t, w, "testdata/...") for a large one.
  • GIVEN/WHEN/THEN comments are for complex tests and may be omitted.

Run make reviewable (setup, generate, fmt, lint, test) before opening, and regenerate mocks whenever a signature changes.

Expect danger-review and autolabels to fail: both are allow_failure: true. This matters beyond the noise — autolabels is the job that would have derived the type:: label from the conventional prefix, so the title-to-label route is dead and the label has to be asked for by comment.

Moving an open merge request

In this order.

  1. Wait until the merge request is prepared, before any bot command. prepared_at null means the diff does not exist yet: the labeller then classifies a change it cannot see, and the automation that runs when preparation finishes resets the workflow label, undoing a ready posted before it. See the preparation-lag paragraph below for the label events this produced on !255300. Poll until prepared_at is non-null and changes_count matches the commit.
  2. Fix the title if it has no conventional prefix, or has one with no file scope. Title and description are the two fields the author may still edit.
  3. Fix the description if it is missing the three headings, the backlink or the reference to issue 2300 (Related to, or Closes on the merge request that finishes it), keeping every piece of evidence it already carries. Do it before the label command: editing a description re-fires apply_labels_from_related_issue.rb, and although that script only labels a merge request carrying no type:: label at all, and so cannot overwrite one already asked for, doing the edit first removes the question entirely.
  4. Ask for the label: post @gitlab-bot label ~"type::feature" (or ~"type::bug") as a comment, with the command at the start of its own line. command_mr_label.rb accepts it from the resource author, and its allowed scopes include type. The rate limit is the module default of 60 per hour keyed on the actor. Pick by what should ship: type::feature cuts a minor, type::bug a patch, type::maintenance cuts nothing. A merge request referencing issue 2300 arrives with type::maintenance already copied onto it, so for one that should cut a release the command is not optional; for the rest there is no urgency: an unlabelled merge request still merges and still ships under "Other Changes", and the analyzer reads labels through the API at release time, so a label added later still counts.
  5. Ask for review, naming a code owner: post @gitlab-bot ready @<code owner>. command_mr_request_review.rb reacts to the author's own note and emits /ready, the workflow::ready for review label and /request_review; with arguments it requests exactly those users. This is the only mechanism by which a non-member gets a chosen name onto the reviewer field. Posted bare it picks a random GitLab-wide merge request coach, who is usually not one of the four code owners. On a documentation-only merge request in gitlab-org/gitlab a bare ready can pick nobody at all, so there it names the page's technical writer instead; "Ready it naming the page's writer" below has the mechanism. One ready command per merge request per hour: the limit is 1 for a non-member and the cache key is the actor plus the merge request path, so several merge requests can be readied within the same hour and a misfire costs an hour on that one only.
  6. Then stop. One approval is required and this account cannot approve. On client-go the fork pipeline is already green and needs no maintainer to start it; on gitlab-org/gitlab a merge can still need a maintainer to start a pipeline in the canonical project, as gitlab-org/gitlab!254699 does. Do not re-post ready if nothing happens: it is rate-limited and would only re-request the same person, and gitlab-bot nudges an unattended merge request on its own. The one exception is a ready that requested nobody, so read the reviewer field after every ready rather than the workflow label, which is set either way. If it holds nobody but GitLabDuo, the remedy is a ready that names someone, never the same bare one again, which fails the same way each time: gitlab-org/gitlab!254540 was readied bare three times and had no reviewer until the fourth named one. The field can also lose a reviewer after the ready: on gitlab-org/gitlab!256936, which changes code and a page, the documentation automation took the coach a bare ready had just requested off the field in the same step that added the page's writer.

A reply under your own note can block the merge. A note posted at the top level is an individual note, which is not resolvable. Replying to it through the discussions endpoint (POST /projects/:id/merge_requests/:iid/discussions/:discussion_id/notes) turns it into a resolvable thread that starts unresolved, and an unresolved thread fails the DISCUSSIONS_NOT_RESOLVED merge check. On gitlab-org/gitlab!254699 a follow-up posted that way under our own informational note added a merge blocker to an approved merge request that had none, at the moment it asked a maintainer to set auto-merge, and the blocker held until the thread was resolved an hour later. Before replying under a note of ours, read the discussion's individual_note: if the reply only adds information, post it as a new top-level note, and if it has to sit under the old one, resolve that thread once it is posted, since it is ours and answers nobody's question. gitlab-bot answers inside the ready's own note whenever the ready requests someone, so such a ready becomes a thread too (a bare ready that requested nobody, as the first three on gitlab-org/gitlab!254540, gets no answer and stays an individual note); on gitlab-org/gitlab!254699 the maintainer resolved it when he approved.

The same gap is usually in GitLab's own documentation

A field client-go does not model is often a field GitLab never documented either, and the merge request is not finished until both are sent. This is not a guess: cross-checking the eight fields of the first tranche against doc/api/, two of them, is_receptive and file_extension, appeared nowhere on their page. A code owner asked for exactly this on !3045 rather than opening it himself, so offering it unprompted saves a round trip.

Check before assuming. Read the page from gitlab-org/gitlab (project 278964) and grep for the field. Do not trust the page's title to name the field's home: the service account page is doc/api/service_accounts.md, not user_service_accounts.md, and a wrong path answers 404 rather than "absent".

Where the change goes. Branch on the community fork gitlab-community/gitlab-org/gitlab (project 41372369, default branch master, developer access), and open the merge request against gitlab-org/gitlab master with target_project_id. One file, one commit, remove_source_branch and allow_collaboration both on.

Four rules the two merge requests sent so far were shaped by.

  1. Every example, not the one that prompted it. doc/api/secure_files.md had four example responses and all four predated the field. Fixing one and leaving three is a worse page than before, because now they disagree.
  2. The attribute table counts as documentation. doc/api/cluster_agents.md documents its responses twice, in a table and in an example, and a field missing from the table is missing whichever the reader trusts. That page needed three tables and four examples.
  3. Other entities share the page. The cluster agents page also documents agent tokens and receptive URL configurations, which are different Grape entities and must not gain the field. Identify the right objects by something structural (the agent examples are the ones carrying config_project), never by position.
  4. Put the field where the entity exposes it, and keep the table aligned. Read the order off gitlab-api-live.json. In a table, keep the new cell no wider than the column already is, or every other row has to be re-padded and a three-line change becomes a whole-table diff.

Two traps in that record, both of which have already produced a wrong published claim. Its per-field gate key is conditions, a list, not condition: reading it as condition reports every field as unconditional and never errors, which is how upstream issue 2300 came to claim that all six API::Entities::ServiceAccount fields are unconditional when unconfirmed_email is gated on unconfirmed_email.present?. And the record is a pin taken at one release, so it lags master and will not know a field or route added since. Before writing a conditionality claim into a public description, confirm it against GitLab's current Ruby, and say "sent only when …" rather than nothing when a field is gated: a documented field a reader cannot find in a response is a bug report waiting to be filed.

Validate before pushing: parse every ```json block on the page and assert the field is present in exactly the objects that should have it and absent from the rest. A trailing comma left behind by hand-editing is invisible in review and breaks the example for anyone who copies it.

A new merge request there reports empty for a while. prepared_at is null until a background job fetches the fork ref and builds the diff, and until then the API answers 0 commits and 0 changed files. That is preparation lag on a repository that size, not a failed push: confirm the work from the branch tip on the fork instead. Compare the merge request's sha with the branch tip on the fork; equal means the push landed and only the diff is missing.

Wait for prepared_at before asking the bot for anything. Every automation that reacts to a new merge request reads its diff, and the one that runs when preparation finishes resets the workflow label, so a ready posted before then is undone a minute later. The label events of !255300 are the whole story, read with resource_label_events:

Shortened here. Read the whole file on GitHub.

Signals

GitHub stars
39
Forks
5
Last commit
Sep 2026

ahel review

  • K2info
    exfiltration

Automated review, not a security audit. Ruleset v1+k2.

Advanced
Item type
skill
Key
upstream-contribution-jmrplens
Source
github.com/jmrplens/gitlab-mcp-server