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
49 changes: 22 additions & 27 deletions apps/web/src/components/code-reviews/ReviewConfigForm.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -205,8 +205,6 @@ export function ReviewConfigForm({
const [selectedModel, setSelectedModel] = useState(PRIMARY_DEFAULT_MODEL);
const [thinkingEffort, setThinkingEffort] = useState<string | null>(null);
const [gateThreshold, setGateThreshold] = useState<'off' | 'all' | 'warning' | 'critical'>('off');
const isCloudAgentNextEnabled = configData?.isCloudAgentNextEnabled ?? false;
const isPrGateEnabled = configData?.isPrGateEnabled ?? false;
const [repositorySelectionMode, setRepositorySelectionMode] = useState<'all' | 'selected'>('all');
const [selectedRepositoryIds, setSelectedRepositoryIds] = useState<number[]>([]);
// Repositories added from search results (for GitLab where pagination limits initial results)
Expand Down Expand Up @@ -551,8 +549,8 @@ export function ReviewConfigForm({
helperText="Choose the AI model to use for code reviews"
/>

{/* Thinking Effort — only shown when cloud-agent-next is available and the model supports variants */}
{availableVariants.length > 0 && isCloudAgentNextEnabled && (
{/* Thinking Effort — only shown when the model supports variants */}
{availableVariants.length > 0 && (
<div className="space-y-2">
<Label>Thinking Effort</Label>
<Select
Expand All @@ -577,29 +575,26 @@ export function ReviewConfigForm({
</div>
)}

{/* PR Gate Threshold — only shown when PR gate feature flag is enabled */}
{isPrGateEnabled && (
<div className="space-y-2">
<Label>PR Gate Threshold</Label>
<Select
value={gateThreshold}
onValueChange={v => setGateThreshold(v as 'off' | 'all' | 'warning' | 'critical')}
>
<SelectTrigger className="w-full">
<SelectValue />
</SelectTrigger>
<SelectContent>
<SelectItem value="off">Off</SelectItem>
<SelectItem value="all">All findings</SelectItem>
<SelectItem value="warning">Warnings and above</SelectItem>
<SelectItem value="critical">Critical issues only</SelectItem>
</SelectContent>
</Select>
<p className="text-muted-foreground text-sm">
Controls when the PR status check reports a failure based on review findings
</p>
</div>
)}
<div className="space-y-2">
<Label>PR Gate Threshold</Label>
<Select
value={gateThreshold}
onValueChange={v => setGateThreshold(v as 'off' | 'all' | 'warning' | 'critical')}
>
<SelectTrigger className="w-full">
<SelectValue />
</SelectTrigger>
<SelectContent>
<SelectItem value="off">Off</SelectItem>
<SelectItem value="all">All findings</SelectItem>
<SelectItem value="warning">Warnings and above</SelectItem>
<SelectItem value="critical">Critical issues only</SelectItem>
</SelectContent>
</Select>
<p className="text-muted-foreground text-sm">
Controls when the PR status check reports a failure based on review findings
</p>
</div>

{/* Review Style */}
<div className="space-y-3">
Expand Down
7 changes: 0 additions & 7 deletions apps/web/src/lib/code-reviews/core/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,6 @@ export const DEFAULT_CODE_REVIEW_MODEL = 'anthropic/claude-sonnet-4.6';
*/
export const DEFAULT_CODE_REVIEW_MODE = 'code' as const;

// ============================================================================
// Feature Flags
// ============================================================================

/** PostHog flag that gates incremental (diff-only) reviews on follow-up pushes */
export const FEATURE_FLAG_INCREMENTAL_REVIEW = 'code-review-incremental';

// ============================================================================
// Pagination
// ============================================================================
Expand Down
12 changes: 12 additions & 0 deletions apps/web/src/lib/code-reviews/db/code-reviews.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -129,4 +129,16 @@ describe('findPreviousCompletedReview', () => {

expect(review?.terminalReason).toBe('billing');
});

it('creates new reviews with agent_version set to v2', async () => {
const id = await createReview('sha-v2-default');

const [review] = await db
.select({ agentVersion: cloud_agent_code_reviews.agent_version })
.from(cloud_agent_code_reviews)
.where(eq(cloud_agent_code_reviews.id, id))
.limit(1);

expect(review?.agentVersion).toBe('v2');
});
});
1 change: 1 addition & 0 deletions apps/web/src/lib/code-reviews/db/code-reviews.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ export async function createCodeReview(params: CreateReviewParams): Promise<stri
head_sha: params.headSha,
platform: params.platform ?? 'github',
platform_project_id: params.platformProjectId ?? null,
agent_version: 'v2',
status: 'pending',
})
.returning({ id: cloud_agent_code_reviews.id });
Expand Down
22 changes: 5 additions & 17 deletions apps/web/src/lib/code-reviews/dispatch/dispatch-pending-reviews.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ import { getAgentConfigForOwner } from '@/lib/agent-config/db/agent-configs';
import { updateCodeReviewStatus } from '../db/code-reviews';
import { captureException } from '@sentry/nextjs';
import { errorExceptInTest, logExceptInTest } from '@/lib/utils.server';
import { isFeatureFlagEnabled } from '@/lib/posthog-feature-flags';
import { codeReviewWorkerClient } from '../client/code-review-worker-client';
import type { CodeReviewPlatform } from '../core/schemas';

Expand Down Expand Up @@ -201,26 +200,15 @@ async function dispatchReview(
);
}

// 2. Evaluate feature flag: use cloud-agent-next?
const useCloudAgentNext =
process.env.NODE_ENV === 'development' ||
(await isFeatureFlagEnabled('code-review-cloud-agent-next', owner.userId));

logExceptInTest('[dispatchReview] Feature flag evaluated', {
reviewId: review.id,
userId: owner.userId,
useCloudAgentNext,
});

// 3. Prepare complete payload for cloud agent
// 2. Prepare complete payload for cloud agent
const payload = await prepareReviewPayload({
reviewId: review.id,
owner,
agentConfig,
platform,
});

// 4. Atomically claim the review to prevent concurrent dispatchers from
// 3. Atomically claim the review to prevent concurrent dispatchers from
// picking the same review. Done as late as possible (after all prep work)
// to minimise the crash window between claim and dispatch.
// Accepts 'pending' (normal) or stale 'queued' (abandoned claim recovery).
Expand Down Expand Up @@ -248,11 +236,11 @@ async function dispatchReview(
return false;
}

// 5. Dispatch to Cloudflare Worker to create CodeReviewOrchestrator DO.
// 4. Dispatch to Cloudflare Worker to create CodeReviewOrchestrator DO.
// If this fails, keep the claim in `queued` and rely on stale-claim
// recovery. A transport failure is ambiguous: the worker may have
// created the DO even if this request did not observe the response.
const agentVersion = useCloudAgentNext ? 'v2' : 'v1';
const agentVersion = 'v2';
Comment thread
alex-alecu marked this conversation as resolved.
try {
await codeReviewWorkerClient.dispatchReview({
...payload,
Expand All @@ -271,7 +259,7 @@ async function dispatchReview(
return false;
}

// 6. Record which agent version was dispatched without rewriting status.
// 5. Record which agent version was dispatched without rewriting status.
// The worker may already have advanced the review to running/completed.
try {
await db
Expand Down
85 changes: 31 additions & 54 deletions apps/web/src/lib/code-reviews/triggers/prepare-review-payload.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,12 +37,7 @@ import type {
} from '../prompts/generate-prompt';
import { getIntegrationById } from '@/lib/integrations/db/platform-integrations';
import { getCodeReviewById, findPreviousCompletedReview } from '../db/code-reviews';
import { isFeatureFlagEnabled } from '@/lib/posthog-feature-flags';
import {
DEFAULT_CODE_REVIEW_MODEL,
DEFAULT_CODE_REVIEW_MODE,
FEATURE_FLAG_INCREMENTAL_REVIEW,
} from '../core/constants';
import { DEFAULT_CODE_REVIEW_MODEL, DEFAULT_CODE_REVIEW_MODE } from '../core/constants';
import type { Owner } from '../core';
import { generateReviewPrompt } from '../prompts/generate-prompt';
import type { CodeReviewAgentConfig } from '@/lib/agent-config/core/types';
Expand Down Expand Up @@ -319,54 +314,41 @@ export async function prepareReviewPayload(
// continuation) are derived from the same review row to avoid mismatches.
let previousHeadSha: string | null = null;
let previousCloudAgentSessionId: string | undefined;
const incrementalEnabled = await isFeatureFlagEnabled(
FEATURE_FLAG_INCREMENTAL_REVIEW,
owner.userId
);

logExceptInTest('[prepareReviewPayload] Incremental review flag evaluated', {
reviewId,
incrementalEnabled,
ownerId: owner.userId,
});

if (incrementalEnabled) {
try {
const previousReview = await findPreviousCompletedReview(
review.repo_full_name,
review.pr_number,
existingReviewState?.headCommitSha ?? review.head_sha,
platform
);
previousHeadSha = previousReview?.head_sha ?? null;
previousCloudAgentSessionId = previousReview?.session_id ?? undefined;

if (previousHeadSha) {
logExceptInTest(
'[prepareReviewPayload] Found previous completed review for incremental mode',
{
reviewId,
previousHeadSha: previousHeadSha.substring(0, 8),
currentHeadSha: review.head_sha.substring(0, 8),
previousCloudAgentSessionId,
}
);
} else {
logExceptInTest(
'[prepareReviewPayload] No previous completed review found, using full review',
{ reviewId }
);
}
} catch (error) {
// Non-critical - fall back to full review
try {
const previousReview = await findPreviousCompletedReview(
review.repo_full_name,
review.pr_number,
existingReviewState?.headCommitSha ?? review.head_sha,
platform
);
previousHeadSha = previousReview?.head_sha ?? null;
previousCloudAgentSessionId = previousReview?.session_id ?? undefined;

if (previousHeadSha) {
logExceptInTest(
'[prepareReviewPayload] Failed to fetch previous review, falling back to full review:',
'[prepareReviewPayload] Found previous completed review for incremental mode',
{
reviewId,
error,
previousHeadSha: previousHeadSha.substring(0, 8),
currentHeadSha: review.head_sha.substring(0, 8),
previousCloudAgentSessionId,
}
);
} else {
logExceptInTest(
'[prepareReviewPayload] No previous completed review found, using full review',
{ reviewId }
);
}
} catch (error) {
// Non-critical - fall back to full review
logExceptInTest(
'[prepareReviewPayload] Failed to fetch previous review, falling back to full review:',
{
reviewId,
error,
}
);
}

// 5. Generate auth token for cloud agent with bot identifier
Expand Down Expand Up @@ -402,12 +384,7 @@ export async function prepareReviewPayload(
// GitHub: uses githubRepo (owner/repo format) + githubToken
// GitLab: uses gitUrl (full HTTPS URL) + gitToken
const variant = config.thinking_effort ?? undefined;
// Defense-in-depth: only send gateThreshold to the agent when the PR gate flag is enabled.
// This prevents a stale non-'off' config from activating gating after the flag is turned off.
const isPrGateEnabled =
process.env.NODE_ENV === 'development' ||
(await isFeatureFlagEnabled('code-review-pr-gate', owner.userId));
const gateThreshold = isPrGateEnabled ? (config.gate_threshold ?? 'off') : 'off';
const gateThreshold = config.gate_threshold ?? 'off';
const sessionInput: SessionInput =
platform === PLATFORM.GITLAB
? {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { resolvePullRequestCheckoutRef } from '@/lib/integrations/platforms/github/webhook-handlers/pull-request-checkout-ref';
import { shouldSkipSynchronizeForMergeCommit } from '@/lib/integrations/platforms/github/webhook-handlers/pull-request-handler';

describe('resolvePullRequestCheckoutRef', () => {
it('uses head.ref for same-repo PRs', () => {
Expand Down Expand Up @@ -63,3 +64,64 @@ describe('resolvePullRequestCheckoutRef', () => {
});
});
});

describe('shouldSkipSynchronizeForMergeCommit', () => {
const baseArgs = {
installationId: 'inst-1',
headOwner: 'acme',
headRepoName: 'widgets',
headSha: 'deadbeef',
appType: 'standard' as const,
};

it('returns false for non-synchronize actions without calling the check', async () => {
for (const action of ['opened', 'reopened', 'ready_for_review']) {
let called = false;
const result = await shouldSkipSynchronizeForMergeCommit({
...baseArgs,
action,
isMergeCommitFn: async () => {
called = true;
return true;
},
});

expect(result).toBe(false);
expect(called).toBe(false);
}
});

it('returns true when synchronize head is a merge commit', async () => {
const result = await shouldSkipSynchronizeForMergeCommit({
...baseArgs,
action: 'synchronize',
isMergeCommitFn: async () => true,
});

expect(result).toBe(true);
});

it('returns false when synchronize head is not a merge commit', async () => {
const result = await shouldSkipSynchronizeForMergeCommit({
...baseArgs,
action: 'synchronize',
isMergeCommitFn: async () => false,
});

expect(result).toBe(false);
});

it('passes the expected arguments to the check function', async () => {
const calls: Array<[string, string, string, string, string]> = [];
await shouldSkipSynchronizeForMergeCommit({
...baseArgs,
action: 'synchronize',
isMergeCommitFn: async (installationId, owner, repo, sha, appType) => {
calls.push([installationId, owner, repo, sha, appType]);
return false;
},
});

expect(calls).toEqual([['inst-1', 'acme', 'widgets', 'deadbeef', 'standard']]);
});
});
Loading
Loading