Uh oh!
There was an error while loading. Please reload this page.
feat: add native GitLab MR review integration - #622
Merged
Conversation
Adds `altimate-code gitlab review <mr-url>` CLI command that fetches MR diffs and posts AI review comments back to GitLab merge requests. - Parses GitLab MR URLs (any instance, nested groups, self-hosted) - Fetches MR metadata, diffs, and existing comments via REST API v4 - Runs AI code review using the existing session/prompt infrastructure - Posts review results as MR notes with deduplication marker - Exports discussion API helper for future inline commenting - Auth via GITLAB_PERSONAL_ACCESS_TOKEN or GITLAB_TOKEN env vars - Self-hosted instances via GITLAB_INSTANCE_URL or auto-detected from URL FixesAltimateAI#618 Co-Authored-By: Vijay Yadav <vjyadav194@gmail.com>
This PR doesn't fully meet our contributing guidelines and PR template. What needs to be fixed:
Please edit this PR description to address the above within 2 hours, or it will be automatically closed. If you believe this was flagged incorrectly, please let a maintainer know. |
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughAdded a new GitLab MR review CLI: parses MR URLs, fetches MR metadata/changes/notes via GitLab API, constructs an AI review prompt, streams model output, and optionally posts or updates the review as a GitLab MR note. Includes unit tests for MR URL parsing. Changes
Sequence DiagramsequenceDiagram
participant User as User/CLI
participant Cmd as GitLab Command
participant API as GitLab API
participant AI as AI Model
participant Bus as Event Bus
User->>Cmd: gitlab review <mr-url>
Cmd->>Cmd: parseGitLabMRUrl(url)
Cmd->>API: GET /projects/:id/merge_requests/:iid (metadata)
API-->>Cmd: metadata
Cmd->>API: GET /projects/:id/merge_requests/:iid/changes
API-->>Cmd: diffs
Cmd->>API: GET /projects/:id/merge_requests/:iid/notes
API-->>Cmd: notes
Cmd->>AI: Build prompt (metadata + diffs + notes) and start Session
AI->>Bus: stream parts/events
Bus-->>Cmd: session-part events
Cmd->>User: stream output to terminal
alt post-comment enabled
Cmd->>API: POST/PUT /projects/:id/merge_requests/:iid/notes
API-->>Cmd: note created/updated
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested labels
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/opencode/src/cli/cmd/gitlab.ts`:
- Around line 374-385: The final assistant output is being printed twice because
subscribeSessionEvents(session) renders the completed review when a part
finishes (part.time?.end) and then the code prints reviewText after runReview()
resolves; modify the post-run printing to avoid duplication by removing or
guarding the UI.println(UI.markdown(reviewText)) step: either have runReview
return undefined when subscribeSessionEvents handles final rendering or add a
conditional check before calling UI.println to only print when
subscribeSessionEvents did not already render the final assistant output; apply
the same change for the similar block around the runReview call at the other
location (the block currently at lines shown also containing
UI.println/UI.markdown usage).
- Around line 195-208: The code defines postMRDiscussion(instanceUrl, projectId,
mrIid, token, body, position) which posts a line-level discussion but is
exported and never invoked, so the current flow always creates a single MR note;
fix by wiring this function into the MR feedback path: where MR notes are
created (the function(s) that currently call gitlabApi to post a single note),
detect when a finding includes file/line information, map that finding to a
GitLab DiscussionPosition and call postMRDiscussion instead of the single-note
API; keep the existing single-note behavior for non-positional feedback and
ensure the exported postMRDiscussion is actually imported/used in the module
that posts MR feedback (update the calling code that currently posts via
gitlabApi to delegate to postMRDiscussion when position is present).
- Around line 225-227: The code filters marker notes from the prompt but still
always creates a new marked review comment, causing duplicate posts; modify the
logic that posts the full-review comment to first scan the existing notes array
for a note whose body starts with "<!-- altimate-code-review -->" (use the same
notes variable/filter as in noteLines), and if found either update that existing
note (using its id) instead of creating a new one or skip posting a new comment
entirely; apply the same change for the other duplicate-creating sites
referenced (the blocks around lines mentioned), ensuring all places that
currently always create a fresh marked note instead update/skip when an existing
marker is present.
- Around line 317-320: The handler is calling process.exit(1) directly after
UI.error (using mrUrl and parsed), which bypasses top-level finally/cleanup
(e.g., Telemetry.shutdown); instead remove process.exit(1) and propagate the
failure by throwing an error (e.g., throw new Error("Invalid GitLab MR URL")
including mrUrl) or return a rejected Promise so the top-level runner can handle
exit and run cleanup. Apply the same change at the other occurrences noted (the
blocks around lines with UI.error and process.exit at the other locations).
- Around line 323-328: The code always prefers parsed.instanceUrl from
parseGitLabMRUrl(), making resolveInstanceUrl()/GITLAB_INSTANCE_URL ineffective;
update the logic in the gitlab command so the environment value can override the
parsed MR URL when present (or explicitly treat the env as a fallthrough):
change the instanceUrl assignment to prefer envInstanceUrl when non-empty (e.g.
instanceUrl = envInstanceUrl || parsed.instanceUrl) or add a documented
flag/parameter to force override; adjust any downstream use of instanceUrl
accordingly and keep resolveInstanceUrl() usage consistent with the chosen
precedence.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 76cda296-dd65-4f24-bb35-dd88a4ef8136
📒 Files selected for processing (3)
packages/opencode/src/cli/cmd/gitlab.tspackages/opencode/src/index.tspackages/opencode/test/cli/gitlab-mr-url.test.ts
Comment on lines
+195
to
+208
| async function postMRDiscussion( | ||
| instanceUrl: string, | ||
| projectId: string, | ||
| mrIid: number, | ||
| token: string, | ||
| body: string, | ||
| position: DiscussionPosition, | ||
| ): Promise<unknown> { | ||
| return gitlabApi( | ||
| instanceUrl, | ||
| `/projects/${projectId}/merge_requests/${mrIid}/discussions`, | ||
| token, | ||
| { method: "POST", body: { body, position } }, | ||
| ) |
There was a problem hiding this comment.
The inline-review half of the feature is still not wired up.
This path always posts a single MR note. postMRDiscussion() is exported, but never used, so the line-level feedback called out in issue #618 still cannot reach GitLab discussions.
If you want, I can help sketch the finding-to-position mapping as a follow-up.
Also applies to: 388-396, 501-502
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/opencode/src/cli/cmd/gitlab.ts` around lines 195 - 208, The code
defines postMRDiscussion(instanceUrl, projectId, mrIid, token, body, position)
which posts a line-level discussion but is exported and never invoked, so the
current flow always creates a single MR note; fix by wiring this function into
the MR feedback path: where MR notes are created (the function(s) that currently
call gitlabApi to post a single note), detect when a finding includes file/line
information, map that finding to a GitLab DiscussionPosition and call
postMRDiscussion instead of the single-note API; keep the existing single-note
behavior for non-positional feedback and ensure the exported postMRDiscussion is
actually imported/used in the module that posts MR feedback (update the calling
code that currently posts via gitlabApi to delegate to postMRDiscussion when
position is present).
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
dev-punia-altimate
commented
Apr 3, 2026
Contributor
❌ Tests — Failures DetectedTypeScript — 15 failure(s)
cc @VJ-yadav |
anandgupta42
commented
Apr 3, 2026
Contributor
@VJ-yadav Can you share screenshot of how you tested the feature? |
- Replace process.exit(1) with thrown errors so top-level finally block runs Telemetry.shutdown() on failure (matches github.ts pattern) - Deduplicate review comments: detect existing marker note and update via PUT instead of always creating a new one - Remove duplicate output printing (subscribeSessionEvents already renders the completed assistant message) - Fix GITLAB_INSTANCE_URL env var precedence: env now overrides the URL parsed from the MR link, enabling self-hosted proxy/mirror use - Remove unused postMRDiscussion/DiscussionPosition dead code; add updateMRNote helper used by the dedup logic Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
packages/opencode/src/cli/cmd/gitlab.ts (1)
364-377:⚠️ Potential issue | 🟠 MajorInline discussion posting is still not wired into the MR feedback path.
Lines 365-377 only create/update a general note. The linked objective for line-level comments via GitLab discussions endpoint is still unmet.
If you want, I can draft a follow-up that maps structured findings with file/line metadata into
/discussionspositions, while keeping summary text in the marker note.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/opencode/src/cli/cmd/gitlab.ts` around lines 364 - 377, The current block only creates/updates a single MR note (commentBody) and never posts file/line inline discussions; extend this path to also create/update GitLab discussions for structured findings by mapping each finding's file/path and line/position into the GitLab discussions API. After computing commentBody as before, iterate the parsed structured findings (the array/structure you produce when generating reviewText) and call a new helper (e.g., postMRDiscussion or updateMRDiscussion) that uses the GitLab /projects/:id/merge_requests/:iid/discussions endpoint for each finding; keep the existing summary note logic (updateMRNote/postMRNote) to store the marker comment, but add calls to create per-file inline discussions using the same token/instanceUrl/projectId/mrIid and the finding's file and line metadata so reviewers see line-level comments in the MR.
🧹 Nitpick comments (1)
packages/opencode/src/cli/cmd/gitlab.ts (1)
358-363: Return and clean up the Bus subscription.Line 454 subscribes without keeping an unsubscribe handle. In repeated invocations within the same process (tests/embedded usage), listeners can accumulate and duplicate output.
♻️ Proposed refactor
- subscribeSessionEvents(session)+ const unsubscribe = subscribeSessionEvents(session)- const reviewText = await runReview(session.id, variant, providerID, modelID, reviewPrompt)+ const reviewText = await runReview(session.id, variant, providerID, modelID, reviewPrompt).finally(() =>+ unsubscribe(),+ )-function subscribeSessionEvents(session: { id: SessionID; title: string; version: string }) {+function subscribeSessionEvents(session: { id: SessionID; title: string; version: string }) { @@ - Bus.subscribe(MessageV2.Event.PartUpdated, async (evt) => {+ const unsubscribe = Bus.subscribe(MessageV2.Event.PartUpdated, async (evt) => { @@ - })+ })+ return unsubscribe }Also applies to: 454-479
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/opencode/src/cli/cmd/gitlab.ts` around lines 358 - 363, The subscription created by subscribeSessionEvents in gitlab.ts is never unsubscribed, causing listener accumulation on repeated runs; modify the call in the function that calls subscribeSessionEvents (the same scope that awaits runReview) to capture its unsubscribe/cleanup handle (e.g., const unsubscribe = subscribeSessionEvents(session) or similar return value) and ensure you call that unsubscribe/cleanup after runReview completes or on early exits/errors (use finally or equivalent). Update any other places noted (the 454–479 block) that call subscribeSessionEvents to follow the same pattern so listeners are removed when done.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/opencode/src/cli/cmd/gitlab.ts`:
- Around line 164-165: The MR notes fetch is limited to a single page of 100
(`/projects/${projectId}/merge_requests/${mrIid}/notes?sort=asc&per_page=100`)
so the dedup/update code later misses marker notes on subsequent pages; change
the notes retrieval to paginate until all pages are retrieved (follow GitLab
pagination via page/next-page or X-Next-Page header) and return a concatenated
list used by the dedup/update logic (identify the fetch call that uses token and
the dedup/update block that checks existing notes, e.g., the function that
constructs that URL and the code around the dedupe check) so the marker note is
found regardless of page.
- Around line 460-463: The current assignment to title incorrectly groups the ||
with the ternary, causing part.state.title to be ignored; update the expression
used when computing title (the const title variable) so the ternary binds
correctly — e.g., ensure you use parentheses so it reads: part.state.title ||
(Object.keys(part.state.input).length > 0 ? JSON.stringify(part.state.input) :
"Unknown"), referencing part.state.title and part.state.input to determine the
displayed title.
- Around line 108-112: The fetch call in gitlab.ts (where `const res = await
fetch(url, {...})` is used by fetchMRMetadata, fetchMRChanges, and fetchMRNotes)
must use an AbortController to enforce a timeout: create an AbortController,
pass its signal to fetch, set a timer (e.g., configurable default like 10s) that
calls controller.abort(), and clear the timer after fetch completes; update the
surrounding function to handle an aborted request (detect AbortError or
response.ok false) and surface a clear timeout error. Ensure the timer is
cleaned up on success or failure and make the timeout value configurable via an
option or constant used by those callers.
---
Duplicate comments:
In `@packages/opencode/src/cli/cmd/gitlab.ts`:
- Around line 364-377: The current block only creates/updates a single MR note
(commentBody) and never posts file/line inline discussions; extend this path to
also create/update GitLab discussions for structured findings by mapping each
finding's file/path and line/position into the GitLab discussions API. After
computing commentBody as before, iterate the parsed structured findings (the
array/structure you produce when generating reviewText) and call a new helper
(e.g., postMRDiscussion or updateMRDiscussion) that uses the GitLab
/projects/:id/merge_requests/:iid/discussions endpoint for each finding; keep
the existing summary note logic (updateMRNote/postMRNote) to store the marker
comment, but add calls to create per-file inline discussions using the same
token/instanceUrl/projectId/mrIid and the finding's file and line metadata so
reviewers see line-level comments in the MR.
---
Nitpick comments:
In `@packages/opencode/src/cli/cmd/gitlab.ts`:
- Around line 358-363: The subscription created by subscribeSessionEvents in
gitlab.ts is never unsubscribed, causing listener accumulation on repeated runs;
modify the call in the function that calls subscribeSessionEvents (the same
scope that awaits runReview) to capture its unsubscribe/cleanup handle (e.g.,
const unsubscribe = subscribeSessionEvents(session) or similar return value) and
ensure you call that unsubscribe/cleanup after runReview completes or on early
exits/errors (use finally or equivalent). Update any other places noted (the
454–479 block) that call subscribeSessionEvents to follow the same pattern so
listeners are removed when done.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: a028bb8a-513a-4c10-a77d-fa6baa4bc208
📒 Files selected for processing (1)
packages/opencode/src/cli/cmd/gitlab.ts
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
VJ-yadav
commented
Apr 4, 2026
ContributorAuthor
Tested against a GitLab MR containing a Python file with intentional security issues (SQL injection, command injection, hardcoded secrets, eval(), path traversal). SETUP: altimate-code gitlab review "https://gitlab.com/altimatecode/mr-review-test/-/merge_requests/1 --model opencode/big-pickle"
 The AI caught all intentional issues: hardcoded credentials, SQL injection, command injection via eval()/os.system()/subprocess.call(shell=True), and path traversal. Review is posted with a marker for deduplication. Additional behavior verified: Re-running the command updates the existing review note instead of creating duplicates |
- Add 30s timeout to gitlabApi fetch via AbortController to prevent indefinite hangs on network stalls - Paginate MR notes (loop pages of 100) so dedup marker detection works on active MRs with >100 notes - Fix ternary precedence in tool title rendering — part.state.title was being ignored due to || binding with the ternary condition Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Uh oh!
There was an error while loading. Please reload this page.
10 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
altimate-code gitlab review <mr-url>CLI command for reviewing GitLab merge requestsTest Plan
GITLAB_TOKEN=xxx altimate-code gitlab review https://gitlab.com/org/repo/-/merge_requests/123--no-post-commentflag to review without posting--modelflag to override the AI modelChecklist
fetch)anytypes — all errors typed asunknown, brandedProviderID/ModelIDtypes usedelsestatementsletmutations (except event subscriber accumulator, matchinggithub.tspattern).catch()preferred overtry/catchmaskToken()helper for error messagesGITLAB_INSTANCE_URLenv varextractResponseTextandformatPromptTooLargeErrorfromgithub.tsaltimate_changemarker convention inindex.tsFixes#618
Summary by CodeRabbit