From 5464ed2518fce944801faf2412d98ac9dbb6bdbc Mon Sep 17 00:00:00 2001 From: Gavin Williams Date: Tue, 5 May 2026 23:01:38 +0100 Subject: [PATCH 1/6] feat(web): add optional PR/MR summary comment to review agent Adds REVIEW_AGENT_SUMMARY_ENABLED (default false) and REVIEW_AGENT_SUMMARY_MAX_LENGTH (default 250) env vars. When enabled, the review agent generates a concise markdown summary of the PR/MR changes and posts it as a top-level comment before pushing inline review comments. Co-Authored-By: Claude Sonnet 4.6 --- .devcontainer/docker-compose.yml | 1 + .env.development | 1 + .../configuration/environment-variables.mdx | 2 + docs/docs/features/agents/review-agent.mdx | 2 + packages/shared/src/env.server.ts | 2 + .../src/features/agents/review-agent/app.ts | 25 ++++++++- .../review-agent/nodes/generatePrSummary.ts | 51 +++++++++++++++++++ .../review-agent/nodes/githubPushPrReviews.ts | 15 +++++- .../review-agent/nodes/gitlabPushMrReviews.ts | 13 +++++ 9 files changed, 109 insertions(+), 3 deletions(-) create mode 100644 packages/web/src/features/agents/review-agent/nodes/generatePrSummary.ts diff --git a/.devcontainer/docker-compose.yml b/.devcontainer/docker-compose.yml index 96a9e7ff1..a82b5a2a6 100644 --- a/.devcontainer/docker-compose.yml +++ b/.devcontainer/docker-compose.yml @@ -27,6 +27,7 @@ services: REVIEW_AGENT_LOGGING_ENABLED: "true" REVIEW_AGENT_AUTO_REVIEW_ENABLED: "false" REVIEW_AGENT_REVIEW_COMMAND: review + REVIEW_AGENT_SUMMARY_ENABLED: "false" depends_on: - postgres - redis diff --git a/.env.development b/.env.development index e9029f539..d0ca5ef2c 100644 --- a/.env.development +++ b/.env.development @@ -49,6 +49,7 @@ REDIS_URL="redis://localhost:6379" REVIEW_AGENT_LOGGING_ENABLED=true REVIEW_AGENT_AUTO_REVIEW_ENABLED=false REVIEW_AGENT_REVIEW_COMMAND=review +REVIEW_AGENT_SUMMARY_ENABLED=false # Misc diff --git a/docs/docs/configuration/environment-variables.mdx b/docs/docs/configuration/environment-variables.mdx index 8c0415fc4..e3b6a52bc 100644 --- a/docs/docs/configuration/environment-variables.mdx +++ b/docs/docs/configuration/environment-variables.mdx @@ -72,6 +72,8 @@ The following environment variables allow you to configure your Sourcebot deploy | `REVIEW_AGENT_AUTO_REVIEW_ENABLED` | `false` |

Enables/disables automatic code reviews by the review agent.

| | `REVIEW_AGENT_LOGGING_ENABLED` | `true` |

Enables/disables logging for the review agent. Logs are saved in `DATA_CACHE_DIR/review-agent`

| | `REVIEW_AGENT_REVIEW_COMMAND` | `review` |

The command used to trigger a code review by the review agent.

| +| `REVIEW_AGENT_SUMMARY_ENABLED` | `false` |

Enables/disables posting a concise summary comment on the PR/MR after each review.

| +| `REVIEW_AGENT_SUMMARY_MAX_LENGTH` | `250` |

Maximum character length of the summary comment posted by the review agent.

