From 58de2701df0de8d7aa26e7543d996004e931310b Mon Sep 17 00:00:00 2001 From: St0rmz1 Date: Fri, 24 Jul 2026 09:59:26 -0700 Subject: [PATCH 1/2] feat(code-review): skip automated reviews of bot PRs by default (GitHub) --- .../code-reviews/ReviewConfigForm.tsx | 29 +++++++++++++ .../pull-request-handler.test.ts | 41 +++++++++++++++++++ .../webhook-handlers/pull-request-handler.ts | 16 ++++++++ apps/web/src/routers/code-reviews-router.ts | 4 ++ .../organization-code-reviews-router.test.ts | 41 +++++++++++++++++++ .../organization-code-reviews-router.ts | 5 +++ packages/db/src/schema-types.ts | 7 ++++ 7 files changed, 143 insertions(+) diff --git a/apps/web/src/components/code-reviews/ReviewConfigForm.tsx b/apps/web/src/components/code-reviews/ReviewConfigForm.tsx index 806e78a127..2dca22273f 100644 --- a/apps/web/src/components/code-reviews/ReviewConfigForm.tsx +++ b/apps/web/src/components/code-reviews/ReviewConfigForm.tsx @@ -275,6 +275,8 @@ export function ReviewConfigForm({ // Optional council label gate, edited as a comma-separated string; parsed to a list on save. const [councilRequiredLabelsInput, setCouncilRequiredLabelsInput] = useState(''); const [useReviewMd, setUseReviewMd] = useState(true); + // Feature-level guardrail; defaults to skipping bot PRs (matches the server default). + const [skipBotPullRequests, setSkipBotPullRequests] = useState(true); // GitLab-specific: auto-configure webhooks const [autoConfigureWebhooks, setAutoConfigureWebhooks] = useState(true); // Webhook sync result from last save @@ -422,6 +424,7 @@ export function ReviewConfigForm({ ) ); setUseReviewMd(!(configData.disableReviewMd ?? false)); + setSkipBotPullRequests(configData.skipBotPullRequests ?? true); } }, [configData, isGitLab]); @@ -683,6 +686,7 @@ export function ReviewConfigForm({ council: councilPayload, councilEnabledRepositoryIds: councilEnabledRepositoryIdsPayload, disableReviewMd: !useReviewMd, + skipBotPullRequests, // GitLab-specific: auto-configure webhooks autoConfigureWebhooks: isGitLab ? autoConfigureWebhooks : undefined, }); @@ -700,6 +704,7 @@ export function ReviewConfigForm({ manuallyAddedRepositories, repositoryModelOverrides: repositoryModelOverridesPayload, disableReviewMd: !useReviewMd, + skipBotPullRequests, // GitLab-specific: auto-configure webhooks autoConfigureWebhooks: isGitLab ? autoConfigureWebhooks : undefined, }); @@ -885,6 +890,30 @@ export function ReviewConfigForm({ /> + {/* GitHub only: bot detection relies on the GitHub-authoritative user.type, which + GitLab/Bitbucket webhooks do not provide, so the toggle is hidden there. */} + {!isGitLab && ( +
+
+ +

+ Do not run automated reviews on {prLabel} opened by bot accounts (for example + Dependabot or Renovate). Turn this off to review bot {prLabel} too. +

+
+ +
+ )} + {/* Focus Areas (global) */}
diff --git a/apps/web/src/lib/integrations/platforms/github/webhook-handlers/pull-request-handler.test.ts b/apps/web/src/lib/integrations/platforms/github/webhook-handlers/pull-request-handler.test.ts index 1022cca6f3..04fc234e95 100644 --- a/apps/web/src/lib/integrations/platforms/github/webhook-handlers/pull-request-handler.test.ts +++ b/apps/web/src/lib/integrations/platforms/github/webhook-handlers/pull-request-handler.test.ts @@ -321,6 +321,9 @@ describe('handlePullRequest', () => { }, // Repo 123 (acme/widgets, from pullRequestPayload) opted into council. council_enabled_repository_ids: [123], + // Review bot PRs so the council bot-exclusion test reaches the council decision; the + // feature-level bot skip (default on) would otherwise drop bot PRs before this point. + skip_bot_pull_requests: false, }, }; @@ -447,6 +450,44 @@ describe('handlePullRequest', () => { }); }); + describe('bot pull request guardrail', () => { + it('skips a bot-authored PR by default (skip_bot_pull_requests unset) and creates no review', async () => { + mockGetBotUserId.mockResolvedValue('bot-user-1'); + mockGetAgentConfigForOwner.mockResolvedValue({ is_enabled: true, config: {} }); + + const payload = pullRequestPayload(); + payload.pull_request.user.type = 'Bot'; + const response = await handlePullRequest(payload, platformIntegration()); + + expect(response.status).toBe(200); + expect(await response.json()).toEqual({ message: 'Skipped bot-authored PR' }); + expect(mockCreateCodeReview).not.toHaveBeenCalled(); + }); + + it('reviews a bot-authored PR when skip_bot_pull_requests is false', async () => { + mockGetBotUserId.mockResolvedValue('bot-user-1'); + mockGetAgentConfigForOwner.mockResolvedValue({ + is_enabled: true, + config: { skip_bot_pull_requests: false }, + }); + + const payload = pullRequestPayload(); + payload.pull_request.user.type = 'Bot'; + await handlePullRequest(payload, platformIntegration()); + + expect(mockCreateCodeReview).toHaveBeenCalled(); + }); + + it('reviews a non-bot PR while the guardrail is on', async () => { + mockGetBotUserId.mockResolvedValue('bot-user-1'); + mockGetAgentConfigForOwner.mockResolvedValue({ is_enabled: true, config: {} }); + + await handlePullRequest(pullRequestPayload(), platformIntegration()); + + expect(mockCreateCodeReview).toHaveBeenCalled(); + }); + }); + it('cancels superseded DB rows, interrupts queued/running only, and creates the new review', async () => { mockGetBotUserId.mockResolvedValue('bot-user-1'); mockGetAgentConfigForOwner.mockResolvedValue({ diff --git a/apps/web/src/lib/integrations/platforms/github/webhook-handlers/pull-request-handler.ts b/apps/web/src/lib/integrations/platforms/github/webhook-handlers/pull-request-handler.ts index dfd5ba5846..c6a9e8141f 100644 --- a/apps/web/src/lib/integrations/platforms/github/webhook-handlers/pull-request-handler.ts +++ b/apps/web/src/lib/integrations/platforms/github/webhook-handlers/pull-request-handler.ts @@ -140,6 +140,22 @@ export async function handlePullRequestCodeReview( // 3. Check if repository is in allowed list (when using selected repositories mode) const config = agentConfig.config as CodeReviewAgentConfig; + + // Feature-level guardrail: by default, skip automated reviews of bot-authored PRs + // (dependabot/renovate/etc.) — high-volume, low-value dependency bumps otherwise consume review + // compute and clutter the PR. Configurable per org via `skip_bot_pull_requests` (defaults to + // skipping when unset). Bot type is GitHub-authoritative (`user.type`), so it cannot be spoofed. + // Applies to standard and council reviews; manual reviews never reach this handler. + const skipBotPullRequests = config.skip_bot_pull_requests ?? true; + if (skipBotPullRequests && pull_request.user.type === 'Bot') { + logExceptInTest('Skipping bot-authored PR:', { + pr_number: pull_request.number, + repo: repository.full_name, + author: pull_request.user.login, + }); + return NextResponse.json({ message: 'Skipped bot-authored PR' }, { status: 200 }); + } + if ( config?.repository_selection_mode === 'selected' && Array.isArray(config?.selected_repository_ids) diff --git a/apps/web/src/routers/code-reviews-router.ts b/apps/web/src/routers/code-reviews-router.ts index b1c2fbbeed..8c5a48ca4d 100644 --- a/apps/web/src/routers/code-reviews-router.ts +++ b/apps/web/src/routers/code-reviews-router.ts @@ -102,6 +102,7 @@ const SaveReviewConfigInputSchema = z.object({ .superRefine(rejectDuplicateRepositoryModelOverrides) .optional(), disableReviewMd: z.boolean().optional(), + skipBotPullRequests: z.boolean().optional(), gateThreshold: z.enum(['off', 'all', 'warning', 'critical']).optional(), // GitLab-specific: auto-configure webhooks autoConfigureWebhooks: z.boolean().optional().default(true), @@ -216,6 +217,7 @@ export const personalReviewAgentRouter = createTRPCRouter({ council: null, councilEnabledRepositoryIds: [], disableReviewMd: true, + skipBotPullRequests: true, reviewMemoryEnabled: false, actionRequired: null, }; @@ -247,6 +249,7 @@ export const personalReviewAgentRouter = createTRPCRouter({ thinkingEffort: override.thinking_effort ?? null, })), disableReviewMd: cfg.disable_review_md ?? true, + skipBotPullRequests: cfg.skip_bot_pull_requests ?? true, // Council is org-only; personal configs never set it, but expose it so the query shape // matches the org query (the form's `configData` is a union of the two). council: cfg.council ?? null, @@ -302,6 +305,7 @@ export const personalReviewAgentRouter = createTRPCRouter({ manually_added_repositories: input.manuallyAddedRepositories || [], repository_model_overrides: repositoryModelOverrides, disable_review_md: input.disableReviewMd ?? true, + skip_bot_pull_requests: input.skipBotPullRequests ?? true, review_memory_enabled: false, review_analytics_enabled: false, }, diff --git a/apps/web/src/routers/organizations/organization-code-reviews-router.test.ts b/apps/web/src/routers/organizations/organization-code-reviews-router.test.ts index 66cff55c14..ae911c2afd 100644 --- a/apps/web/src/routers/organizations/organization-code-reviews-router.test.ts +++ b/apps/web/src/routers/organizations/organization-code-reviews-router.test.ts @@ -199,3 +199,44 @@ describe('organization review agent router: council config', () => { expect(cfg.council?.required_labels).toEqual(['council', 'needs-deep-review']); }); }); + +describe('organization review agent router: skip bot pull requests', () => { + afterAll(async () => { + for (const organizationId of createdOrganizationIds) { + await db + .delete(organization_audit_logs) + .where(eq(organization_audit_logs.organization_id, organizationId)); + await db + .delete(agent_configs) + .where(eq(agent_configs.owned_by_organization_id, organizationId)); + await db.delete(organizations).where(eq(organizations.id, organizationId)); + } + }); + + it('defaults skipBotPullRequests to true and round-trips an explicit false', async () => { + const { owner, organization } = await createFixtureOrganization(); + const caller = await createCallerForUser(owner.id); + + // Default (no config saved) is to skip bot PRs. + const defaults = await caller.organizations.reviewAgent.getReviewConfig({ + organizationId: organization.id, + platform: 'github', + }); + expect(defaults.skipBotPullRequests).toBe(true); + + // Saving false persists and reloads as false. + await caller.organizations.reviewAgent.saveReviewConfig({ + organizationId: organization.id, + platform: 'github', + reviewStyle: 'balanced', + focusAreas: [], + modelSlug: 'anthropic/claude-sonnet-5', + skipBotPullRequests: false, + }); + const cfg = await caller.organizations.reviewAgent.getReviewConfig({ + organizationId: organization.id, + platform: 'github', + }); + expect(cfg.skipBotPullRequests).toBe(false); + }); +}); diff --git a/apps/web/src/routers/organizations/organization-code-reviews-router.ts b/apps/web/src/routers/organizations/organization-code-reviews-router.ts index f9cf5a19d7..e76ddd5c84 100644 --- a/apps/web/src/routers/organizations/organization-code-reviews-router.ts +++ b/apps/web/src/routers/organizations/organization-code-reviews-router.ts @@ -133,6 +133,8 @@ const SaveReviewConfigInputSchema = OrganizationIdInputSchema.extend({ .superRefine(rejectDuplicateRepositoryModelOverrides) .optional(), disableReviewMd: z.boolean().optional(), + // Feature-level guardrail: skip automated reviews of bot-authored PRs. Defaults to true. + skipBotPullRequests: z.boolean().optional(), gateThreshold: z.enum(['off', 'all', 'warning', 'critical']).optional(), // Org-level council config (specialists + governance), shared by every council-enabled repo. // `null`/absent leaves council unset. Persisted only for entitled orgs (checked in the handler). @@ -499,6 +501,7 @@ export const organizationReviewAgentRouter = createTRPCRouter({ council: null, councilEnabledRepositoryIds: [], disableReviewMd: true, + skipBotPullRequests: true, reviewMemoryEnabled: false, actionRequired: null, }; @@ -539,6 +542,7 @@ export const organizationReviewAgentRouter = createTRPCRouter({ thinkingEffort: override.thinking_effort ?? null, })), disableReviewMd: isBitbucket ? true : (cfg.disable_review_md ?? true), + skipBotPullRequests: cfg.skip_bot_pull_requests ?? true, council: cfg.council ?? null, councilEnabledRepositoryIds: cfg.council_enabled_repository_ids ?? [], reviewMemoryEnabled: isBitbucket ? false : getReviewMemoryEnabledFromConfig(config.config), @@ -632,6 +636,7 @@ export const organizationReviewAgentRouter = createTRPCRouter({ council, council_enabled_repository_ids: input.councilEnabledRepositoryIds ?? [], disable_review_md: isBitbucket ? true : (input.disableReviewMd ?? true), + skip_bot_pull_requests: input.skipBotPullRequests ?? true, review_memory_enabled: false, review_analytics_enabled: false, }, diff --git a/packages/db/src/schema-types.ts b/packages/db/src/schema-types.ts index 245e8a3fea..8d69211957 100644 --- a/packages/db/src/schema-types.ts +++ b/packages/db/src/schema-types.ts @@ -1512,6 +1512,13 @@ export const CodeReviewAgentConfigSchema = z.object({ // council-entitled and `council` is configured + active; the automated trigger re-checks both. // Matched against the platform repository ID the same way as `selected_repository_ids`. council_enabled_repository_ids: z.array(z.union([z.number(), z.string()])).optional(), + // Feature-level guardrail: when true (the default when absent), skip automated (webhook) code + // reviews for bot-authored pull requests (dependabot/renovate/etc.). Applies to standard and + // council reviews alike; manual reviews are unaffected. Set false to review bot PRs. + // Enforced on GitHub, where the bot signal (`user.type`) is in the webhook payload. GitLab and + // Bitbucket do not expose an authoritative bot flag in their webhooks, so the setting has no + // effect there yet. + skip_bot_pull_requests: z.boolean().optional(), disable_review_md: z.boolean().optional(), // Controls when the PR gate check (GitHub Check Run / GitLab commit status) // reports a failure based on review findings. From 77ade1f4b0af6f6d7e8cca9b9bd61d634cfca404 Mon Sep 17 00:00:00 2001 From: St0rmz1 Date: Fri, 24 Jul 2026 11:26:21 -0700 Subject: [PATCH 2/2] fix(code-review): defer merge-commit path to the bot-skip guardrail --- .../pull-request-handler.test.ts | 54 +++++++++++++++++++ .../webhook-handlers/pull-request-handler.ts | 34 +++++++----- 2 files changed, 75 insertions(+), 13 deletions(-) diff --git a/apps/web/src/lib/integrations/platforms/github/webhook-handlers/pull-request-handler.test.ts b/apps/web/src/lib/integrations/platforms/github/webhook-handlers/pull-request-handler.test.ts index 04fc234e95..05257ee1f6 100644 --- a/apps/web/src/lib/integrations/platforms/github/webhook-handlers/pull-request-handler.test.ts +++ b/apps/web/src/lib/integrations/platforms/github/webhook-handlers/pull-request-handler.test.ts @@ -464,6 +464,60 @@ describe('handlePullRequest', () => { expect(mockCreateCodeReview).not.toHaveBeenCalled(); }); + it('still cancels a superseded review and resolves its stale check run before skipping a bot PR', async () => { + mockGetBotUserId.mockResolvedValue('bot-user-1'); + mockGetAgentConfigForOwner.mockResolvedValue({ is_enabled: true, config: {} }); + // A prior in-flight review (from before the skip was enabled) superseded by this bot push. + mockCancelSupersededReviewsForPR.mockResolvedValue([ + { + id: 'queued-review', + prevStatus: 'queued', + sessionId: 'session-queued', + latestActiveAttemptId: 'queued-attempt', + checkRunId: 555, + headSha: 'old-sha', + platform: 'github', + platformProjectId: null, + platformIntegrationId: 'integration-1', + }, + ]); + + const payload = pullRequestPayload(); + payload.pull_request.user.type = 'Bot'; + const response = await handlePullRequest(payload, platformIntegration()); + + // The bot PR is still skipped (no new review)... + expect(response.status).toBe(200); + expect(mockCreateCodeReview).not.toHaveBeenCalled(); + // ...but the stale review is cancelled and its check run resolved, so the PR isn't left stuck. + expect(mockCancelSupersededReviewsForPR).toHaveBeenCalled(); + expect(mockUpdateCheckRun).toHaveBeenCalledWith( + '98765', + 'acme', + 'widgets', + 555, + expect.objectContaining({ status: 'completed', conclusion: 'cancelled' }), + 'standard' + ); + }); + + it('routes a bot PR with a merge-commit head through skip, not merge-commit migration', async () => { + mockGetBotUserId.mockResolvedValue('bot-user-1'); + mockGetAgentConfigForOwner.mockResolvedValue({ is_enabled: true, config: {} }); + // A non-bot PR here would hit the merge-commit path (step 4) and return 'Skipped merge commit'. + mockIsMergeCommit.mockResolvedValue(true); + + const payload = pullRequestPayload(); + payload.pull_request.user.type = 'Bot'; + const response = await handlePullRequest(payload, platformIntegration()); + + // The bot-skip decision defers step 4, so the PR is skipped as a bot PR (not re-pointed to a + // new SHA with a fresh check run by migrateInFlightReviewsToMergeCommitHead). + expect(response.status).toBe(200); + expect(await response.json()).toEqual({ message: 'Skipped bot-authored PR' }); + expect(mockCreateCodeReview).not.toHaveBeenCalled(); + }); + it('reviews a bot-authored PR when skip_bot_pull_requests is false', async () => { mockGetBotUserId.mockResolvedValue('bot-user-1'); mockGetAgentConfigForOwner.mockResolvedValue({ diff --git a/apps/web/src/lib/integrations/platforms/github/webhook-handlers/pull-request-handler.ts b/apps/web/src/lib/integrations/platforms/github/webhook-handlers/pull-request-handler.ts index c6a9e8141f..79b964c541 100644 --- a/apps/web/src/lib/integrations/platforms/github/webhook-handlers/pull-request-handler.ts +++ b/apps/web/src/lib/integrations/platforms/github/webhook-handlers/pull-request-handler.ts @@ -141,20 +141,12 @@ export async function handlePullRequestCodeReview( // 3. Check if repository is in allowed list (when using selected repositories mode) const config = agentConfig.config as CodeReviewAgentConfig; - // Feature-level guardrail: by default, skip automated reviews of bot-authored PRs - // (dependabot/renovate/etc.) — high-volume, low-value dependency bumps otherwise consume review - // compute and clutter the PR. Configurable per org via `skip_bot_pull_requests` (defaults to - // skipping when unset). Bot type is GitHub-authoritative (`user.type`), so it cannot be spoofed. - // Applies to standard and council reviews; manual reviews never reach this handler. + // Bot PRs are skipped by default (enforced at step 5b). Compute the decision up front so the + // merge-commit path (step 4) also defers to it: otherwise a bot PR whose head is a merge commit + // would be re-pointed to a new SHA with a fresh check run, keeping alive a review the guardrail + // is meant to skip. When true, the PR falls through to cancellation + skip instead. const skipBotPullRequests = config.skip_bot_pull_requests ?? true; - if (skipBotPullRequests && pull_request.user.type === 'Bot') { - logExceptInTest('Skipping bot-authored PR:', { - pr_number: pull_request.number, - repo: repository.full_name, - author: pull_request.user.login, - }); - return NextResponse.json({ message: 'Skipped bot-authored PR' }, { status: 200 }); - } + const isBotPullRequestSkip = skipBotPullRequests && pull_request.user.type === 'Bot'; if ( config?.repository_selection_mode === 'selected' && @@ -192,6 +184,7 @@ export async function handlePullRequestCodeReview( // Runs before cancellation so that an in-flight review at an earlier SHA is preserved: // a merge commit introduces no new feature work and should not supersede the existing review. if ( + !isBotPullRequestSkip && headOwner && headRepoName && (await shouldSkipSynchronizeForMergeCommit({ @@ -304,6 +297,21 @@ export async function handlePullRequestCodeReview( ); } + // 5b. Feature-level guardrail: by default, skip automated reviews of bot-authored PRs + // (dependabot/renovate/etc.) — high-volume, low-value dependency bumps otherwise consume review + // compute and clutter the PR. Configurable per org via `skip_bot_pull_requests` (see the + // decision computed before step 4). Applies to standard and council reviews; manual reviews + // never reach this handler. Runs AFTER supersession (step 5) so a bot push still cancels any + // stale in-flight review and resolves its check run, instead of leaving it stuck. + if (isBotPullRequestSkip) { + logExceptInTest('Skipping bot-authored PR:', { + pr_number: pull_request.number, + repo: repository.full_name, + author: pull_request.user.login, + }); + return NextResponse.json({ message: 'Skipped bot-authored PR' }, { status: 200 }); + } + // 6. Check for duplicate review (same repo, PR, SHA) const existingReview = await findExistingReview(reviewScope, pull_request.head.sha);