Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 29 additions & 0 deletions apps/web/src/components/code-reviews/ReviewConfigForm.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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<string>('');
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
Expand Down Expand Up @@ -422,6 +424,7 @@ export function ReviewConfigForm({
)
);
setUseReviewMd(!(configData.disableReviewMd ?? false));
setSkipBotPullRequests(configData.skipBotPullRequests ?? true);
}
}, [configData, isGitLab]);

Expand Down Expand Up @@ -683,6 +686,7 @@ export function ReviewConfigForm({
council: councilPayload,
councilEnabledRepositoryIds: councilEnabledRepositoryIdsPayload,
disableReviewMd: !useReviewMd,
skipBotPullRequests,
// GitLab-specific: auto-configure webhooks
autoConfigureWebhooks: isGitLab ? autoConfigureWebhooks : undefined,
});
Expand All @@ -700,6 +704,7 @@ export function ReviewConfigForm({
manuallyAddedRepositories,
repositoryModelOverrides: repositoryModelOverridesPayload,
disableReviewMd: !useReviewMd,
skipBotPullRequests,
// GitLab-specific: auto-configure webhooks
autoConfigureWebhooks: isGitLab ? autoConfigureWebhooks : undefined,
});
Expand Down Expand Up @@ -885,6 +890,30 @@ export function ReviewConfigForm({
/>
</div>

{/* 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 && (
<div className="flex items-center justify-between rounded-lg border p-4">
<div className="space-y-0.5">
<Label htmlFor="skip-bot-prs" className="text-base font-semibold">
Skip pull requests from bots
</Label>
<p className="text-muted-foreground text-sm">
Do not run automated reviews on {prLabel} opened by bot accounts (for example
Dependabot or Renovate). Turn this off to review bot {prLabel} too.
</p>
</div>
<Switch
id="skip-bot-prs"
checked={skipBotPullRequests}
onCheckedChange={setSkipBotPullRequests}
disabled={
orgSaveMutation.isPending || personalSaveMutation.isPending || !isEnabled
}
/>
</div>
)}

{/* Focus Areas (global) */}
<div className="space-y-3">
<Label>Focus Areas</Label>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
},
};

Expand Down Expand Up @@ -447,6 +450,98 @@ 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('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({
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({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,14 @@ export async function handlePullRequestCodeReview(

// 3. Check if repository is in allowed list (when using selected repositories mode)
const config = agentConfig.config as CodeReviewAgentConfig;

// 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;
const isBotPullRequestSkip = skipBotPullRequests && pull_request.user.type === 'Bot';

if (
config?.repository_selection_mode === 'selected' &&
Array.isArray(config?.selected_repository_ids)
Expand Down Expand Up @@ -176,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({
Expand Down Expand Up @@ -288,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);

Expand Down
4 changes: 4 additions & 0 deletions apps/web/src/routers/code-reviews-router.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down Expand Up @@ -216,6 +217,7 @@ export const personalReviewAgentRouter = createTRPCRouter({
council: null,
councilEnabledRepositoryIds: [],
disableReviewMd: true,
skipBotPullRequests: true,
reviewMemoryEnabled: false,
actionRequired: null,
};
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -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).
Expand Down Expand Up @@ -499,6 +501,7 @@ export const organizationReviewAgentRouter = createTRPCRouter({
council: null,
councilEnabledRepositoryIds: [],
disableReviewMd: true,
skipBotPullRequests: true,
reviewMemoryEnabled: false,
actionRequired: null,
};
Expand Down Expand Up @@ -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),
Expand Down Expand Up @@ -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,
},
Expand Down
7 changes: 7 additions & 0 deletions packages/db/src/schema-types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down