| ### Overriding environment variables from the config diff --git a/docs/docs/features/agents/review-agent.mdx b/docs/docs/features/agents/review-agent.mdx index b4dc9c2f6..dc775d462 100644 --- a/docs/docs/features/agents/review-agent.mdx +++ b/docs/docs/features/agents/review-agent.mdx @@ -145,3 +145,5 @@ You can also trigger a review manually by commenting `/review` on any PR or MR. | `REVIEW_AGENT_REVIEW_COMMAND` | `review` | Comment command that triggers a manual review (without the `/`) | | `REVIEW_AGENT_MODEL` | first configured model | `displayName` of the language model to use for reviews | | `REVIEW_AGENT_LOGGING_ENABLED` | unset | Write prompt and response logs to disk for debugging | +| `REVIEW_AGENT_SUMMARY_ENABLED` | `false` | Post a concise summary comment on the PR/MR after each review | +| `REVIEW_AGENT_SUMMARY_MAX_LENGTH` | `250` | Maximum character length of the summary comment | diff --git a/packages/shared/src/env.server.ts b/packages/shared/src/env.server.ts index 712ac9d41..e140c5a02 100644 --- a/packages/shared/src/env.server.ts +++ b/packages/shared/src/env.server.ts @@ -245,6 +245,8 @@ const options = { REVIEW_AGENT_LOGGING_ENABLED: booleanSchema.default('true'), REVIEW_AGENT_AUTO_REVIEW_ENABLED: booleanSchema.default('false'), REVIEW_AGENT_REVIEW_COMMAND: z.string().default('review'), + REVIEW_AGENT_SUMMARY_ENABLED: booleanSchema.default('false'), + REVIEW_AGENT_SUMMARY_MAX_LENGTH: numberSchema.default(250), ANTHROPIC_API_KEY: z.string().optional(), ANTHROPIC_AUTH_TOKEN: z.string().optional(), diff --git a/packages/web/src/features/agents/review-agent/app.ts b/packages/web/src/features/agents/review-agent/app.ts index 043a80fad..da67bd968 100644 --- a/packages/web/src/features/agents/review-agent/app.ts +++ b/packages/web/src/features/agents/review-agent/app.ts @@ -1,6 +1,7 @@ import { Octokit } from "octokit"; import { Gitlab } from "@gitbeaker/rest"; import { generatePrReviews } from "@/features/agents/review-agent/nodes/generatePrReview"; +import { generatePrSummary } from "@/features/agents/review-agent/nodes/generatePrSummary"; import { githubPushPrReviews } from "@/features/agents/review-agent/nodes/githubPushPrReviews"; import { githubPrParser } from "@/features/agents/review-agent/nodes/githubPrParser"; import { getReviewAgentLogDir } from "@/features/agents/review-agent/nodes/invokeDiffReviewLlm"; @@ -55,7 +56,17 @@ export async function processGitHubPullRequest(octokit: Octokit, pullRequest: Gi const prPayload = await githubPrParser(octokit, pullRequest); const fileDiffReviews = await generatePrReviews(reviewAgentLogPath, prPayload, rules); - await githubPushPrReviews(octokit, prPayload, fileDiffReviews); + + let summary: string | undefined; + if (env.REVIEW_AGENT_SUMMARY_ENABLED) { + try { + summary = await generatePrSummary(prPayload); + } catch (error) { + logger.error(`Error generating PR summary: ${error}`); + } + } + + await githubPushPrReviews(octokit, prPayload, fileDiffReviews, summary); } export async function processGitLabMergeRequest( @@ -70,5 +81,15 @@ export async function processGitLabMergeRequest( const prPayload = await gitlabMrParser(gitlabClient, mrPayload, hostDomain); const fileDiffReviews = await generatePrReviews(reviewAgentLogPath, prPayload, rules); - await gitlabPushMrReviews(gitlabClient, projectId, prPayload, fileDiffReviews); + + let summary: string | undefined; + if (env.REVIEW_AGENT_SUMMARY_ENABLED) { + try { + summary = await generatePrSummary(prPayload); + } catch (error) { + logger.error(`Error generating MR summary: ${error}`); + } + } + + await gitlabPushMrReviews(gitlabClient, projectId, prPayload, fileDiffReviews, summary); } diff --git a/packages/web/src/features/agents/review-agent/nodes/generatePrSummary.ts b/packages/web/src/features/agents/review-agent/nodes/generatePrSummary.ts new file mode 100644 index 000000000..21ba69ff5 --- /dev/null +++ b/packages/web/src/features/agents/review-agent/nodes/generatePrSummary.ts @@ -0,0 +1,51 @@ +import { sourcebot_pr_payload } from "@/features/agents/review-agent/types"; +import { getAISDKLanguageModelAndOptions, getConfiguredLanguageModels } from "@/features/chat/utils.server"; +import { env } from "@sourcebot/shared"; +import { generateText } from "ai"; +import { createLogger } from "@sourcebot/shared"; + +const logger = createLogger('generate-pr-summary'); + +export const generatePrSummary = async (prPayload: sourcebot_pr_payload): Promise => { + const maxSummaryLength = env.REVIEW_AGENT_SUMMARY_MAX_LENGTH; + logger.debug("Executing generate_pr_summary"); + + const models = await getConfiguredLanguageModels(); + if (models.length === 0) { + throw new Error("No language models are configured"); + } + + let selectedModel = models[0]; + if (env.REVIEW_AGENT_MODEL) { + const match = models.find((m) => m.displayName === env.REVIEW_AGENT_MODEL); + if (match) { + selectedModel = match; + } else { + logger.warn(`REVIEW_AGENT_MODEL="${env.REVIEW_AGENT_MODEL}" did not match any configured model displayName. Falling back to the first configured model.`); + } + } + + const { model, providerOptions, temperature } = await getAISDKLanguageModelAndOptions(selectedModel); + + const filesChanged = prPayload.file_diffs.map(f => f.to).join(", "); + + const prompt = `Summarize the following pull request changes in ${maxSummaryLength} characters or fewer. Be concise and focus on what changed and why. You may use inline markdown (e.g. \`code\`, **bold**) but avoid headers, bullet lists, and block-level formatting. + +PR Title: ${prPayload.title} +PR Description: ${prPayload.description} +Files changed: ${filesChanged} +`; + + const result = await generateText({ + model, + system: `You are a code review assistant. Generate a concise markdown-compatible summary of pull request changes. The summary must be ${maxSummaryLength} characters or fewer. Avoid headers, bullet lists, and block-level formatting.`, + prompt, + providerOptions, + temperature, + }); + + const summary = result.text.trim().slice(0, maxSummaryLength); + + logger.debug("Completed generate_pr_summary"); + return summary; +}; diff --git a/packages/web/src/features/agents/review-agent/nodes/githubPushPrReviews.ts b/packages/web/src/features/agents/review-agent/nodes/githubPushPrReviews.ts index a6ef88d2b..9450a6743 100644 --- a/packages/web/src/features/agents/review-agent/nodes/githubPushPrReviews.ts +++ b/packages/web/src/features/agents/review-agent/nodes/githubPushPrReviews.ts @@ -4,9 +4,22 @@ import { createLogger } from "@sourcebot/shared"; const logger = createLogger('github-push-pr-reviews'); -export const githubPushPrReviews = async (octokit: Octokit, pr_payload: sourcebot_pr_payload, file_diff_reviews: sourcebot_file_diff_review[]) => { +export const githubPushPrReviews = async (octokit: Octokit, pr_payload: sourcebot_pr_payload, file_diff_reviews: sourcebot_file_diff_review[], summary?: string) => { logger.info("Executing github_push_pr_reviews"); + if (summary) { + try { + await octokit.rest.issues.createComment({ + owner: pr_payload.owner, + repo: pr_payload.repo, + issue_number: pr_payload.number, + body: summary, + }); + } catch (error) { + logger.error(`Error posting PR summary comment: ${error}`); + } + } + try { for (const file_diff_review of file_diff_reviews) { for (const review of file_diff_review.reviews) { diff --git a/packages/web/src/features/agents/review-agent/nodes/gitlabPushMrReviews.ts b/packages/web/src/features/agents/review-agent/nodes/gitlabPushMrReviews.ts index ff98ecb3c..454cfb152 100644 --- a/packages/web/src/features/agents/review-agent/nodes/gitlabPushMrReviews.ts +++ b/packages/web/src/features/agents/review-agent/nodes/gitlabPushMrReviews.ts @@ -9,9 +9,22 @@ export const gitlabPushMrReviews = async ( projectId: number, prPayload: sourcebot_pr_payload, fileDiffReviews: sourcebot_file_diff_review[], + summary?: string, ): Promise => { logger.info("Executing gitlab_push_mr_reviews"); + if (summary) { + try { + await gitlabClient.MergeRequestNotes.create( + projectId, + prPayload.number, + summary, + ); + } catch (error) { + logger.error(`Error posting MR summary note: ${error}`); + } + } + if (!prPayload.diff_refs) { logger.error("diff_refs is missing from pr_payload, cannot post inline GitLab MR reviews"); return; From c30a12419a9b568adab320dc9ed9c71c56592662 Mon Sep 17 00:00:00 2001 From: Gavin Williams Date: Thu, 7 May 2026 09:05:57 +0100 Subject: [PATCH 2/6] chore: Add Changelog entry --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8ab886a30..40be407a6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added - [EE] Added prompt caching for Ask Sourcebot. For Anthropic models, the static prompt prefix (tool definitions, system prompt, and conversation history) is marked with a cache breakpoint so it is billed at the provider's discounted cache-read rate on subsequent agent steps and follow-up turns. Toggle with `SOURCEBOT_CHAT_PROMPT_CACHING_ENABLED` (default `true`). [#1278](https://github.com/sourcebot-dev/sourcebot/pull/1278) - [EE] Added a cached-token breakdown to the Ask Sourcebot message details, showing what share of the input tokens were served from the model provider's prompt cache. [#1278](https://github.com/sourcebot-dev/sourcebot/pull/1278) +- [Experimental] Added support for generating a summary comment to the AI Code review agent. [#1175](https://github.com/sourcebot-dev/sourcebot/pull/1175) ### Fixed - Upgraded `protobufjs` to `^7.6.2`. [#1281](https://github.com/sourcebot-dev/sourcebot/pull/1281) From 96f375f162255c57a4a3f2290dbafec8d9d6f247 Mon Sep 17 00:00:00 2001 From: Gavin Williams Date: Tue, 5 May 2026 23:19:01 +0100 Subject: [PATCH 3/6] refactor(web): extract duplicated summary generation into generateSummarySafely helper Co-Authored-By: Claude Sonnet 4.6 --- .../src/features/agents/review-agent/app.ts | 34 ++++++++----------- 1 file changed, 15 insertions(+), 19 deletions(-) diff --git a/packages/web/src/features/agents/review-agent/app.ts b/packages/web/src/features/agents/review-agent/app.ts index da67bd968..1279fb7b3 100644 --- a/packages/web/src/features/agents/review-agent/app.ts +++ b/packages/web/src/features/agents/review-agent/app.ts @@ -7,7 +7,7 @@ import { githubPrParser } from "@/features/agents/review-agent/nodes/githubPrPar import { getReviewAgentLogDir } from "@/features/agents/review-agent/nodes/invokeDiffReviewLlm"; import { gitlabMrParser } from "@/features/agents/review-agent/nodes/gitlabMrParser"; import { gitlabPushMrReviews } from "@/features/agents/review-agent/nodes/gitlabPushMrReviews"; -import { GitHubPullRequest, GitLabMergeRequestPayload } from "@/features/agents/review-agent/types"; +import { GitHubPullRequest, GitLabMergeRequestPayload, sourcebot_pr_payload } from "@/features/agents/review-agent/types"; import { env } from "@sourcebot/shared"; import path from "path"; import fs from "fs"; @@ -25,6 +25,18 @@ const rules = [ const logger = createLogger('review-agent'); +async function generateSummarySafely(payload: sourcebot_pr_payload, label: string): Promise { + if (!env.REVIEW_AGENT_SUMMARY_ENABLED) { + return undefined; + } + try { + return await generatePrSummary(payload); + } catch (error) { + logger.error(`Error generating ${label} summary: ${error}`); + return undefined; + } +} + function getReviewAgentLogPath(identifier: string): string | undefined { if (!env.REVIEW_AGENT_LOGGING_ENABLED) { return undefined; @@ -57,15 +69,7 @@ export async function processGitHubPullRequest(octokit: Octokit, pullRequest: Gi const prPayload = await githubPrParser(octokit, pullRequest); const fileDiffReviews = await generatePrReviews(reviewAgentLogPath, prPayload, rules); - let summary: string | undefined; - if (env.REVIEW_AGENT_SUMMARY_ENABLED) { - try { - summary = await generatePrSummary(prPayload); - } catch (error) { - logger.error(`Error generating PR summary: ${error}`); - } - } - + const summary = await generateSummarySafely(prPayload, "PR"); await githubPushPrReviews(octokit, prPayload, fileDiffReviews, summary); } @@ -82,14 +86,6 @@ export async function processGitLabMergeRequest( const prPayload = await gitlabMrParser(gitlabClient, mrPayload, hostDomain); const fileDiffReviews = await generatePrReviews(reviewAgentLogPath, prPayload, rules); - let summary: string | undefined; - if (env.REVIEW_AGENT_SUMMARY_ENABLED) { - try { - summary = await generatePrSummary(prPayload); - } catch (error) { - logger.error(`Error generating MR summary: ${error}`); - } - } - + const summary = await generateSummarySafely(prPayload, "MR"); await gitlabPushMrReviews(gitlabClient, projectId, prPayload, fileDiffReviews, summary); } From 9f9e1d7075aec16af125a7483b6836b5053308ef Mon Sep 17 00:00:00 2001 From: Gavin Williams Date: Tue, 5 May 2026 23:28:54 +0100 Subject: [PATCH 4/6] feat(web): make summary comments idempotent and add created/updated footer GitHub and GitLab summary posting now checks for an existing comment/note containing a hidden `` marker before writing. If found, the existing comment is updated in place (`updateComment` / `MergeRequestNotes.edit`); otherwise a new one is created, preventing duplicate summary comments on re-runs. A markdown footer is appended to every summary body showing "Created: " on first post and "Updated: " on subsequent runs, giving reviewers visibility into when the last review was performed. Tests: * mock helpers updated to include `issues.listComments/createComment/ updateComment`` and `MergeRequestNotes.all/edit`. * New test suites added for both GitHub and GitLab handlers covering: * no summary -> no API calls; * first post -> create with marker and "Created:" footer; * re-run -> update with "Updated:" footer and no duplicate create; * API failure -> no throw propagated. Co-Authored-By: Claude Sonnet 4.6 --- .../nodes/githubPushPrReviews.test.ts | 66 ++++++++++++++++++- .../review-agent/nodes/githubPushPrReviews.ts | 22 ++++++- .../nodes/gitlabPushMrReviews.test.ts | 60 ++++++++++++++++- .../review-agent/nodes/gitlabPushMrReviews.ts | 24 +++++-- 4 files changed, 161 insertions(+), 11 deletions(-) diff --git a/packages/web/src/features/agents/review-agent/nodes/githubPushPrReviews.test.ts b/packages/web/src/features/agents/review-agent/nodes/githubPushPrReviews.test.ts index 210fffb44..78faab332 100644 --- a/packages/web/src/features/agents/review-agent/nodes/githubPushPrReviews.test.ts +++ b/packages/web/src/features/agents/review-agent/nodes/githubPushPrReviews.test.ts @@ -30,7 +30,7 @@ const SINGLE_REVIEW: sourcebot_file_diff_review[] = [ }, ]; -function makeMockOctokit(createReviewCommentResult: 'resolve' | 'reject' = 'resolve') { +function makeMockOctokit(createReviewCommentResult: 'resolve' | 'reject' = 'resolve', existingComments: { id: number; body: string }[] = []) { return { rest: { pulls: { @@ -38,6 +38,11 @@ function makeMockOctokit(createReviewCommentResult: 'resolve' | 'reject' = 'reso ? vi.fn().mockResolvedValue({}) : vi.fn().mockRejectedValue(new Error('Unprocessable Entity')), }, + issues: { + listComments: vi.fn().mockResolvedValue({ data: existingComments }), + createComment: vi.fn().mockResolvedValue({}), + updateComment: vi.fn().mockResolvedValue({}), + }, }, } as unknown as Octokit; } @@ -147,3 +152,62 @@ describe('githubPushPrReviews', () => { expect(octokit.rest.pulls.createReviewComment).not.toHaveBeenCalled(); }); }); + +describe('githubPushPrReviews – summary comment', () => { + const SUMMARY_MARKER = ''; + + test('does not call issues API when summary is undefined', async () => { + const octokit = makeMockOctokit(); + + await githubPushPrReviews(octokit, MOCK_PAYLOAD, [], undefined); + + expect(octokit.rest.issues.listComments).not.toHaveBeenCalled(); + expect(octokit.rest.issues.createComment).not.toHaveBeenCalled(); + expect(octokit.rest.issues.updateComment).not.toHaveBeenCalled(); + }); + + test('creates a new comment including the marker when no existing comment found', async () => { + const octokit = makeMockOctokit('resolve', []); + + await githubPushPrReviews(octokit, MOCK_PAYLOAD, [], 'Summary text'); + + expect(octokit.rest.issues.listComments).toHaveBeenCalledWith({ + owner: 'my-org', + repo: 'my-repo', + issue_number: 7, + }); + expect(octokit.rest.issues.createComment).toHaveBeenCalledOnce(); + const body = octokit.rest.issues.createComment.mock.calls[0][0].body as string; + expect(body).toContain(SUMMARY_MARKER); + expect(body).toContain('Summary text'); + expect(body).toContain('Created:'); + expect(body).not.toContain('Updated:'); + expect(octokit.rest.issues.updateComment).not.toHaveBeenCalled(); + }); + + test('updates the existing comment when the marker is already present', async () => { + const existingComments = [{ id: 99, body: `${SUMMARY_MARKER}\nOld summary` }]; + const octokit = makeMockOctokit('resolve', existingComments); + + await githubPushPrReviews(octokit, MOCK_PAYLOAD, [], 'New summary'); + + expect(octokit.rest.issues.updateComment).toHaveBeenCalledOnce(); + expect(octokit.rest.issues.updateComment).toHaveBeenCalledWith( + expect.objectContaining({ comment_id: 99 }), + ); + const body = octokit.rest.issues.updateComment.mock.calls[0][0].body as string; + expect(body).toContain('New summary'); + expect(body).toContain('Updated:'); + expect(body).not.toContain('Created:'); + expect(octokit.rest.issues.createComment).not.toHaveBeenCalled(); + }); + + test('does not throw when listComments fails', async () => { + const octokit = makeMockOctokit(); + octokit.rest.issues.listComments = vi.fn().mockRejectedValue(new Error('403 Forbidden')); + + await expect( + githubPushPrReviews(octokit, MOCK_PAYLOAD, [], 'Summary text'), + ).resolves.not.toThrow(); + }); +}); diff --git a/packages/web/src/features/agents/review-agent/nodes/githubPushPrReviews.ts b/packages/web/src/features/agents/review-agent/nodes/githubPushPrReviews.ts index 9450a6743..a83d58bca 100644 --- a/packages/web/src/features/agents/review-agent/nodes/githubPushPrReviews.ts +++ b/packages/web/src/features/agents/review-agent/nodes/githubPushPrReviews.ts @@ -8,13 +8,31 @@ export const githubPushPrReviews = async (octokit: Octokit, pr_payload: sourcebo logger.info("Executing github_push_pr_reviews"); if (summary) { + const SUMMARY_MARKER = ""; try { - await octokit.rest.issues.createComment({ + const { data: comments } = await octokit.rest.issues.listComments({ owner: pr_payload.owner, repo: pr_payload.repo, issue_number: pr_payload.number, - body: summary, }); + const existing = comments.find(c => c.body?.includes(SUMMARY_MARKER)); + const action = existing ? "Updated" : "Created"; + const body = `${SUMMARY_MARKER}\n${summary}\n\n---\n*${action}: ${new Date().toUTCString()}*`; + if (existing) { + await octokit.rest.issues.updateComment({ + owner: pr_payload.owner, + repo: pr_payload.repo, + comment_id: existing.id, + body, + }); + } else { + await octokit.rest.issues.createComment({ + owner: pr_payload.owner, + repo: pr_payload.repo, + issue_number: pr_payload.number, + body, + }); + } } catch (error) { logger.error(`Error posting PR summary comment: ${error}`); } diff --git a/packages/web/src/features/agents/review-agent/nodes/gitlabPushMrReviews.test.ts b/packages/web/src/features/agents/review-agent/nodes/gitlabPushMrReviews.test.ts index 98caad2d9..4aa17c6ff 100644 --- a/packages/web/src/features/agents/review-agent/nodes/gitlabPushMrReviews.test.ts +++ b/packages/web/src/features/agents/review-agent/nodes/gitlabPushMrReviews.test.ts @@ -37,7 +37,7 @@ const SINGLE_REVIEW: sourcebot_file_diff_review[] = [ }, ]; -function makeMockClient(discussionResult: 'resolve' | 'reject' = 'resolve') { +function makeMockClient(discussionResult: 'resolve' | 'reject' = 'resolve', existingNotes: { id: number; body: string }[] = []) { return { MergeRequestDiscussions: { create: discussionResult === 'resolve' @@ -45,7 +45,9 @@ function makeMockClient(discussionResult: 'resolve' | 'reject' = 'resolve') { : vi.fn().mockRejectedValue(new Error('400 Bad Request')), }, MergeRequestNotes: { + all: vi.fn().mockResolvedValue(existingNotes), create: vi.fn().mockResolvedValue({}), + edit: vi.fn().mockResolvedValue({}), }, } as unknown as GitlabClient; } @@ -180,11 +182,63 @@ describe('gitlabPushMrReviews', () => { test('does not throw when both discussion and note creation fail', async () => { const client = { MergeRequestDiscussions: { create: vi.fn().mockRejectedValue(new Error('500')) }, - MergeRequestNotes: { create: vi.fn().mockRejectedValue(new Error('500')) }, - } as unknown as GitlabClient; + MergeRequestNotes: { all: vi.fn().mockResolvedValue([]), create: vi.fn().mockRejectedValue(new Error('500')), edit: vi.fn() }, + } as any; await expect( gitlabPushMrReviews(client, 101, MOCK_PAYLOAD, SINGLE_REVIEW), ).resolves.not.toThrow(); }); }); + +describe('gitlabPushMrReviews – summary note', () => { + const SUMMARY_MARKER = ''; + + test('does not call MergeRequestNotes API when summary is undefined', async () => { + const client = makeMockClient(); + + await gitlabPushMrReviews(client, 101, MOCK_PAYLOAD, [], undefined); + + expect(client.MergeRequestNotes.all).not.toHaveBeenCalled(); + expect(client.MergeRequestNotes.create).not.toHaveBeenCalled(); + expect(client.MergeRequestNotes.edit).not.toHaveBeenCalled(); + }); + + test('creates a new note including the marker when no existing note found', async () => { + const client = makeMockClient('resolve', []); + + await gitlabPushMrReviews(client, 101, MOCK_PAYLOAD, [], 'Summary text'); + + expect(client.MergeRequestNotes.all).toHaveBeenCalledWith(101, 42); + expect(client.MergeRequestNotes.create).toHaveBeenCalledOnce(); + const body = client.MergeRequestNotes.create.mock.calls[0][2] as string; + expect(body).toContain(SUMMARY_MARKER); + expect(body).toContain('Summary text'); + expect(body).toContain('Created:'); + expect(body).not.toContain('Updated:'); + expect(client.MergeRequestNotes.edit).not.toHaveBeenCalled(); + }); + + test('updates the existing note when the marker is already present', async () => { + const existingNotes = [{ id: 55, body: `${SUMMARY_MARKER}\nOld summary` }]; + const client = makeMockClient('resolve', existingNotes); + + await gitlabPushMrReviews(client, 101, MOCK_PAYLOAD, [], 'New summary'); + + expect(client.MergeRequestNotes.edit).toHaveBeenCalledOnce(); + const editOptions = client.MergeRequestNotes.edit.mock.calls[0][3] as { body: string }; + expect(editOptions.body).toContain('New summary'); + expect(editOptions.body).toContain('Updated:'); + expect(editOptions.body).not.toContain('Created:'); + expect(client.MergeRequestNotes.create).not.toHaveBeenCalled(); + }); + + test('does not throw when MergeRequestNotes.all fails', async () => { + const client = makeMockClient(); + client.MergeRequestNotes.all = vi.fn().mockRejectedValue(new Error('403 Forbidden')); + + await expect( + gitlabPushMrReviews(client, 101, MOCK_PAYLOAD, [], 'Summary text'), + ).resolves.not.toThrow(); + }); +}); diff --git a/packages/web/src/features/agents/review-agent/nodes/gitlabPushMrReviews.ts b/packages/web/src/features/agents/review-agent/nodes/gitlabPushMrReviews.ts index 454cfb152..360bb8f4d 100644 --- a/packages/web/src/features/agents/review-agent/nodes/gitlabPushMrReviews.ts +++ b/packages/web/src/features/agents/review-agent/nodes/gitlabPushMrReviews.ts @@ -14,12 +14,26 @@ export const gitlabPushMrReviews = async ( logger.info("Executing gitlab_push_mr_reviews"); if (summary) { + const SUMMARY_MARKER = ""; try { - await gitlabClient.MergeRequestNotes.create( - projectId, - prPayload.number, - summary, - ); + const notes = await gitlabClient.MergeRequestNotes.all(projectId, prPayload.number); + const existing = notes.find(n => n.body?.includes(SUMMARY_MARKER)); + const action = existing ? "Updated" : "Created"; + const body = `${SUMMARY_MARKER}\n${summary}\n\n---\n*${action}: ${new Date().toUTCString()}*`; + if (existing) { + await gitlabClient.MergeRequestNotes.edit( + projectId, + prPayload.number, + existing.id, + { body }, + ); + } else { + await gitlabClient.MergeRequestNotes.create( + projectId, + prPayload.number, + body, + ); + } } catch (error) { logger.error(`Error posting MR summary note: ${error}`); } From 6f5934a064cec2ded429afa0abb41628ae1f79cc Mon Sep 17 00:00:00 2001 From: Gavin Williams Date: Thu, 7 May 2026 09:11:57 +0100 Subject: [PATCH 5/6] fix(web): paginate all PR comments when searching for existing summary Replaces the single-page octokit.rest.issues.listComments call with octokit.paginate(...) so all comment pages are searched before deciding whether to create or update the summary comment. Without this, the marker could be missed on PRs with many comments, causing duplicate summary comments on re-runs. Tests updated to mock octokit.paginate directly instead of rest.issues.listComments, and assertions updated accordingly. Co-Authored-By: Claude Sonnet 4.6 --- .../nodes/githubPushPrReviews.test.ts | 23 ++++++++++--------- .../review-agent/nodes/githubPushPrReviews.ts | 2 +- 2 files changed, 13 insertions(+), 12 deletions(-) diff --git a/packages/web/src/features/agents/review-agent/nodes/githubPushPrReviews.test.ts b/packages/web/src/features/agents/review-agent/nodes/githubPushPrReviews.test.ts index 78faab332..c6a2e75ec 100644 --- a/packages/web/src/features/agents/review-agent/nodes/githubPushPrReviews.test.ts +++ b/packages/web/src/features/agents/review-agent/nodes/githubPushPrReviews.test.ts @@ -31,7 +31,8 @@ const SINGLE_REVIEW: sourcebot_file_diff_review[] = [ ]; function makeMockOctokit(createReviewCommentResult: 'resolve' | 'reject' = 'resolve', existingComments: { id: number; body: string }[] = []) { - return { + const octokit = { + paginate: vi.fn().mockResolvedValue(existingComments), rest: { pulls: { createReviewComment: createReviewCommentResult === 'resolve' @@ -39,12 +40,13 @@ function makeMockOctokit(createReviewCommentResult: 'resolve' | 'reject' = 'reso : vi.fn().mockRejectedValue(new Error('Unprocessable Entity')), }, issues: { - listComments: vi.fn().mockResolvedValue({ data: existingComments }), + listComments: vi.fn(), createComment: vi.fn().mockResolvedValue({}), updateComment: vi.fn().mockResolvedValue({}), }, }, - } as unknown as Octokit; + } as any; + return octokit; } describe('githubPushPrReviews', () => { @@ -161,7 +163,7 @@ describe('githubPushPrReviews – summary comment', () => { await githubPushPrReviews(octokit, MOCK_PAYLOAD, [], undefined); - expect(octokit.rest.issues.listComments).not.toHaveBeenCalled(); + expect(octokit.paginate).not.toHaveBeenCalled(); expect(octokit.rest.issues.createComment).not.toHaveBeenCalled(); expect(octokit.rest.issues.updateComment).not.toHaveBeenCalled(); }); @@ -171,11 +173,10 @@ describe('githubPushPrReviews – summary comment', () => { await githubPushPrReviews(octokit, MOCK_PAYLOAD, [], 'Summary text'); - expect(octokit.rest.issues.listComments).toHaveBeenCalledWith({ - owner: 'my-org', - repo: 'my-repo', - issue_number: 7, - }); + expect(octokit.paginate).toHaveBeenCalledWith( + octokit.rest.issues.listComments, + { owner: 'my-org', repo: 'my-repo', issue_number: 7 }, + ); expect(octokit.rest.issues.createComment).toHaveBeenCalledOnce(); const body = octokit.rest.issues.createComment.mock.calls[0][0].body as string; expect(body).toContain(SUMMARY_MARKER); @@ -202,9 +203,9 @@ describe('githubPushPrReviews – summary comment', () => { expect(octokit.rest.issues.createComment).not.toHaveBeenCalled(); }); - test('does not throw when listComments fails', async () => { + test('does not throw when paginate fails', async () => { const octokit = makeMockOctokit(); - octokit.rest.issues.listComments = vi.fn().mockRejectedValue(new Error('403 Forbidden')); + octokit.paginate = vi.fn().mockRejectedValue(new Error('403 Forbidden')); await expect( githubPushPrReviews(octokit, MOCK_PAYLOAD, [], 'Summary text'), diff --git a/packages/web/src/features/agents/review-agent/nodes/githubPushPrReviews.ts b/packages/web/src/features/agents/review-agent/nodes/githubPushPrReviews.ts index a83d58bca..8e0c74762 100644 --- a/packages/web/src/features/agents/review-agent/nodes/githubPushPrReviews.ts +++ b/packages/web/src/features/agents/review-agent/nodes/githubPushPrReviews.ts @@ -10,7 +10,7 @@ export const githubPushPrReviews = async (octokit: Octokit, pr_payload: sourcebo if (summary) { const SUMMARY_MARKER = ""; try { - const { data: comments } = await octokit.rest.issues.listComments({ + const comments = await octokit.paginate(octokit.rest.issues.listComments, { owner: pr_payload.owner, repo: pr_payload.repo, issue_number: pr_payload.number, From 79184375adc430380441de5b59a96fa82e59f9a7 Mon Sep 17 00:00:00 2001 From: Gavin Williams Date: Fri, 5 Jun 2026 21:20:01 +0100 Subject: [PATCH 6/6] fix(web): replace `as any` with `as unknown as ` in review agent tests Co-Authored-By: Claude Sonnet 4.6 --- .../agents/review-agent/nodes/githubPushPrReviews.test.ts | 2 +- .../agents/review-agent/nodes/gitlabPushMrReviews.test.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/web/src/features/agents/review-agent/nodes/githubPushPrReviews.test.ts b/packages/web/src/features/agents/review-agent/nodes/githubPushPrReviews.test.ts index c6a2e75ec..db961a945 100644 --- a/packages/web/src/features/agents/review-agent/nodes/githubPushPrReviews.test.ts +++ b/packages/web/src/features/agents/review-agent/nodes/githubPushPrReviews.test.ts @@ -45,7 +45,7 @@ function makeMockOctokit(createReviewCommentResult: 'resolve' | 'reject' = 'reso updateComment: vi.fn().mockResolvedValue({}), }, }, - } as any; + } as unknown as Octokit; return octokit; } diff --git a/packages/web/src/features/agents/review-agent/nodes/gitlabPushMrReviews.test.ts b/packages/web/src/features/agents/review-agent/nodes/gitlabPushMrReviews.test.ts index 4aa17c6ff..03606bc0e 100644 --- a/packages/web/src/features/agents/review-agent/nodes/gitlabPushMrReviews.test.ts +++ b/packages/web/src/features/agents/review-agent/nodes/gitlabPushMrReviews.test.ts @@ -183,7 +183,7 @@ describe('gitlabPushMrReviews', () => { const client = { MergeRequestDiscussions: { create: vi.fn().mockRejectedValue(new Error('500')) }, MergeRequestNotes: { all: vi.fn().mockResolvedValue([]), create: vi.fn().mockRejectedValue(new Error('500')), edit: vi.fn() }, - } as any; + } as unknown as GitlabClient; await expect( gitlabPushMrReviews(client, 101, MOCK_PAYLOAD, SINGLE_REVIEW),