Actionable comments posted: 6

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.

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.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: a2462b8d-0146-426e-8d4c-f6797bd49a90

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

• yarn.lock is excluded by !**/yarn.lock, !**/*.lock

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

Actionable comments posted: 2

packages/web/src/features/agents/review-agent/types.ts (1)

69-73: ⚠️ Potential issue | 🟡 Minor

Make diff_refs nullable/optional in both GitLab webhook interfaces.

packages/web/src/features/agents/review-agent/nodes/gitlabMrParser.ts already documents that merge request hooks can omit or null diff_refs on some actions, but these interfaces still require it. That keeps downstream code falsely narrowed and masks the null-handling path the GitLab flow already depends on.

Suggested type fix

 export interface GitLabMergeRequestPayload {
     object_kind: string;
     object_attributes: {
         iid: number;
         title: string;
         description: string | null;
         action: string;
         last_commit: {
             id: string;
         };
-        diff_refs: {
+        diff_refs?: {
             base_sha: string;
             head_sha: string;
             start_sha: string;
-        };
+        } | null;
     };
     project: {
         id: number;
         name: string;
         path_with_namespace: string;
@@
 export interface GitLabNotePayload {
     object_kind: string;
     object_attributes: {
         note: string;
         noteable_type: string;
     };
     merge_request: {
         iid: number;
         title: string;
         description: string | null;
         last_commit: {
             id: string;
         };
-        diff_refs: {
+        diff_refs?: {
             base_sha: string;
             head_sha: string;
             start_sha: string;
-        };
+        } | null;
     };
     project: {
         id: number;
         name: string;
         path_with_namespace: string;

Also applies to: 97-101

🤖 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 69 - 73,
The diff_refs property in the GitLab webhook interfaces is currently required
but can be omitted/null for some MR actions; update both occurrences of
diff_refs in this file to be nullable/optional (e.g., change from diff_refs: {
base_sha: string; head_sha: string; start_sha: string; } to diff_refs?: {
base_sha: string; head_sha: string; start_sha: string; } | null) so downstream
code can handle the null/undefined path; ensure you update both interface
occurrences referenced in the comment (the MR webhook interface and the other
GitLab webhook interface at the second occurrence) and run type checks to
confirm no remaining narrow assumptions.

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@packages/web/src/app/api/`(server)/webhook/route.ts:
- Around line 99-117: The GitLab webhook branch uses body typed as unknown and
relies on isGitLabMergeRequestEvent / isGitLabNoteEvent guards but still
dereferences body.project and body.merge_request (e.g., body.project.id,
body.merge_request.iid/title/description/last_commit/diff_refs) without
validating their shapes; add Zod schemas (e.g., GitLabProjectSchema,
GitLabMergeRequestSchema, GitLabNoteSchema or a combined GitLabWebhookSchema)
and in the route handler parse await request.json(), call schema.safeParse(); if
parse fails return requestBodySchemaValidationError, otherwise use the parsed
value for downstream logic instead of assuming fields exist. Ensure you
reference and replace unchecked accesses in the branches that used
isGitLabMergeRequestEvent / isGitLabNoteEvent to use the validated parsed
object.

In `@packages/web/src/features/agents/review-agent/nodes/gitlabMrParser.ts`:
- Around line 33-34: The repo lookup key is derived incorrectly from
mrPayload.project.name; change the repo variable (repoName) to use the last
segment of mrPayload.project.path_with_namespace instead (e.g., const repoName =
mrPayload.project.path_with_namespace.split('/').pop() or equivalent) so that
the repo used when calling fetchFileContent() matches the stored repo.name;
update any use of namespace/repoName pair (namespace and repoName) accordingly.

---

Duplicate comments:
In `@packages/web/src/features/agents/review-agent/types.ts`:
- Around line 69-73: The diff_refs property in the GitLab webhook interfaces is
currently required but can be omitted/null for some MR actions; update both
occurrences of diff_refs in this file to be nullable/optional (e.g., change from
diff_refs: { base_sha: string; head_sha: string; start_sha: string; } to
diff_refs?: { base_sha: string; head_sha: string; start_sha: string; } | null)
so downstream code can handle the null/undefined path; ensure you update both
interface occurrences referenced in the comment (the MR webhook interface and
the other GitLab webhook interface at the second occurrence) and run type checks
to confirm no remaining narrow assumptions.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 2460ae46-0310-47d2-adc3-95f59f241b30

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

• yarn.lock is excluded by !**/yarn.lock, !**/*.lock

• .gitignore
• CHANGELOG.md
• 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

• .gitignore
• packages/web/package.json
• CHANGELOG.md
• packages/web/src/features/agents/review-agent/nodes/githubPushPrReviews.test.ts

• packages/shared/src/env.server.ts
• packages/web/src/app/(app)/components/navigationMenu/index.tsx
• packages/web/src/app/(app)/agents/page.tsx
• packages/web/src/features/agents/review-agent/nodes/invokeDiffReviewLlm.ts
• packages/web/src/features/agents/review-agent/nodes/githubPrParser.test.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/git/getFileSourceApi.ts
• packages/web/src/app/(app)/components/navigationMenu/navigationItems.tsx

packages/web/src/app/api/(server)/webhook/route.ts (1)

249-260: Consider adding explicit type annotation to synthetic MR payload.

The inline mrPayload object is structurally correct, but adding an explicit type annotation would provide compile-time verification that it matches GitLabMergeRequestPayload and improve code clarity.

♻️ Add type annotation

+            import { GitLabMergeRequestPayload } from "@/features/agents/review-agent/types";
             // ... in the handler:
-                const mrPayload = {
+                const mrPayload: GitLabMergeRequestPayload = {
                     object_kind: "merge_request",
                     object_attributes: {

🤖 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 249 - 260,
The inline mrPayload object lacks an explicit type annotation; annotate it as
GitLabMergeRequestPayload to get compile-time checking and clarity. Update the
declaration of mrPayload (the object built from parsed.data.merge_request and
parsed.data.project) to include the type GitLabMergeRequestPayload, ensure the
fields you set (object_kind, object_attributes, project) conform to that type,
and adjust any property names or casts if TypeScript reports mismatches in the
function handling the webhook in route.ts.

packages/web/src/features/agents/review-agent/nodes/gitlabMrParser.ts (1)

37-49: Minor style note: type assertions before validation.

Lines 38-39 cast to string before the null check on line 41. While functionally safe (falsy check catches undefined), a cleaner pattern would be to access the properties without casting and let the null check handle type narrowing.

♻️ Optional cleaner pattern

     const sourcebotFileDiffs: (sourcebot_file_diff | null)[] = fileDiffs.map((fileDiff) => {
-        const fromPath = fileDiff.old_path as string;
-        const toPath = fileDiff.new_path as string;
+        const fromPath = fileDiff.old_path;
+        const toPath = fileDiff.new_path;

         if (!fromPath || !toPath) {
             logger.debug(`Skipping file due to missing old_path (${fromPath}) or new_path (${toPath})`);
             return null;
         }

🤖 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 37 - 49, The current code assigns fromPath and toPath using type
assertions before checking for null/undefined; change the pattern in the
fileDiffs mapping (the sourcebotFileDiffs creation) to read fileDiff.old_path
and fileDiff.new_path without "as string" first, perform the
null/undefined/falsy check (if (!fromPath || !toPath) ...) to let TypeScript
narrow the types, and only cast or treat them as strings after the check (or
simply rely on the narrowed type) so the null check is meaningful and avoids
premature assertions in the sourcebotFileDiffs / fileDiffs mapping logic.

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@packages/web/src/app/api/`(server)/webhook/route.ts:
- Around line 249-260: The inline mrPayload object lacks an explicit type
annotation; annotate it as GitLabMergeRequestPayload to get compile-time
checking and clarity. Update the declaration of mrPayload (the object built from
parsed.data.merge_request and parsed.data.project) to include the type
GitLabMergeRequestPayload, ensure the fields you set (object_kind,
object_attributes, project) conform to that type, and adjust any property names
or casts if TypeScript reports mismatches in the function handling the webhook
in route.ts.

In `@packages/web/src/features/agents/review-agent/nodes/gitlabMrParser.ts`:
- Around line 37-49: The current code assigns fromPath and toPath using type
assertions before checking for null/undefined; change the pattern in the
fileDiffs mapping (the sourcebotFileDiffs creation) to read fileDiff.old_path
and fileDiff.new_path without "as string" first, perform the
null/undefined/falsy check (if (!fromPath || !toPath) ...) to let TypeScript
narrow the types, and only cast or treat them as strings after the check (or
simply rely on the narrowed type) so the null check is meaningful and avoids
premature assertions in the sourcebotFileDiffs / fileDiffs mapping logic.

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: e1fa14f8-a173-41ae-ac05-734028458bd7

Reviewing files that changed from the base of the PR and between 6c006a0 and cd9fda5.

• packages/web/src/app/api/(server)/webhook/route.ts
• packages/web/src/features/agents/review-agent/nodes/gitlabMrParser.ts
• packages/web/src/features/agents/review-agent/types.ts

• packages/web/src/features/agents/review-agent/types.ts

Actionable comments posted: 1

packages/web/src/features/agents/review-agent/types.ts (1)

73-101: Tighten the webhook discriminators.

object_kind and noteable_type are plain z.string(), so unrelated GitLab payloads can still satisfy these schemas if the rest of the shape matches. Using literals here would reject unsupported hooks earlier and make downstream narrowing safer.

🔎 Suggested schema tightening

 export const gitLabMergeRequestPayloadSchema = z.object({
-    object_kind: z.string(),
+    object_kind: z.literal("merge_request"),
     object_attributes: z.object({
         iid: z.number(),
         title: z.string(),
         description: z.string().nullable(),
         action: z.string(),
         last_commit: z.object({ id: z.string() }),
         diff_refs: gitLabDiffRefsSchema,
     }),
     project: gitLabProjectSchema,
 });
 
 export const gitLabNotePayloadSchema = z.object({
-    object_kind: z.string(),
+    object_kind: z.literal("note"),
     object_attributes: z.object({
         note: z.string(),
-        noteable_type: z.string(),
+        noteable_type: z.literal("MergeRequest"),
     }),

🤖 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 73 -
101, The schemas gitLabMergeRequestPayloadSchema and gitLabNotePayloadSchema use
broad z.string() for discriminators allowing unrelated webhooks to pass; update
the object_kind fields to z.literal('merge_request') in both
gitLabMergeRequestPayloadSchema and gitLabNotePayloadSchema, and change
noteable_type in gitLabNotePayloadSchema to the appropriate literal (e.g.,
z.literal('MergeRequest') or the exact GitLab value) so the schemas reject
non-merge-request hooks and make downstream type narrowing safe.

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@packages/web/src/features/agents/review-agent/nodes/gitlabMrParser.ts`:
- Line 100: The head_sha assignment currently uses mr.sha ?? "" which can
discard a valid webhook commit; change the fallback in the head_sha field to
prefer the webhook value by using mr.sha ??
mrPayload.object_attributes.last_commit.id ?? "" (update the head_sha
construction inside gitlabMrParser.ts where head_sha is set) so the MR flow
retains the webhook-provided commit id when MergeRequests.show() lacks sha.

---

Nitpick comments:
In `@packages/web/src/features/agents/review-agent/types.ts`:
- Around line 73-101: The schemas gitLabMergeRequestPayloadSchema and
gitLabNotePayloadSchema use broad z.string() for discriminators allowing
unrelated webhooks to pass; update the object_kind fields to
z.literal('merge_request') in both gitLabMergeRequestPayloadSchema and
gitLabNotePayloadSchema, and change noteable_type in gitLabNotePayloadSchema to
the appropriate literal (e.g., z.literal('MergeRequest') or the exact GitLab
value) so the schemas reject non-merge-request hooks and make downstream type
narrowing safe.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 2f200f46-48a5-40e2-8f26-35d18cb99089

Reviewing files that changed from the base of the PR and between 6c006a0 and ca8ace9.

• packages/shared/src/env.server.ts
• packages/web/src/app/api/(server)/webhook/route.ts
• packages/web/src/features/agents/review-agent/nodes/gitlabMrParser.ts
• packages/web/src/features/agents/review-agent/types.ts

• packages/shared/src/env.server.ts

• packages/web/src/app/api/(server)/webhook/route.ts

packages/shared/src/env.server.ts (1)

201-201: Allow host:port in GITLAB_REVIEW_AGENT_HOST validation.

Current validation rejects : so values like gitlab.example.com:8443 fail, even though downstream URL assembly supports them. Consider widening validation to accept optional ports.

Suggested adjustment

-        GITLAB_REVIEW_AGENT_HOST: z.string().default('gitlab.com').transform(s => s.replace(/^https?:\/\//, '').replace(/\/+$/, '')).refine(s => /^[a-z0-9.-]+$/i.test(s), { message: 'invalid hostname' }),
+        GITLAB_REVIEW_AGENT_HOST: z.string()
+            .default('gitlab.com')
+            .transform(s => s.trim().replace(/^https?:\/\//i, '').replace(/\/+$/, ''))
+            .refine(
+                s => /^(?:[a-z0-9](?:[a-z0-9-]{0,61}[a-z0-9])?\.)*[a-z0-9](?:[a-z0-9-]{0,61}[a-z0-9])?(?::\d{1,5})?$/i.test(s),
+                { message: 'invalid host[:port]' }
+            ),

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/shared/src/env.server.ts` at line 201, The GITLAB_REVIEW_AGENT_HOST
zod schema currently rejects host:port because the refine regex disallows ':';
update the validation on GITLAB_REVIEW_AGENT_HOST (the .transform(...) and
.refine(...) chain) to allow an optional port by changing the regex to accept an
optional colon followed by digits (e.g. adjust the refine pattern from
/^[a-z0-9.-]+$/i to something like /^[a-z0-9.-]+(?::\d+)?$/i), keep the existing
transform that strips http(s):// and trailing slashes, and keep or update the
refine error message to indicate "invalid hostname or port" if you prefer.

packages/web/src/features/agents/review-agent/app.ts (1)

60-72: Remove the extra project-id source of truth.

mrPayload.project.id is already in scope and gitlabMrParser() uses that value. Keeping a separate projectId parameter makes it possible to parse one project and post comments to another if a caller ever passes mismatched values.

♻️ Suggested simplification

 export async function processGitLabMergeRequest(
     gitlabClient: InstanceType<typeof Gitlab>,
-    projectId: number,
     mrPayload: GitLabMergeRequestPayload,
     hostDomain: string,
 ) {
@@
-    await gitlabPushMrReviews(gitlabClient, projectId, prPayload, fileDiffReviews);
+    await gitlabPushMrReviews(gitlabClient, mrPayload.project.id, prPayload, fileDiffReviews);
 }

🤖 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 60 - 72,
The function processGitLabMergeRequest currently accepts a separate projectId
parameter which duplicates mrPayload.project.id and risks mismatched usage;
remove the projectId parameter from processGitLabMergeRequest's signature and
update the body to derive the project id from mrPayload.project.id (e.g., use
mrPayload.project.id when calling gitlabPushMrReviews), then update all call
sites to stop passing projectId so the single source of truth is mrPayload; also
ensure gitlabMrParser and any other usages still receive the correct values.

packages/web/src/features/agents/review-agent/nodes/gitlabMrParser.test.ts (1)

57-251: Add regression coverage for the new fallback branches in gitlabMrParser().

The suite never exercises mr.sha ?? last_commit.id or mr.diff_refs == null, so the exact GitLab edge cases this PR hardened can regress unnoticed.

🧪 Suggested tests

+    test('falls back to webhook last_commit.id when show() omits sha', async () => {
+        const client = makeMockGitlabClient([], { sha: undefined });
+        const result = await gitlabMrParser(client, MOCK_MR_PAYLOAD, 'gitlab.com');
+        expect(result.head_sha).toBe('abc123def456');
+    });
+
+    test('omits diff_refs when show() returns null diff_refs', async () => {
+        const client = makeMockGitlabClient([], { diff_refs: null });
+        const result = await gitlabMrParser(client, MOCK_MR_PAYLOAD, 'gitlab.com');
+        expect(result.diff_refs).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/gitlabMrParser.test.ts`
around lines 57 - 251, The tests need to cover the new fallback branches in
gitlabMrParser: add unit cases that (1) pass an MR payload where mr.sha is null
and last_commit.id is present and assert the resulting head_sha equals
last_commit.id, and (2) pass an MR payload where mr.diff_refs is null (or
missing) and assert diff_refs on the result are derived via the parser's
fallback logic; locate the test cases in the gitlabMrParser.test.ts suite and
use the existing makeMockGitlabClient / MOCK_MR_PAYLOAD patterns, invoking
gitlabMrParser(client, payload, host) and asserting on result.head_sha and
result.diff_refs to ensure the fallbacks (mr.sha ?? last_commit.id and handling
of mr.diff_refs == null) are exercised.

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@packages/shared/src/env.server.ts`:
- Line 201: The GITLAB_REVIEW_AGENT_HOST zod schema currently rejects host:port
because the refine regex disallows ':'; update the validation on
GITLAB_REVIEW_AGENT_HOST (the .transform(...) and .refine(...) chain) to allow
an optional port by changing the regex to accept an optional colon followed by
digits (e.g. adjust the refine pattern from /^[a-z0-9.-]+$/i to something like
/^[a-z0-9.-]+(?::\d+)?$/i), keep the existing transform that strips http(s)://
and trailing slashes, and keep or update the refine error message to indicate
"invalid hostname or port" if you prefer.

In `@packages/web/src/features/agents/review-agent/app.ts`:
- Around line 60-72: The function processGitLabMergeRequest currently accepts a
separate projectId parameter which duplicates mrPayload.project.id and risks
mismatched usage; remove the projectId parameter from
processGitLabMergeRequest's signature and update the body to derive the project
id from mrPayload.project.id (e.g., use mrPayload.project.id when calling
gitlabPushMrReviews), then update all call sites to stop passing projectId so
the single source of truth is mrPayload; also ensure gitlabMrParser and any
other usages still receive the correct values.

In `@packages/web/src/features/agents/review-agent/nodes/gitlabMrParser.test.ts`:
- Around line 57-251: The tests need to cover the new fallback branches in
gitlabMrParser: add unit cases that (1) pass an MR payload where mr.sha is null
and last_commit.id is present and assert the resulting head_sha equals
last_commit.id, and (2) pass an MR payload where mr.diff_refs is null (or
missing) and assert diff_refs on the result are derived via the parser's
fallback logic; locate the test cases in the gitlabMrParser.test.ts suite and
use the existing makeMockGitlabClient / MOCK_MR_PAYLOAD patterns, invoking
gitlabMrParser(client, payload, host) and asserting on result.head_sha and
result.diff_refs to ensure the fallbacks (mr.sha ?? last_commit.id and handling
of mr.diff_refs == null) are exercised.

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: c140fde0-e600-49bc-ad81-f06bab25ab74

Reviewing files that changed from the base of the PR and between ca8ace9 and a824d7a.

• yarn.lock is excluded by !**/yarn.lock, !**/*.lock

• .gitignore
• CHANGELOG.md
• docs/docs/features/agents/review-agent.mdx
• 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

• packages/web/package.json
• .gitignore
• CHANGELOG.md
• packages/web/src/features/agents/review-agent/nodes/gitlabPushMrReviews.test.ts
• packages/web/src/features/agents/review-agent/nodes/githubPushPrReviews.test.ts

• 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
• packages/web/src/features/agents/review-agent/nodes/githubPrParser.test.ts
• packages/web/src/features/agents/review-agent/nodes/invokeDiffReviewLlm.ts
• docs/docs/features/agents/review-agent.mdx
• packages/web/src/features/agents/review-agent/nodes/gitlabPushMrReviews.ts
• packages/web/src/app/api/(server)/webhook/route.ts

LGTM thanks!