Skip to content

feat(gitprovider): GitLab merge request review support#513

Open
piekstra wants to merge 1 commit into
mainfrom
piekstra/510-gitlab-mr-support
Open

feat(gitprovider): GitLab merge request review support#513
piekstra wants to merge 1 commit into
mainfrom
piekstra/510-gitlab-mr-support

Conversation

@piekstra

Copy link
Copy Markdown
Contributor

Summary

Adds GitLab merge request review support (#510): cr review, cr respond, cr agents, cr me, and cr config resolve-profile now work against gitlab.com and self-managed GitLab instances with PAT auth, alongside the existing GitHub support.

Config. git.provider is a new optional discriminator (github | gitlab). Empty means GitHub, so existing configs are untouched and never rewritten. GitLab profiles are restricted to auth_mode: pat and reuse the existing git_token credstore key — cr set-credential works unchanged.

URL parsing. prref.ParsePullURL recognizes both GitHub /pull/N URLs and GitLab /-/merge_requests/N URLs (nested namespaces and the legacy form without /-/ included), and every command cross-checks the URL's provider family against the resolved profile with a usage error on mismatch. Nested namespaces flow through as slashed PRRef.Owner; ledger keys already percent-encode segments so no key-format change was needed.

GitLab adapter (internal/gitprovider/gitlab, hand-rolled net/http over REST v4, mirroring the github package's structure, error taxonomy, secret redaction, and pagination host-pinning):

  • Inline threads map to diff discussions; diff-note positions are built from the MR's diff_refs, with old/new line counterparts computed from the per-file diff so comments on unchanged context lines post correctly, and rename-aware old_path/new_path. File-level positions are native (position_type: file).
  • SubmitReview maps approve to the approvals API (SHA-guarded; 409 → stale-SHA), request_changes revokes any active self-approval, and posts the review summary as an MR note. The approval state change happens before the summary note so interrupted posts retry safely (the note carries the idempotency marker).
  • ListReviews maps current approvals; ResolveThread uses PUT .../discussions/:id?resolved=true; ReviewAuthority uses the members API (Developer+ = eligible).
  • GetDiff uses raw_diffs (GitLab ≥ 15.7) with a per-file reconstruction fallback for older instances; stale head SHAs are rejected before any write.

Provider-neutral seams that had GitHub baked in:

  • ProviderCaps.ReviewSummaryAsComment — outbox reconciles submit-review markers against issue comments for hosts without first-class review objects.
  • ProviderCaps.HeadRefNamespace — workbench fetches fork MR heads from refs/merge-requests/N/head instead of refs/pull/N/head; workbench remote URLs accept nested namespaces.
  • gitexec pairs the token with a provider-specific HTTPS basic-auth username (oauth2 for GitLab, x-access-token unchanged for GitHub).
  • New internal/gitproviders dispatch package is the single construction seam used by internal/app, agentscmd, and mecmd.

Deliberately out of scope (follow-ups): cr init wizard support for GitLab (profiles are added via config.yml, documented in the README), GitLab CI job-token / group-token auth modes, and benchmark suites against GitLab MRs.

Note: TestDryRunResumesPinnedReviewRunByPinnedSHAs and TestDryRunWithPinnedReviewSHAsUsesCompareDiffAndPinnedFileRefs fail locally on this machine on main too — trustCurrentTempFixtures points TMPDIR at a directory that is never created and this git build won't write temp files there. Unrelated to this change; happy to file separately.

Type of Change

  • New feature
  • Documentation update

Checklist

  • I have read the contributing guide
  • My change follows the project structure and conventions
  • make tidy passes
  • make lint passes
  • make test passes (two pre-existing environment-dependent pipeline test failures reproduce on main, see note above)
  • make build passes

Related Issues

Closes #510

Adds a gitlab provider adapter over the GitLab REST v4 API and wires it
through config, URL parsing, provider dispatch, posting semantics, and
the git transport so cr review/respond/agents/me/config work against
gitlab.com and self-managed GitLab with PAT auth.

- config: git.provider discriminator (github default, gitlab), gitlab
  restricted to pat auth
- prref: ParsePullURL recognizes GitHub pull URLs and GitLab merge
  request URLs (nested namespaces, legacy form), commands cross-check
  the URL provider family against the profile
- gitlab adapter: discussions as inline threads, position-based diff
  notes with old/new line mapping, approvals API for approve/revoke,
  discussion resolution, raw_diffs with per-file reconstruction
  fallback, members API for review authority
- caps: ReviewSummaryAsComment reconciles submit-review markers against
  notes; HeadRefNamespace fetches refs/merge-requests/N/head for fork
  MRs; workbench accepts nested namespaces in remote URLs
- gitexec: provider-specific HTTPS basic auth username (oauth2)
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.

Feature request: GitLab merge request review support

1 participant