Skip to content

feat(web): GitLab MR Review Agent#1104

Open
fatmcgav wants to merge 2 commits intosourcebot-dev:mainfrom
fatmcgav:feat-gitlab-mr-agent
Open

feat(web): GitLab MR Review Agent#1104
fatmcgav wants to merge 2 commits intosourcebot-dev:mainfrom
fatmcgav:feat-gitlab-mr-agent

Conversation

@fatmcgav
Copy link
Copy Markdown
Contributor

@fatmcgav fatmcgav commented Apr 9, 2026

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

Summary by CodeRabbit

  • New Features

    • GitLab merge request support for the automated code review agent alongside GitHub
    • Separate GitHub and GitLab agents with independent configuration and a new Agents page (shown when configured)
    • Review agent can select a configured language model at runtime
  • Chores

    • Added GitLab client dependency and updated changelog and ignore rules

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` |
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Apr 9, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 157c8035-a0f6-46d4-bab0-1a438c73acb8

📥 Commits

Reviewing files that changed from the base of the PR and between de3bd15 and ec2dc25.

📒 Files selected for processing (1)
  • CHANGELOG.md
✅ Files skipped from review due to trivial changes (1)
  • CHANGELOG.md

Walkthrough

This 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

Cohort / File(s) Summary
Configuration & Dependencies
/.gitignore, packages/web/package.json, packages/shared/src/env.server.ts
Ignore .claude; add @gitbeaker/rest dependency; add GITLAB_REVIEW_AGENT_WEBHOOK_SECRET, GITLAB_REVIEW_AGENT_TOKEN, GITLAB_REVIEW_AGENT_HOST, and REVIEW_AGENT_MODEL env entries.
Navigation & UI
packages/web/src/app/(app)/components/navigationMenu/index.tsx, packages/web/src/app/(app)/components/navigationMenu/navigationItems.tsx, packages/web/src/app/(app)/agents/page.tsx
Expose isAgentsVisible based on GitHub/GitLab config; add conditional “Agents” nav item and split agents list into github-review-agent and gitlab-review-agent with configureUrl.
Webhook Handler
packages/web/src/app/api/(server)/webhook/route.ts
Add GitLab event handling: init GitLab client, validate x-gitlab-token, detect MR and note events, and route to processGitLabMergeRequest/MR note handling.
Core Review Agent Logic
packages/web/src/features/agents/review-agent/app.ts
Remove OpenAI API key early-exit in GitHub flow; add exported processGitLabMergeRequest mirroring GitHub processing.
GitLab Parser & Pusher
packages/web/src/features/agents/review-agent/nodes/gitlabMrParser.ts, packages/web/src/features/agents/review-agent/nodes/gitlabPushMrReviews.ts
Add gitlabMrParser to fetch/parse MR diffs and gitlabPushMrReviews to post inline discussions with fallback notes.
LLM Model Selection
packages/web/src/features/agents/review-agent/nodes/invokeDiffReviewLlm.ts
Replace direct OpenAI calls with AI SDK generateText, select model from configured LM metadata, and remove hard OpenAI key guard.
File Source API
packages/web/src/features/git/getFileSourceApi.ts
Add getFileSourceForRepo (org/prisma passed directly); make getFileSource an auth-aware wrapper delegating to the new function.
Type Definitions
packages/web/src/features/agents/review-agent/types.ts
Add sourcebot_diff_refs schema/type; extend sourcebot_pr_payload with optional diff_refs; add GitLabMergeRequestPayload and GitLabNotePayload interfaces.
File Content Fetch
packages/web/src/features/agents/review-agent/nodes/fetchFileContent.ts
Switch to getFileSourceForRepo and add org lookup via Prisma; throw if org not found.
Tests
packages/web/src/features/agents/review-agent/nodes/*.{test.ts,ts}
Add comprehensive tests for GitHub/GitLab parsers and push-review behaviors (parsing, multi-file, inline/fallback posting, error recovery).
Changelog
CHANGELOG.md
Document “Support Gitlab MRs in the AI Code Review Agent” under Unreleased.

Sequence Diagram

sequenceDiagram
    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" }
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

  • feat(auth): github app #570: Touches the GitHub review-agent integration and related env/config changes (strong overlap with agent/webhook changes).

Suggested labels

sourcebot-team

Suggested reviewers

  • brendan-kellam
  • msukkari
🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 10.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'feat(web): GitLab MR Review Agent' accurately summarizes the main change in the PR: adding GitLab Merge Request support to the web package's Review Agent.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@fatmcgav
Copy link
Copy Markdown
Contributor Author

fatmcgav commented Apr 9, 2026

I've tested this locally against a Gitlab repo using AWS Bedrock, and got a successful review comment:
image

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 6

🧹 Nitpick comments (7)
packages/web/src/features/git/getFileSourceApi.ts (1)

10-10: Prefer import type for purely type-level imports.

Org and PrismaClient are 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 agents is statically two entries, the agents.length === 1 conditionals 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.

fetchFileContent is called repeatedly per diff, so this org lookup is repeated unnecessarily. Consider resolving org once 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) and processGitLabMergeRequest (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 wrapping processGitLabMergeRequest in try-catch to prevent retry storms.

If processGitLabMergeRequest throws, 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 system while 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

📥 Commits

Reviewing files that changed from the base of the PR and between 251f5b5 and de3bd15.

⛔ Files ignored due to path filters (1)
  • yarn.lock is excluded by !**/yarn.lock, !**/*.lock
📒 Files selected for processing (18)
  • .gitignore
  • packages/shared/src/env.server.ts
  • packages/web/package.json
  • packages/web/src/app/(app)/agents/page.tsx
  • packages/web/src/app/(app)/components/navigationMenu/index.tsx
  • packages/web/src/app/(app)/components/navigationMenu/navigationItems.tsx
  • packages/web/src/app/api/(server)/webhook/route.ts
  • packages/web/src/features/agents/review-agent/app.ts
  • packages/web/src/features/agents/review-agent/nodes/fetchFileContent.ts
  • packages/web/src/features/agents/review-agent/nodes/githubPrParser.test.ts
  • packages/web/src/features/agents/review-agent/nodes/githubPushPrReviews.test.ts
  • packages/web/src/features/agents/review-agent/nodes/gitlabMrParser.test.ts
  • packages/web/src/features/agents/review-agent/nodes/gitlabMrParser.ts
  • packages/web/src/features/agents/review-agent/nodes/gitlabPushMrReviews.test.ts
  • packages/web/src/features/agents/review-agent/nodes/gitlabPushMrReviews.ts
  • packages/web/src/features/agents/review-agent/nodes/invokeDiffReviewLlm.ts
  • packages/web/src/features/agents/review-agent/types.ts
  • packages/web/src/features/git/getFileSourceApi.ts

Comment on lines +198 to +202
// 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(),
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

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.

Suggested change
// 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.

Comment on lines 21 to +27
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 });
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

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.

Suggested change
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.

Comment on lines +133 to +139
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();
});
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 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.

Suggested change
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.

Comment on lines +99 to +101
head_sha: mr.sha,
diff_refs: mr.diff_refs as { base_sha: string; head_sha: string; start_sha: string },
};
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

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.

Comment on lines +29 to +38
{
position: {
positionType: "text",
baseSha: base_sha,
headSha: head_sha,
startSha: start_sha,
newPath: fileDiffReview.filename,
newLine: String(review.line_end),
},
},
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 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] and position[new_path] are required for text positions.
  • Use only new_line for added lines, only old_line for 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.

Comment on lines +59 to +74
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;
};
};
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

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.

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.

1 participant