Skip to content

Add GitLab provider support - #6

Merged
simion merged 8 commits into
simion:mainfrom
Nystya:gitlab-support
Oct 2, 2026
Merged

simion merged 8 commits into
simion:mainfrom
Nystya:gitlab-support

Conversation

@Nystya

@Nystya Nystya commented Oct 2, 2026 •

Copy link
Copy Markdown

What this does

reviewd could only review pull requests on GitHub and BitBucket. This PR adds GitLab merge requests.

GitLab works like the other two providers. Point a repo at it with provider: gitlab, and reviewd polls for open MRs, runs the review, and posts the result: a summary comment, plus inline comments on the changed lines. If auto-approve is enabled, it also approves the MR.

gitlab:
  token: ${GITLAB_TOKEN}                 # personal/project/group token with the `api` scope

repos:
  - name: my-repo
    repo_slug: group/subgroup/my-repo    # full project path, nested groups are fine
    path: ~/repos/my-repo
    provider: gitlab

reviewd init also detects GitLab remotes, so the setup wizard writes this config for you.

Expand E4 under "Manual testing" to see what a review looks like on a GitLab MR.

Where to look

Most of the new code is in providers/gitlab.py. It is written in the same style as the GitHub provider.

  • Inline comments are posted as GitLab "diff discussions". GitLab needs the MR's base, start and head commits for these. reviewd saves them when it lists or fetches an MR, and fetches them again if the MR has new commits.
  • Line positions: for a comment on an unchanged line, GitLab needs both the old and the new line number, and the file's old name if it was renamed. reviewd reads the MR's diff (from GitLab's /diffs API, once per head commit) to work these out. Added lines only get the new line number. Findings always refer to lines in the new file, so they never target a deleted line. If a line isn't in the diff, or GitLab collapsed the file, reviewd doesn't guess: it posts a file-level comment that starts with Line N: and leaves out the suggestion block.
  • Re-reviews delete the previous reviewd comments, as on the other providers.
  • Approval: if GitLab refuses an approval (for example, you can't approve your own MR), reviewd logs a warning and carries on. GitLab sometimes answers 401 for "you can't approve this", so on a 401 reviewd checks the token against /user. If the token is also rejected there, or a 403 says its scope is too narrow, reviewd raises the error instead of hiding it.

Smaller changes:

  • Forks and deleted branches: if the source branch can't be fetched, the reviewer now also tries refs/merge-requests/<iid>/head, the ref GitLab keeps for every MR. reviewer.py gets one line for this.
  • Wizard: detects GitLab remotes, asks for a token, and checks it before saving it.
  • Shared code: the Link-header pagination parser and the 429 retry code were copied into every provider. They now live once in providers/base.py. This touches the GitHub and BitBucket providers but doesn't change how they behave.
  • Docs: a GitLab section in the README, config.example.yaml and CLAUDE.md.

The commits are meant to be read one at a time. The first adds the feature, and the rest are follow-ups from a self-review.

How it was tested

Automated

There are 67 new tests, and the whole suite passes (129 tests). The GitLab API is mocked with respx, so nothing calls the real GitLab.

  • Provider: listing MRs, pagination, draft detection, summary and inline comments, line positions (added and unchanged lines, renamed files, and the file-level fallback for lines outside the diff and collapsed files), deleting comments, approving (accepted, refused, and a rejected or read-only token), retrying on 429, and an MR with no diff info yet.
  • Config: ${ENV_VAR} tokens, a per-repo block overriding the global one, a clear error when no GitLab config exists, and GitHub, GitLab and BitBucket repos in one config.
  • Wizard: remote detection over SSH and HTTPS (nested groups, credentials in the URL), not mistaking other hosts for GitLab, and a full round trip: the YAML the wizard generates loads back into working providers.
  • Full review cycle: the real GitLab provider runs through reviewd's comment posting, with mocked HTTP. Covered: inline comments GitLab rejects are skipped, re-reviews delete old notes, and critical_task is skipped on GitLab.
uv sync --group dev
uv run pytest -q

Manual testing

Run by hand against a throwaway project on gitlab.com. Each check has its screenshot in a collapsed section; click a title to expand it.

Setup and listing

✅ E1 · reviewd init detects the GitLab repo and writes a working config Screenshot 2026-10-02 at 20 49 19
✅ E2 · reviewd ls lists open MRs with the right authors, branches and draft state Screenshot 2026-10-02 at 20 51 47 Screenshot 2026-10-02 at 20 52 15

Reviewing

✅ E3 · --dry-run prints the review and posts nothing Screenshot 2026-10-02 at 21 38 07
✅ E4 · A real review posts the summary note and an inline comment on the right line Screenshot 2026-10-02 at 21 41 45 Screenshot 2026-10-02 at 21 42 34 Screenshot 2026-10-02 at 21 42 40
✅ E5 · The bot marker in comments is invisible in the GitLab UI
✅ E6 · Pushing a new commit triggers a re-review that replaces the old comments Screenshot 2026-10-02 at 21 43 52

Filtering

✅ E7 · Draft MRs are skipped until tagged [review] Screenshot 2026-10-02 at 21 44 56
✅ E13 · Pagination: with a smaller page size, MRs spread over several pages are all listed

Not tested with more than 100 real open MRs.

Screenshot 2026-10-02 at 21 46 11

Branch edge cases

✅ E9 · An MR whose source branch was deleted still gets reviewed
✅ E8 · An MR from a fork gets reviewed
❌ E8b · An MR from a fork whose branch name also exists in this repo (known issue, see below)

The main repo's branch gets reviewed instead of the fork's.

Tokens and approvals

✅ E12 · A read-only token fails with a clear error
✅ E10 · A bot account auto-approves an MR
✅ E11 · Trying to approve your own MR is handled gracefully

Known limitations

  • Fork branch with the same name as a main-repo branch (E8b): if a fork's MR uses a branch name that also exists in the main repo, reviewd reviews the main repo's branch. This also affects GitHub and isn't specific to GitLab. I'd fix it in a separate PR: check that the fetched branch is at the MR's commit, and fall back to the MR ref when it isn't.
  • Inline comments only anchor on lines that are part of the diff. A finding on any other line is posted as a file-level comment that names the line.
  • critical_task stays BitBucket-only for now. GitLab has no direct equivalent.

Not in this PR

There's a pagination bug in the GitHub and BitBucket providers. After the first page, httpx drops the query string from the next-page URL. GitHub repos with more than 100 open PRs could probably loop forever, and BitBucket only ever reads the first page. The GitLab provider avoids this. I can send the fix for the other two as its own PR.

🤖 Generated with Claude Code

Catalin added 6 commits October 2, 2026 19:16
- GitlabProvider for GitLab REST API v4 (gitlab.com and self-hosted via url)
- MR summary notes, inline diff discussions, comment deletion, approval
- Global and per-repo gitlab config blocks with env var support
- Fall back to refs/merge-requests/<iid>/head for forks and deleted branches
- Wizard detects GitLab remotes and validates tokens per instance
- Tests for provider, config, wizard and review cycle
@simion

simion commented Oct 2, 2026

Copy link
Copy Markdown
Owner

Inline diff discussions only set new_line (gitlab.py:576), so comments on context or deleted lines will be rejected or anchored to the wrong line, and this path has no test, every positioning test uses an added line. approve_pr also swallows 401 as a self-approve case (gitlab.py:597), which can mask a real auth failure instead of just the self-approve/approvals-disabled cases. Pagination refactor and the params={} bug claim both check out against the other providers. Fix those two and this looks ready.

@simion

simion commented Oct 2, 2026

Copy link
Copy Markdown
Owner

Hehe, nice to see you again. Fix those two and it's in.

@simion

simion commented Oct 2, 2026

Copy link
Copy Markdown
Owner

Fix for the approve_pr masking, narrow the caught statuses so a real auth failure doesn't get swallowed as a benign self-approve:

def approve_pr(self, repo_slug: str, pr_id: int) -> bool:
    resp = self._request_raw('POST', f'{self._mr_url(repo_slug, pr_id)}/approve')
    if resp.status_code in (403, 405):
        logger.warning('Cannot approve MR !%d (self-approve or approvals disabled): %s', pr_id, resp.text[:200])
        return False
    resp.raise_for_status()
    logger.info('Approved MR !%d', pr_id)
    return True

401 means the token itself is bad. That should raise like every other call here, not look like a self-approve.

@simion

simion commented Oct 2, 2026

Copy link
Copy Markdown
Owner

For the inline positioning bug (gitlab.py:576-578): new_line alone only works when the target line is a pure addition. GitLab's diff-discussion API wants old_line too for context lines, and old_line only for removed lines.

reviewd doesn't parse diff hunks today, so the minimal correct fix is to classify the target line against the unified diff you already generate for the AI, then set the position accordingly:

if is_added_line:
    position['new_line'] = target_line
elif is_removed_line:
    position['old_line'] = target_line
else:  # context line, present on both sides
    position['old_line'] = old_line_for(target_line)
    position['new_line'] = target_line

If a line can't be classified confidently, fall back to position_type: 'file' instead of guessing. A file-level comment that mentions the line beats a silently wrong anchor.

…ures on approve

- Read the MR diff (once per head commit) to map each new-file line to its
  old line. Unchanged lines now send both old_line and new_line, as GitLab
  requires; added lines keep new_line only. Renamed files use the diff's
  old_path. Files GitLab collapsed (empty diff) fall back to new_line only.
- approve_pr no longer treats every 401 as "can't approve". GitLab also
  answers 401 for self-approval, so a 401 is only swallowed when the token
  still works against /user. A 403 insufficient_scope (read-only token) now
  raises too.
@simion

simion commented Oct 2, 2026

Copy link
Copy Markdown
Owner

A intrat Nanoclaw pă fir 😎 am cerut review de pe telegram :))

