feat(web): GitLab MR Review Agent#1104
Conversation
Adds support for the AI Review Agent to review GitLab Merge Requests, mirroring the existing GitHub PR review functionality. Also fixes several bugs discovered during implementation and improves the shared review pipeline.
---
## New files
### `packages/web/src/features/agents/review-agent/nodes/gitlabMrParser.ts`
Parses a GitLab MR webhook payload into the shared `sourcebot_pr_payload` format. Calls `MergeRequests.show()` and `MergeRequests.allDiffs()` in parallel — the API response is used for `title`, `description`, `sha`, and `diff_refs` (which can be absent in webhook payloads for `update` action events), while per-file diffs are parsed using the existing `parse-diff` library.
### `packages/web/src/features/agents/review-agent/nodes/gitlabPushMrReviews.ts`
Posts review comments back to GitLab using `MergeRequestDiscussions.create()` with a position object carrying `base_sha`, `head_sha`, and `start_sha`. Falls back to `MergeRequestNotes.create()` (a general MR note) if the inline comment is rejected by the API (e.g. the line is not within the diff), ensuring reviews are always surfaced even when precise positioning fails.
### Test files (4 new files, 34 tests total)
- `githubPrParser.test.ts` — diff parsing and metadata mapping for the GitHub parser
- `githubPushPrReviews.test.ts` — single-line vs multi-line comment parameters, error resilience
- `gitlabMrParser.test.ts` — API call arguments, metadata mapping, diff parsing, edge cases (empty diffs, nested groups, null description, API failures)
- `gitlabPushMrReviews.test.ts` — inline comment posting, fallback behaviour, missing `diff_refs` guard, multi-file iteration
---
## Modified files
### `packages/web/src/features/agents/review-agent/types.ts`
- Added `sourcebot_diff_refs` schema/type (`base_sha`, `head_sha`, `start_sha`) and an optional `diff_refs` field on `sourcebot_pr_payload`
- Added `GitLabMergeRequestPayload` and `GitLabNotePayload` interfaces for webhook event typing
### `packages/web/src/features/agents/review-agent/app.ts`
- Added `processGitLabMergeRequest()` function mirroring `processGitHubPullRequest()`: sets up logging, runs the GitLab parser, generates reviews via the shared LLM pipeline, and pushes results
- Removed stale `OPENAI_API_KEY` guards (model availability is now enforced inside `invokeDiffReviewLlm`)
### `packages/web/src/features/agents/review-agent/nodes/invokeDiffReviewLlm.ts`
**Replaces the hardcoded OpenAI client** with the Vercel AI SDK's `generateText` and the shared `getAISDKLanguageModelAndOptions` / `getConfiguredLanguageModels` utilities from `chat/utils.server.ts`. The review agent now uses whichever language model is configured in `config.json`, supporting all providers (Anthropic, Bedrock, Azure, etc.).
- `REVIEW_AGENT_MODEL` env var (matched against `displayName`) selects a specific model when multiple are configured; falls back to `models[0]` with a warning if the name is not found
- Prompt is passed via the `system` parameter with a `"Review the code changes."` user turn, satisfying providers (e.g. Bedrock/Anthropic) that require conversations to begin with a user message
### `packages/web/src/features/agents/review-agent/nodes/fetchFileContent.ts`
**Fixes "Not authenticated" error** when the review agent calls `getFileSource`. The original implementation used `withOptionalAuth`, which reads a session cookie — absent in webhook handlers. Now calls `getFileSourceForRepo` directly with `__unsafePrisma` and the single-tenant org, bypassing the session-based auth layer. The webhook handler has already authenticated the request via its own mechanism (GitHub App signature / GitLab token).
### `packages/web/src/features/git/getFileSourceApi.ts`
- Extracted the core repo-lookup + git + language-detection logic into a new exported `getFileSourceForRepo({ path, repo, ref }, { org, prisma })` function
- `getFileSource` now handles auth and audit logging then delegates to `getFileSourceForRepo` — all existing callers are unchanged
### `packages/web/src/app/api/(server)/webhook/route.ts`
- Added GitLab webhook handling alongside the existing GitHub branch
- Verifies `x-gitlab-token` against `GITLAB_REVIEW_AGENT_WEBHOOK_SECRET`
- Handles `Merge Request Hook` events (auto-review on `open`, `update`, `reopen`) and `Note Hook` events (manual `/review` command on MR comments)
- Initialises a `Gitlab` client at module load if `GITLAB_REVIEW_AGENT_TOKEN` is set
### `packages/web/src/app/(app)/agents/page.tsx`
- Split the single "Review Agent" card into two separate cards: **GitHub Review Agent** and **GitLab Review Agent**, each showing its own configuration status
- Removed `OPENAI_API_KEY` from the GitHub card's required env vars (no longer applicable)
### `packages/web/src/app/(app)/components/navigationMenu/navigationItems.tsx` & `index.tsx`
- Added an **Agents** nav item (with `BotIcon`) between Repositories and Settings
- Visible when the user is authenticated **and** at least one agent is configured (GitHub App triple or GitLab token pair), computed in the server component and passed down as `isAgentsVisible`
### `packages/shared/src/env.server.ts`
Added four new environment variables:
| Variable | Purpose |
|---|---|
| `GITLAB_REVIEW_AGENT_WEBHOOK_SECRET` | Verifies the `x-gitlab-token` header on incoming webhooks |
| `GITLAB_REVIEW_AGENT_TOKEN` | Personal or project access token used for GitLab API calls |
| `GITLAB_REVIEW_AGENT_HOST` | GitLab hostname (defaults to `gitlab.com`; set for self-hosted instances) |
| `REVIEW_AGENT_MODEL` | `displayName` of the configured language model to use for reviews; falls back to the first model if unset or not matched |
### `packages/web/package.json`
Added `@gitbeaker/rest` dependency (already used in `packages/backend`).
---
## Bug fixes
| Bug | Fix |
|---|---|
| `"Not authenticated"` when fetching file content from the review agent | `fetchFileContent` now calls `getFileSourceForRepo` directly instead of `getFileSource` (which gates on session auth) |
| `"diff_refs is missing"` when posting GitLab MR reviews | `gitlabMrParser` now fetches the full MR via `MergeRequests.show()` instead of relying on the webhook payload, which omits `diff_refs` on `update` events |
| Bedrock/Anthropic rejection: `"A conversation must start with a user message"` | `invokeDiffReviewLlm` now passes the prompt via `system` + a `prompt` user turn instead of a `system`-role entry inside `messages` |
| Review agent silently used `models[0]` with no way to specify a different model | New `REVIEW_AGENT_MODEL` env var selects by `displayName` |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
WalkthroughThis pull request adds GitLab support to the review agent: new env vars, webhook handling, MR parsing, and review posting. UI now shows separate GitHub and GitLab agents and conditional “Agents” navigation. LLM invocation is refactored to select configured models via the AI SDK. Changes
Sequence DiagramsequenceDiagram
participant User as User
participant Webhook as Webhook Handler
participant GitLab as GitLab API
participant Parser as MR Parser
participant LLM as LLM Service
participant Pusher as MR Pusher
User->>Webhook: POST GitLab webhook (MR or note)
Webhook->>Webhook: validate x-gitlab-token vs GITLAB_REVIEW_AGENT_WEBHOOK_SECRET
Webhook->>GitLab: MergeRequests.show(projectId, mrIid)
GitLab-->>Webhook: MR details
Webhook->>Parser: gitlabMrParser(client, payload)
Parser->>GitLab: MergeRequests.allDiffs(projectId, mrIid)
GitLab-->>Parser: file diffs
Parser-->>Webhook: parsed PR payload (with file_diffs, diff_refs)
Webhook->>LLM: generatePrReviews(prPayload)
LLM-->>Webhook: review items per file
Webhook->>Pusher: gitlabPushMrReviews(client, projectId, prPayload, reviews)
Pusher->>GitLab: MergeRequestDiscussions.create (inline)
alt inline succeeds
GitLab-->>Pusher: discussion created
else inline fails
Pusher->>GitLab: MergeRequestNotes.create (fallback note)
GitLab-->>Pusher: note created
end
Pusher-->>Webhook: done
Webhook-->>User: { status: "ok" }
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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: 6
🧹 Nitpick comments (7)
packages/web/src/features/git/getFileSourceApi.ts (1)
10-10: Preferimport typefor purely type-level imports.
OrgandPrismaClientare only used in type positions here.Suggested cleanup
-import { Org, PrismaClient } from '@sourcebot/db'; +import type { Org, PrismaClient } from '@sourcebot/db';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/web/src/features/git/getFileSourceApi.ts` at line 10, The import currently brings in Org and PrismaClient as runtime imports even though they are only used as types; change the import in getFileSourceApi.ts to a type-only import by replacing "import { Org, PrismaClient } from '@sourcebot/db';" with "import type { Org, PrismaClient } from '@sourcebot/db';" and ensure no runtime references to Org or PrismaClient remain so the type-only import is valid.packages/web/src/app/(app)/agents/page.tsx (1)
30-42: Single-agent layout branches are now dead code.Since
agentsis statically two entries, theagents.length === 1conditionals are no longer reachable and can be simplified.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/web/src/app/`(app)/agents/page.tsx around lines 30 - 42, The conditional branches checking agents.length === 1 are dead code; simplify the layout by removing the single-agent branches and using the multi-agent classes directly for the container and the agent card. Update the outer container JSX (where agents.map is rendered) to always use "grid grid-cols-1 md:grid-cols-2 lg:grid-cols-3 gap-10" and update the agent card JSX (the div with key={agent.id} and className logic) to always use the multi-agent class "relative flex flex-col items-center border border-border rounded-2xl p-8 bg-card shadow-xl" so the unnecessary ternary checks around agents.length are eliminated.packages/web/src/features/agents/review-agent/nodes/fetchFileContent.ts (1)
13-18: Avoid re-querying the org on every file fetch call.
fetchFileContentis called repeatedly per diff, so this org lookup is repeated unnecessarily. Consider resolvingorgonce upstream and passing it down.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/web/src/features/agents/review-agent/nodes/fetchFileContent.ts` around lines 13 - 18, The org lookup inside fetchFileContent (currently using __unsafePrisma.org.findUnique with SINGLE_TENANT_ORG_ID and throwing if org is missing) causes repeated DB queries; refactor fetchFileContent to accept an org (or orgId/needed org fields) as an argument and remove the internal __unsafePrisma.org.findUnique call so callers resolve org once upstream and pass it in, or alternatively add a lightweight in-memory cache keyed by SINGLE_TENANT_ORG_ID and reuse the cached org instead of re-querying each call.packages/web/src/features/agents/review-agent/app.ts (1)
62-80: Consider extracting duplicate log setup logic.The log path initialization (directory creation, timestamp formatting, path construction) is duplicated between
processGitHubPullRequest(lines 29-47) andprocessGitLabMergeRequest(lines 62-80). This could be extracted to a shared helper.♻️ Example helper extraction
function getReviewAgentLogPath(identifier: string): string | undefined { if (!env.REVIEW_AGENT_LOGGING_ENABLED) { return undefined; } const reviewAgentLogDir = path.join(env.DATA_CACHE_DIR, "review-agent"); if (!fs.existsSync(reviewAgentLogDir)) { fs.mkdirSync(reviewAgentLogDir, { recursive: true }); } const timestamp = new Date().toLocaleString('en-US', { year: 'numeric', month: '2-digit', day: '2-digit', hour: '2-digit', minute: '2-digit', second: '2-digit', hour12: false }).replace(/(\d+)\/(\d+)\/(\d+), (\d+):(\d+):(\d+)/, '$3_$1_$2_$4_$5_$6'); const logPath = path.join(reviewAgentLogDir, `review-agent-${identifier}-${timestamp}.log`); logger.info(`Review agent logging to ${logPath}`); return logPath; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/web/src/features/agents/review-agent/app.ts` around lines 62 - 80, Extract the duplicated log setup in processGitHubPullRequest and processGitLabMergeRequest into a single helper (e.g., getReviewAgentLogPath(identifier)) that checks env.REVIEW_AGENT_LOGGING_ENABLED, ensures the review-agent directory exists (fs.existsSync + fs.mkdirSync with recursive), builds the timestamp using the same toLocaleString/replace logic, composes the filename with the provided identifier (replace uses of mrPayload.object_attributes.iid or pullRequest number), calls logger.info with the final path, and returns the path or undefined; then replace the duplicate blocks in both functions with a call to this helper.packages/web/src/app/api/(server)/webhook/route.ts (2)
246-252: Same error handling concern for note event processing.Apply similar try-catch wrapping here to prevent webhook retry storms on processing errors.
♻️ Suggested error handling
+ try { await processGitLabMergeRequest( gitlabClient, body.project.id, mrPayload, env.GITLAB_REVIEW_AGENT_HOST, ); + } catch (error) { + logger.error('Error processing GitLab note event:', error); + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/web/src/app/api/`(server)/webhook/route.ts around lines 246 - 252, Wrap the call to processGitLabMergeRequest in a try-catch to mirror the note-event handling: invoke await processGitLabMergeRequest(gitlabClient, body.project.id, mrPayload, env.GITLAB_REVIEW_AGENT_HOST) inside try, and on catch log the error with context (including body.project.id and event type) and return a successful webhook response so GitLab doesn't retry; ensure you reference processGitLabMergeRequest, gitlabClient, mrPayload and env.GITLAB_REVIEW_AGENT_HOST when adding the logging/context.
220-226: Consider wrappingprocessGitLabMergeRequestin try-catch to prevent retry storms.If
processGitLabMergeRequestthrows, the webhook returns a 500 error, which may cause GitLab to retry the webhook delivery repeatedly. This could lead to duplicate review attempts or excessive load.♻️ Suggested error handling
if (isGitLabMergeRequestEvent(gitlabEvent, body)) { if (env.REVIEW_AGENT_AUTO_REVIEW_ENABLED === "false") { logger.info('Review agent auto review (REVIEW_AGENT_AUTO_REVIEW_ENABLED) is disabled, skipping'); return Response.json({ status: 'ok' }); } + try { await processGitLabMergeRequest( gitlabClient, body.project.id, body, env.GITLAB_REVIEW_AGENT_HOST, ); + } catch (error) { + logger.error('Error processing GitLab merge request:', error); + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/web/src/app/api/`(server)/webhook/route.ts around lines 220 - 226, Wrap the call to processGitLabMergeRequest(gitlabClient, body.project.id, body, env.GITLAB_REVIEW_AGENT_HOST) in a try-catch inside the webhook route handler; on error catch the exception, log the full error (including contextual values like body.project.id and env.GITLAB_REVIEW_AGENT_HOST) using the existing logger (or console.error if none), and return a non-retriable HTTP response (e.g., 200/202 with an error payload) instead of letting the exception bubble up to produce a 500. Ensure the catch only suppresses delivery retries while still surfacing enough detail in logs for debugging.packages/web/src/features/agents/review-agent/nodes/invokeDiffReviewLlm.ts (1)
35-41: Consider swapping system and prompt content for better provider compatibility.The detailed review prompt is placed in
systemwhile the user turn only contains "Review the code changes." This inverted usage may cause suboptimal results with some providers that expect the system message to contain role/behavior instructions and the user message to contain the actual task content.♻️ Suggested refactor
const result = await generateText({ model, - system: prompt, - prompt: "Review the code changes.", + system: "You are a code review assistant. Respond only with valid JSON matching the expected schema.", + prompt: prompt, providerOptions, temperature, });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/web/src/features/agents/review-agent/nodes/invokeDiffReviewLlm.ts` around lines 35 - 41, The generateText call in invokeDiffReviewLlm.ts currently passes the detailed review instructions in the system parameter and a generic "Review the code changes." as the prompt, which can confuse some providers; update the call to use a concise role/behavior string for system (e.g., a short reviewer persona) and pass the detailed review text stored in the prompt variable as the prompt/user message instead. Modify the generateText invocation (function generateText, params model, system, prompt, providerOptions, temperature) so system contains role/behavior guidance and prompt is the detailed review instructions currently in the prompt variable, keeping providerOptions and temperature unchanged.
🤖 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/shared/src/env.server.ts`:
- Around line 198-202: The GITLAB_REVIEW_AGENT_HOST env entry currently accepts
any string which can include a protocol and break the later `https://${...}` URL
assembly; update the env schema for GITLAB_REVIEW_AGENT_HOST in
packages/shared/src/env.server.ts (the env schema definition) to
validate/normalize it as a hostname: replace the current
z.string().default('gitlab.com') with a schema that strips any leading
protocol/slashes and validates the remainder as a hostname (e.g., use
z.string().default('gitlab.com').transform(s => s.replace(/^https?:\/\//,
'').replace(/\/+$/, '')).refine(s => /^[a-z0-9.-]+$/i.test(s), { message:
'invalid hostname' })), so downstream code that constructs
`https://${GITLAB_REVIEW_AGENT_HOST}` always receives a bare hostname.
In `@packages/web/src/features/agents/review-agent/nodes/fetchFileContent.ts`:
- Around line 21-27: fileSourceRequest currently omits the Git ref so
getFileSourceForRepo may read the default branch; add a ref property to
fileSourceRequest set to the PR/MR head SHA (the head SHA variable available in
this scope) before calling getFileSourceForRepo so the fetch uses the exact
revision under review; update the object passed to getFileSourceForRepo
(fileSourceRequest) to include ref and ensure you pass the same org and
__unsafePrisma options.
In
`@packages/web/src/features/agents/review-agent/nodes/githubPushPrReviews.test.ts`:
- Around line 133-139: The async test "does not throw when all review comments
fail" incorrectly uses resolves.not.toThrow(); update the assertion to await
that the promise resolves to undefined by replacing the matcher with
resolves.toBeUndefined() when calling githubPushPrReviews(octokit, MOCK_PAYLOAD,
SINGLE_REVIEW) so the test correctly asserts the resolved value; locate the test
block containing githubPushPrReviews and change the expectation accordingly.
In `@packages/web/src/features/agents/review-agent/nodes/gitlabMrParser.ts`:
- Around line 99-101: The code in gitlabMrParser.ts currently force-casts
mr.diff_refs to a typed object when building the MR payload
(head_sha/diff_refs), which can throw if mr.diff_refs is null; update the logic
in the function that constructs the object (where mr.diff_refs is accessed) to
first validate mr.diff_refs !== null && mr.diff_refs !== undefined before the
type assertion, and if missing, set diff_refs to undefined or an empty safe
fallback and ensure head_sha is derived from mr.sha or guarded similarly so
downstream gitlabPushMrReviews can gracefully fall back to general notes; this
avoids runtime errors by failing over rather than force-casting a potentially
null value.
In `@packages/web/src/features/agents/review-agent/nodes/gitlabPushMrReviews.ts`:
- Around line 29-38: The position object sent to GitLab is using camelCase and
wrong types; update the object in gitlabPushMrReviews.ts (the position block
that references base_sha, head_sha, start_sha, fileDiffReview.filename and
review.line_end) to use GitLab's snake_case keys: position_type, base_sha,
head_sha, start_sha, new_path and include old_path (set to
fileDiffReview.filename), and ensure new_line is a number (use review.line_end
as a number, not String). Preserve the surrounding structure but replace
camelCase keys (baseSha, headSha, startSha, newPath, newLine) with the correct
snake_case ones so `@gitbeaker/rest` forwards a valid GitLab API payload.
In `@packages/web/src/features/agents/review-agent/types.ts`:
- Around line 59-74: The GitLab webhook interfaces assume diff_refs is always
present, which is unsafe; update
GitLabMergeRequestPayload.object_attributes.diff_refs and
GitLabNotePayload.merge_request.diff_refs to be optional or nullable (e.g., mark
as diff_refs?: { base_sha: string; head_sha: string; start_sha: string } | null)
so downstream code must handle missing/null diff_refs; adjust any related type
imports/exports if necessary and run type checks to ensure no remaining non-null
assumptions.
---
Nitpick comments:
In `@packages/web/src/app/`(app)/agents/page.tsx:
- Around line 30-42: The conditional branches checking agents.length === 1 are
dead code; simplify the layout by removing the single-agent branches and using
the multi-agent classes directly for the container and the agent card. Update
the outer container JSX (where agents.map is rendered) to always use "grid
grid-cols-1 md:grid-cols-2 lg:grid-cols-3 gap-10" and update the agent card JSX
(the div with key={agent.id} and className logic) to always use the multi-agent
class "relative flex flex-col items-center border border-border rounded-2xl p-8
bg-card shadow-xl" so the unnecessary ternary checks around agents.length are
eliminated.
In `@packages/web/src/app/api/`(server)/webhook/route.ts:
- Around line 246-252: Wrap the call to processGitLabMergeRequest in a try-catch
to mirror the note-event handling: invoke await
processGitLabMergeRequest(gitlabClient, body.project.id, mrPayload,
env.GITLAB_REVIEW_AGENT_HOST) inside try, and on catch log the error with
context (including body.project.id and event type) and return a successful
webhook response so GitLab doesn't retry; ensure you reference
processGitLabMergeRequest, gitlabClient, mrPayload and
env.GITLAB_REVIEW_AGENT_HOST when adding the logging/context.
- Around line 220-226: Wrap the call to processGitLabMergeRequest(gitlabClient,
body.project.id, body, env.GITLAB_REVIEW_AGENT_HOST) in a try-catch inside the
webhook route handler; on error catch the exception, log the full error
(including contextual values like body.project.id and
env.GITLAB_REVIEW_AGENT_HOST) using the existing logger (or console.error if
none), and return a non-retriable HTTP response (e.g., 200/202 with an error
payload) instead of letting the exception bubble up to produce a 500. Ensure the
catch only suppresses delivery retries while still surfacing enough detail in
logs for debugging.
In `@packages/web/src/features/agents/review-agent/app.ts`:
- Around line 62-80: Extract the duplicated log setup in
processGitHubPullRequest and processGitLabMergeRequest into a single helper
(e.g., getReviewAgentLogPath(identifier)) that checks
env.REVIEW_AGENT_LOGGING_ENABLED, ensures the review-agent directory exists
(fs.existsSync + fs.mkdirSync with recursive), builds the timestamp using the
same toLocaleString/replace logic, composes the filename with the provided
identifier (replace uses of mrPayload.object_attributes.iid or pullRequest
number), calls logger.info with the final path, and returns the path or
undefined; then replace the duplicate blocks in both functions with a call to
this helper.
In `@packages/web/src/features/agents/review-agent/nodes/fetchFileContent.ts`:
- Around line 13-18: The org lookup inside fetchFileContent (currently using
__unsafePrisma.org.findUnique with SINGLE_TENANT_ORG_ID and throwing if org is
missing) causes repeated DB queries; refactor fetchFileContent to accept an org
(or orgId/needed org fields) as an argument and remove the internal
__unsafePrisma.org.findUnique call so callers resolve org once upstream and pass
it in, or alternatively add a lightweight in-memory cache keyed by
SINGLE_TENANT_ORG_ID and reuse the cached org instead of re-querying each call.
In `@packages/web/src/features/agents/review-agent/nodes/invokeDiffReviewLlm.ts`:
- Around line 35-41: The generateText call in invokeDiffReviewLlm.ts currently
passes the detailed review instructions in the system parameter and a generic
"Review the code changes." as the prompt, which can confuse some providers;
update the call to use a concise role/behavior string for system (e.g., a short
reviewer persona) and pass the detailed review text stored in the prompt
variable as the prompt/user message instead. Modify the generateText invocation
(function generateText, params model, system, prompt, providerOptions,
temperature) so system contains role/behavior guidance and prompt is the
detailed review instructions currently in the prompt variable, keeping
providerOptions and temperature unchanged.
In `@packages/web/src/features/git/getFileSourceApi.ts`:
- Line 10: The import currently brings in Org and PrismaClient as runtime
imports even though they are only used as types; change the import in
getFileSourceApi.ts to a type-only import by replacing "import { Org,
PrismaClient } from '@sourcebot/db';" with "import type { Org, PrismaClient }
from '@sourcebot/db';" and ensure no runtime references to Org or PrismaClient
remain so the type-only import is valid.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a2462b8d-0146-426e-8d4c-f6797bd49a90
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (18)
.gitignorepackages/shared/src/env.server.tspackages/web/package.jsonpackages/web/src/app/(app)/agents/page.tsxpackages/web/src/app/(app)/components/navigationMenu/index.tsxpackages/web/src/app/(app)/components/navigationMenu/navigationItems.tsxpackages/web/src/app/api/(server)/webhook/route.tspackages/web/src/features/agents/review-agent/app.tspackages/web/src/features/agents/review-agent/nodes/fetchFileContent.tspackages/web/src/features/agents/review-agent/nodes/githubPrParser.test.tspackages/web/src/features/agents/review-agent/nodes/githubPushPrReviews.test.tspackages/web/src/features/agents/review-agent/nodes/gitlabMrParser.test.tspackages/web/src/features/agents/review-agent/nodes/gitlabMrParser.tspackages/web/src/features/agents/review-agent/nodes/gitlabPushMrReviews.test.tspackages/web/src/features/agents/review-agent/nodes/gitlabPushMrReviews.tspackages/web/src/features/agents/review-agent/nodes/invokeDiffReviewLlm.tspackages/web/src/features/agents/review-agent/types.tspackages/web/src/features/git/getFileSourceApi.ts
| // GitLab for review agent | ||
| GITLAB_REVIEW_AGENT_WEBHOOK_SECRET: z.string().optional(), | ||
| GITLAB_REVIEW_AGENT_TOKEN: z.string().optional(), | ||
| GITLAB_REVIEW_AGENT_HOST: z.string().default('gitlab.com'), | ||
| REVIEW_AGENT_MODEL: z.string().optional(), |
There was a problem hiding this comment.
Validate/normalize GITLAB_REVIEW_AGENT_HOST as a hostname.
GITLAB_REVIEW_AGENT_HOST is currently any string. Since the GitLab client host is later assembled as https://${...}, a value that already includes protocol will produce an invalid URL.
Suggested hardening
- GITLAB_REVIEW_AGENT_HOST: z.string().default('gitlab.com'),
+ GITLAB_REVIEW_AGENT_HOST: z.string()
+ .default('gitlab.com')
+ .transform(v => v.replace(/^https?:\/\//, '').replace(/\/+$/, '')),📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // GitLab for review agent | |
| GITLAB_REVIEW_AGENT_WEBHOOK_SECRET: z.string().optional(), | |
| GITLAB_REVIEW_AGENT_TOKEN: z.string().optional(), | |
| GITLAB_REVIEW_AGENT_HOST: z.string().default('gitlab.com'), | |
| REVIEW_AGENT_MODEL: z.string().optional(), | |
| // GitLab for review agent | |
| GITLAB_REVIEW_AGENT_WEBHOOK_SECRET: z.string().optional(), | |
| GITLAB_REVIEW_AGENT_TOKEN: z.string().optional(), | |
| GITLAB_REVIEW_AGENT_HOST: z.string() | |
| .default('gitlab.com') | |
| .transform(v => v.replace(/^https?:\/\//, '').replace(/\/+$/, '')), | |
| REVIEW_AGENT_MODEL: z.string().optional(), |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/shared/src/env.server.ts` around lines 198 - 202, The
GITLAB_REVIEW_AGENT_HOST env entry currently accepts any string which can
include a protocol and break the later `https://${...}` URL assembly; update the
env schema for GITLAB_REVIEW_AGENT_HOST in packages/shared/src/env.server.ts
(the env schema definition) to validate/normalize it as a hostname: replace the
current z.string().default('gitlab.com') with a schema that strips any leading
protocol/slashes and validates the remainder as a hostname (e.g., use
z.string().default('gitlab.com').transform(s => s.replace(/^https?:\/\//,
'').replace(/\/+$/, '')).refine(s => /^[a-z0-9.-]+$/i.test(s), { message:
'invalid hostname' })), so downstream code that constructs
`https://${GITLAB_REVIEW_AGENT_HOST}` always receives a bare hostname.
| const fileSourceRequest = { | ||
| path: filename, | ||
| repo: repoPath, | ||
| } | ||
| }; | ||
| logger.debug(JSON.stringify(fileSourceRequest, null, 2)); | ||
|
|
||
| const response = await getFileSource(fileSourceRequest); | ||
| const response = await getFileSourceForRepo(fileSourceRequest, { org, prisma: __unsafePrisma }); |
There was a problem hiding this comment.
Pass PR/MR head SHA as ref when fetching file content.
Right now fileSourceRequest omits ref, so getFileSourceForRepo can read from the repository default branch instead of the current PR/MR revision. That can produce incorrect context for LLM review.
Suggested fix
const fileSourceRequest = {
path: filename,
repo: repoPath,
+ ref: pr_payload.head_sha,
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const fileSourceRequest = { | |
| path: filename, | |
| repo: repoPath, | |
| } | |
| }; | |
| logger.debug(JSON.stringify(fileSourceRequest, null, 2)); | |
| const response = await getFileSource(fileSourceRequest); | |
| const response = await getFileSourceForRepo(fileSourceRequest, { org, prisma: __unsafePrisma }); | |
| const fileSourceRequest = { | |
| path: filename, | |
| repo: repoPath, | |
| ref: pr_payload.head_sha, | |
| }; | |
| logger.debug(JSON.stringify(fileSourceRequest, null, 2)); | |
| const response = await getFileSourceForRepo(fileSourceRequest, { org, prisma: __unsafePrisma }); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/web/src/features/agents/review-agent/nodes/fetchFileContent.ts`
around lines 21 - 27, fileSourceRequest currently omits the Git ref so
getFileSourceForRepo may read the default branch; add a ref property to
fileSourceRequest set to the PR/MR head SHA (the head SHA variable available in
this scope) before calling getFileSourceForRepo so the fetch uses the exact
revision under review; update the object passed to getFileSourceForRepo
(fileSourceRequest) to include ref and ensure you pass the same org and
__unsafePrisma options.
| test('does not throw when all review comments fail', async () => { | ||
| const octokit = makeMockOctokit('reject'); | ||
|
|
||
| await expect( | ||
| githubPushPrReviews(octokit, MOCK_PAYLOAD, SINGLE_REVIEW), | ||
| ).resolves.not.toThrow(); | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In Vitest, is await expect(promise).resolves.not.toThrow()valid when the promise resolves to undefined, or shouldresolves.toBeUndefined() be used instead?
💡 Result:
await expect(promise).resolves.not.toThrow() is not the right assertion when the promise resolves (e.g., to undefined). In Vitest, toThrow asserts that a function throws when called, and is meant for synchronous throws (or with rejects for async errors), not for “a promise resolved successfully”. [1]
If your promise resolves to undefined (typical for Promise<void>), use:
await expect(promise).resolves.toBeUndefined()because resolves unwraps the promise’s resolved value and then runs normal matchers against that value. [2]
Sources: Vitest toThrow docs [1], Vitest resolves docs [2].
[1] (vitest.dev)
[2] (vitest.dev)
Citations:
Use resolves.toBeUndefined() instead of resolves.not.toThrow() for this async assertion.
The toThrow matcher asserts that a function throws when called, not that a resolved promise value is valid. For a promise that resolves to undefined, use resolves.toBeUndefined() to properly unwrap and assert the resolved value.
Suggested fix
- await expect(
- githubPushPrReviews(octokit, MOCK_PAYLOAD, SINGLE_REVIEW),
- ).resolves.not.toThrow();
+ await expect(
+ githubPushPrReviews(octokit, MOCK_PAYLOAD, SINGLE_REVIEW),
+ ).resolves.toBeUndefined();📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| test('does not throw when all review comments fail', async () => { | |
| const octokit = makeMockOctokit('reject'); | |
| await expect( | |
| githubPushPrReviews(octokit, MOCK_PAYLOAD, SINGLE_REVIEW), | |
| ).resolves.not.toThrow(); | |
| }); | |
| test('does not throw when all review comments fail', async () => { | |
| const octokit = makeMockOctokit('reject'); | |
| await expect( | |
| githubPushPrReviews(octokit, MOCK_PAYLOAD, SINGLE_REVIEW), | |
| ).resolves.toBeUndefined(); | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@packages/web/src/features/agents/review-agent/nodes/githubPushPrReviews.test.ts`
around lines 133 - 139, The async test "does not throw when all review comments
fail" incorrectly uses resolves.not.toThrow(); update the assertion to await
that the promise resolves to undefined by replacing the matcher with
resolves.toBeUndefined() when calling githubPushPrReviews(octokit, MOCK_PAYLOAD,
SINGLE_REVIEW) so the test correctly asserts the resolved value; locate the test
block containing githubPushPrReviews and change the expectation accordingly.
| head_sha: mr.sha, | ||
| diff_refs: mr.diff_refs as { base_sha: string; head_sha: string; start_sha: string }, | ||
| }; |
There was a problem hiding this comment.
Consider adding validation for mr.diff_refs before type assertion.The GitLab API can return diff_refs as None (null) even when fetching via the API, particularly shortly after MR creation. The delay before diff_refs becomes available can be between 1-5 seconds. This type assertion could cause a runtime error if mr.diff_refs is null.
🛡️ Suggested validation
logger.debug("Completed gitlab_mr_parser");
+
+ if (!mr.diff_refs || !mr.diff_refs.base_sha || !mr.diff_refs.head_sha || !mr.diff_refs.start_sha) {
+ logger.warn("diff_refs not yet available from GitLab API");
+ }
+
return {
title: mr.title,
description: mr.description ?? "",
hostDomain,
owner: namespace,
repo: repoName,
file_diffs: filteredSourcebotFileDiffs,
number: mrIid,
head_sha: mr.sha,
- diff_refs: mr.diff_refs as { base_sha: string; head_sha: string; start_sha: string },
+ diff_refs: mr.diff_refs ? {
+ base_sha: mr.diff_refs.base_sha,
+ head_sha: mr.diff_refs.head_sha,
+ start_sha: mr.diff_refs.start_sha,
+ } : undefined,
};Note: The downstream gitlabPushMrReviews already handles missing diff_refs by falling back to general notes, so this change would gracefully degrade rather than risk a runtime error.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/web/src/features/agents/review-agent/nodes/gitlabMrParser.ts` around
lines 99 - 101, The code in gitlabMrParser.ts currently force-casts mr.diff_refs
to a typed object when building the MR payload (head_sha/diff_refs), which can
throw if mr.diff_refs is null; update the logic in the function that constructs
the object (where mr.diff_refs is accessed) to first validate mr.diff_refs !==
null && mr.diff_refs !== undefined before the type assertion, and if missing,
set diff_refs to undefined or an empty safe fallback and ensure head_sha is
derived from mr.sha or guarded similarly so downstream gitlabPushMrReviews can
gracefully fall back to general notes; this avoids runtime errors by failing
over rather than force-casting a potentially null value.
| { | ||
| position: { | ||
| positionType: "text", | ||
| baseSha: base_sha, | ||
| headSha: head_sha, | ||
| startSha: start_sha, | ||
| newPath: fileDiffReview.filename, | ||
| newLine: String(review.line_end), | ||
| }, | ||
| }, |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
@gitbeaker/rest MergeRequestDiscussions create position parameter format
💡 Result:
For @gitbeaker/rest MergeRequestDiscussions.create, the position you pass should match GitLab’s “Create a new thread in the merge request diff” payload: i.e., an object that contains the same snake_case keys GitLab documents under position[...] (GitBeaker forwards these as request fields). [1]
Single-line diff comment (position_type: "text")
Provide the SHA triplet from the latest MR version + file paths + either new_line or old_line (or both for unchanged lines):
position: {
position_type: 'text',
base_sha: '<base_commit_sha>',
start_sha: '<start_commit_sha>',
head_sha: '<head_commit_sha>',
new_path: 'path/to/file.ext',
old_path: 'path/to/file.ext',
// added line:
new_line: 18,
// removed line (use instead of new_line):
// old_line: 18,
// unchanged line (include both):
// new_line: 18,
// old_line: 18,
}Notes from GitLab:
position[old_path]andposition[new_path]are required for text positions.- Use only
new_linefor added lines, onlyold_linefor removed lines, and both for unchanged lines. [1]
Multi-line diff comment (position[line_range])
Add position[line_range][start] and position[line_range][end] with line_code + type:
position: {
position_type: 'text',
base_sha: '<base_commit_sha>',
start_sha: '<start_commit_sha>',
head_sha: '<head_commit_sha>',
new_path: 'path/to/file.ext',
old_path: 'path/to/file.ext',
line_range: {
start: { line_code: '<sha1(path)>_<old>_<new>', type: 'new', new_line: 10 },
end: { line_code: '<sha1(path)>_<old>_<new>', type: 'new', new_line: 12 },
},
}GitLab’s documented line_code format is: <SHA>_<old>_<new> where <SHA> is the SHA1 of the filename (path). [1] Also, line_code is required when using position.line_range. [2]
Where to get base_sha/start_sha/head_sha: call the MR “versions” endpoint and use the latest item’s base_commit_sha, start_commit_sha, and head_commit_sha. [1]
Sources: GitLab Discussions API docs (merge request diff discussions + line_range + line_code format) [1], StackOverflow clarification on line_code usage [2].
Fix position object property casing and format for GitLab API compatibility.
The position object uses incorrect camelCase properties when @gitbeaker/rest forwards these to GitLab's API as snake_case. Additionally, new_line must be a number, not a string, and old_path is required alongside new_path:
Correct format
{
position: {
position_type: "text",
base_sha: base_sha,
head_sha: head_sha,
start_sha: start_sha,
new_path: fileDiffReview.filename,
old_path: fileDiffReview.filename, // required
new_line: review.line_end, // number, not string
},
},Current properties (baseSha, headSha, startSha, newPath, newLine) will not be recognized by GitLab's API, causing the request to fail.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/web/src/features/agents/review-agent/nodes/gitlabPushMrReviews.ts`
around lines 29 - 38, The position object sent to GitLab is using camelCase and
wrong types; update the object in gitlabPushMrReviews.ts (the position block
that references base_sha, head_sha, start_sha, fileDiffReview.filename and
review.line_end) to use GitLab's snake_case keys: position_type, base_sha,
head_sha, start_sha, new_path and include old_path (set to
fileDiffReview.filename), and ensure new_line is a number (use review.line_end
as a number, not String). Preserve the surrounding structure but replace
camelCase keys (baseSha, headSha, startSha, newPath, newLine) with the correct
snake_case ones so `@gitbeaker/rest` forwards a valid GitLab API payload.
| export interface GitLabMergeRequestPayload { | ||
| object_kind: string; | ||
| object_attributes: { | ||
| iid: number; | ||
| title: string; | ||
| description: string | null; | ||
| action: string; | ||
| last_commit: { | ||
| id: string; | ||
| }; | ||
| diff_refs: { | ||
| base_sha: string; | ||
| head_sha: string; | ||
| start_sha: string; | ||
| }; | ||
| }; |
There was a problem hiding this comment.
Model GitLab diff_refs as nullable/optional in webhook interfaces.
These interfaces currently require diff_refs, but GitLab MR/note webhook payloads can omit or null it on some actions. Keeping it required creates unsafe assumptions in downstream code.
Suggested type fix
- diff_refs: {
+ diff_refs?: {
base_sha: string;
head_sha: string;
start_sha: string;
- };
+ } | null;Apply the same change to both GitLabMergeRequestPayload.object_attributes.diff_refs and GitLabNotePayload.merge_request.diff_refs.
Also applies to: 90-102
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/web/src/features/agents/review-agent/types.ts` around lines 59 - 74,
The GitLab webhook interfaces assume diff_refs is always present, which is
unsafe; update GitLabMergeRequestPayload.object_attributes.diff_refs and
GitLabNotePayload.merge_request.diff_refs to be optional or nullable (e.g., mark
as diff_refs?: { base_sha: string; head_sha: string; start_sha: string } | null)
so downstream code must handle missing/null diff_refs; adjust any related type
imports/exports if necessary and run type checks to ensure no remaining non-null
assumptions.

Adds support for the AI Review Agent to review GitLab Merge Requests, mirroring the existing GitHub PR review functionality. Also fixes several bugs discovered during implementation and improves the shared review pipeline.
New files
packages/web/src/features/agents/review-agent/nodes/gitlabMrParser.tsParses a GitLab MR webhook payload into the shared
sourcebot_pr_payloadformat. CallsMergeRequests.show()andMergeRequests.allDiffs()in parallel — the API response is used fortitle,description,sha, anddiff_refs(which can be absent in webhook payloads forupdateaction events), while per-file diffs are parsed using the existingparse-difflibrary.packages/web/src/features/agents/review-agent/nodes/gitlabPushMrReviews.tsPosts review comments back to GitLab using
MergeRequestDiscussions.create()with a position object carryingbase_sha,head_sha, andstart_sha. Falls back toMergeRequestNotes.create()(a general MR note) if the inline comment is rejected by the API (e.g. the line is not within the diff), ensuring reviews are always surfaced even when precise positioning fails.Test files (4 new files, 34 tests total)
githubPrParser.test.ts— diff parsing and metadata mapping for the GitHub parsergithubPushPrReviews.test.ts— single-line vs multi-line comment parameters, error resiliencegitlabMrParser.test.ts— API call arguments, metadata mapping, diff parsing, edge cases (empty diffs, nested groups, null description, API failures)gitlabPushMrReviews.test.ts— inline comment posting, fallback behaviour, missingdiff_refsguard, multi-file iterationModified files
packages/web/src/features/agents/review-agent/types.tssourcebot_diff_refsschema/type (base_sha,head_sha,start_sha) and an optionaldiff_refsfield onsourcebot_pr_payloadGitLabMergeRequestPayloadandGitLabNotePayloadinterfaces for webhook event typingpackages/web/src/features/agents/review-agent/app.tsprocessGitLabMergeRequest()function mirroringprocessGitHubPullRequest(): sets up logging, runs the GitLab parser, generates reviews via the shared LLM pipeline, and pushes resultsOPENAI_API_KEYguards (model availability is now enforced insideinvokeDiffReviewLlm)packages/web/src/features/agents/review-agent/nodes/invokeDiffReviewLlm.tsReplaces the hardcoded OpenAI client with the Vercel AI SDK's
generateTextand the sharedgetAISDKLanguageModelAndOptions/getConfiguredLanguageModelsutilities fromchat/utils.server.ts. The review agent now uses whichever language model is configured inconfig.json, supporting all providers (Anthropic, Bedrock, Azure, etc.).REVIEW_AGENT_MODELenv var (matched againstdisplayName) selects a specific model when multiple are configured; falls back tomodels[0]with a warning if the name is not foundsystemparameter with a"Review the code changes."user turn, satisfying providers (e.g. Bedrock/Anthropic) that require conversations to begin with a user messagepackages/web/src/features/agents/review-agent/nodes/fetchFileContent.tsFixes "Not authenticated" error when the review agent calls
getFileSource. The original implementation usedwithOptionalAuth, which reads a session cookie — absent in webhook handlers. Now callsgetFileSourceForRepodirectly with__unsafePrismaand the single-tenant org, bypassing the session-based auth layer. The webhook handler has already authenticated the request via its own mechanism (GitHub App signature / GitLab token).packages/web/src/features/git/getFileSourceApi.tsgetFileSourceForRepo({ path, repo, ref }, { org, prisma })functiongetFileSourcenow handles auth and audit logging then delegates togetFileSourceForRepo— all existing callers are unchangedpackages/web/src/app/api/(server)/webhook/route.tsx-gitlab-tokenagainstGITLAB_REVIEW_AGENT_WEBHOOK_SECRETMerge Request Hookevents (auto-review onopen,update,reopen) andNote Hookevents (manual/reviewcommand on MR comments)Gitlabclient at module load ifGITLAB_REVIEW_AGENT_TOKENis setpackages/web/src/app/(app)/agents/page.tsxOPENAI_API_KEYfrom the GitHub card's required env vars (no longer applicable)packages/web/src/app/(app)/components/navigationMenu/navigationItems.tsx&index.tsxBotIcon) between Repositories and SettingsisAgentsVisiblepackages/shared/src/env.server.tsAdded four new environment variables:
GITLAB_REVIEW_AGENT_WEBHOOK_SECRETx-gitlab-tokenheader on incoming webhooksGITLAB_REVIEW_AGENT_TOKENGITLAB_REVIEW_AGENT_HOSTgitlab.com; set for self-hosted instances)REVIEW_AGENT_MODELdisplayNameof the configured language model to use for reviews; falls back to the first model if unset or not matchedpackages/web/package.jsonAdded
@gitbeaker/restdependency (already used inpackages/backend).Bug fixes
"Not authenticated"when fetching file content from the review agentfetchFileContentnow callsgetFileSourceForRepodirectly instead ofgetFileSource(which gates on session auth)"diff_refs is missing"when posting GitLab MR reviewsgitlabMrParsernow fetches the full MR viaMergeRequests.show()instead of relying on the webhook payload, which omitsdiff_refsonupdateevents"A conversation must start with a user message"invokeDiffReviewLlmnow passes the prompt viasystem+ apromptuser turn instead of asystem-role entry insidemessagesmodels[0]with no way to specify a different modelREVIEW_AGENT_MODELenv var selects bydisplayNameSummary by CodeRabbit
New Features
Chores