Lines outside the MR diff, and lines in files GitLab collapsed, no longer get
a guessed old_line/new_line. They're posted as file-level comments that start
with "Line N:", without the suggestion block (it needs a line to apply to).
@Nystya

Nystya commented Oct 2, 2026 •

Copy link
Copy Markdown
Author
  • Added line: new_line only
  • Unchanged line: old_line + new_line
Screenshot 2026-10-02 at 22 28 37
  • Anything outside the diff, or in a file GitLab collapsed: it falls back to a file-level comment, like you suggested. The comment starts with "Line N:" so the reader knows where it was meant to
    go, and the suggestion block is dropped since there's no line to apply it to.
Screenshot 2026-10-02 at 22 37 38

Regarding the approve_pr error:

Before switching to your version, though, I checked what gitlab.com actually returns, and it's a bit annoying:
approving an MR that's already approved also comes back as a 401. Same token, same MR, the first approve gave me a 201 and the second a 401:

Screenshot 2026-10-02 at 22 42 07

So with only 403/405 caught, every re-review of an MR reviewd had already approved would blow up.

What I did instead (06083fb): on a 401, reviewd makes a quick GET /user with the same token. If that's rejected too, the token really is bad, and it raises the original error like every other
call. If the token works, it's the "already approved / can't approve" case, so it logs a warning and moves on. I also made a 403 insufficient_scope raise, so a read-only token doesn't get
mistaken for a self-approve either.

@Nystya
Nystya marked this pull request as ready for review October 2, 2026 19:44
@Nystya

Nystya commented Oct 2, 2026

Copy link
Copy Markdown
Author

Should be ok now. Mersi de raspuns :)

@simion
simion merged commit 182b134 into simion:main Oct 2, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants