diff --git a/cloud-agent-next/src/persistence/CloudAgentSession.ts b/cloud-agent-next/src/persistence/CloudAgentSession.ts index 7b475bfaf6..3868ae65ae 100644 --- a/cloud-agent-next/src/persistence/CloudAgentSession.ts +++ b/cloud-agent-next/src/persistence/CloudAgentSession.ts @@ -598,6 +598,25 @@ export class CloudAgentSession extends DurableObject { await this.updateMetadata(updated); } + /** + * Update the callback target for this session. + * This allows redirecting completion callbacks to a new URL (e.g., for follow-up reviews). + */ + private async updateCallbackTarget(callbackTarget: CallbackTarget): Promise { + const metadata = await this.getMetadata(); + if (!metadata) { + throw new Error('Cannot update callbackTarget: session metadata not found'); + } + + const updated = { + ...metadata, + callbackTarget, + version: Date.now(), // Bump version for cache invalidation + }; + + await this.updateMetadata(updated); + } + /** * Update the upstream branch for this session. * This allows capturing the branch after kilo execution without a full metadata write. @@ -785,7 +804,15 @@ export class CloudAgentSession extends DurableObject { if (!metadata?.preparedAt) { return { success: false, error: 'Session has not been prepared' }; } - if (metadata.initiatedAt) { + + // callbackTarget can be updated even after initiation (needed for follow-up + // reviews that reuse an existing session with a new callback URL). + // All other fields are immutable once initiated. + const allKeys = Object.keys(updates).filter( + k => updates[k as keyof typeof updates] !== undefined + ); + const onlyCallbackTarget = allKeys.length === 1 && allKeys[0] === 'callbackTarget'; + if (metadata.initiatedAt && !onlyCallbackTarget) { return { success: false, error: 'Session has already been initiated' }; } @@ -1208,13 +1235,19 @@ export class CloudAgentSession extends DurableObject { /** * Update execution status with state machine validation. + * + * When `suppressCallback` is true the status is persisted but no callback + * notification is enqueued. Used on the followup path where the caller + * (orchestrator) handles the error synchronously and enqueuing a callback + * would race with a fallback session's callbacks. */ async updateExecutionStatus( - params: UpdateExecutionStatusParams + params: UpdateExecutionStatusParams, + opts?: { suppressCallback?: boolean } ): Promise> { const result = await this.executionQueries.updateStatus(params); - if (result.ok && this.isTerminalStatus(params.status)) { + if (result.ok && this.isTerminalStatus(params.status) && !opts?.suppressCallback) { await this.enqueueCallbackNotification( params.executionId, params.status, @@ -1343,6 +1376,8 @@ export class CloudAgentSession extends DurableObject { error: string; streamEventType: string; streamPayload?: Record; + /** When true, skip enqueuing the callback notification. */ + suppressCallback?: boolean; }): Promise { const { executionId, status, error, streamEventType, streamPayload } = params; @@ -1354,13 +1389,16 @@ export class CloudAgentSession extends DurableObject { // decide whether to clean up the interrupt flag afterward. const wasActive = (await this.executionQueries.getActiveExecutionId()) === executionId; - // 1. Update status (enqueues callback notification on terminal) - const statusResult = await this.updateExecutionStatus({ - executionId, - status, - error, - completedAt: Date.now(), - }); + // 1. Update status (enqueues callback notification on terminal unless suppressed) + const statusResult = await this.updateExecutionStatus( + { + executionId, + status, + error, + completedAt: Date.now(), + }, + { suppressCallback: params.suppressCallback } + ); if (!statusResult.ok) { logger @@ -1932,7 +1970,6 @@ export class CloudAgentSession extends DurableObject { await this.updateGitToken(request.tokenOverrides.gitToken); metadata.gitToken = request.tokenOverrides.gitToken; } - const mode = (request.mode ?? metadata.mode ?? 'code') as ExecutionMode; const model = normalizeKilocodeModel(request.model ?? metadata.model); const variant = request.variant ?? metadata.variant; @@ -1984,7 +2021,12 @@ export class CloudAgentSession extends DurableObject { kiloSessionId: metadata.kiloSessionId, }); - return await this.executeDirectly(plan); + // Suppress failure callback for followup executions: the caller + // (orchestrator) receives the error synchronously via the tRPC + // response and has its own fallback logic. Enqueuing a callback + // here would race with the fallback session's callbacks and + // corrupt the new review's state (see PLAN-callback-race-fix.md). + return await this.executeDirectly(plan, { suppressCallbackOnError: true }); } catch (error) { // Handle ExecutionError specifically for proper error code mapping if (isExecutionError(error)) { @@ -2018,8 +2060,16 @@ export class CloudAgentSession extends DurableObject { /** * Execute a plan directly using the orchestrator. * This replaces the queue-based enqueueExecution pattern. + * + * @param suppressCallbackOnError — when true, a pre-start failure (e.g. + * workspace restore) will NOT enqueue a callback notification. The caller + * is expected to handle the error synchronously (used by the followup path + * where the orchestrator falls back to a fresh session on failure). */ - private async executeDirectly(plan: ExecutionPlan): Promise { + private async executeDirectly( + plan: ExecutionPlan, + opts?: { suppressCallbackOnError?: boolean } + ): Promise { const { executionId, sessionId, mode } = plan; logger.withFields({ sessionId, executionId }).info('executeDirectly called'); @@ -2066,6 +2116,7 @@ export class CloudAgentSession extends DurableObject { status: 'failed', error: errorMessage, streamEventType: 'error', + suppressCallback: opts?.suppressCallbackOnError, }); throw error; diff --git a/cloudflare-code-review-infra/src/code-review-orchestrator.ts b/cloudflare-code-review-infra/src/code-review-orchestrator.ts index ffbbaa63bb..273ad19f18 100644 --- a/cloudflare-code-review-infra/src/code-review-orchestrator.ts +++ b/cloudflare-code-review-infra/src/code-review-orchestrator.ts @@ -7,6 +7,10 @@ */ import { DurableObject } from 'cloudflare:workers'; +import { + createCloudAgentNextFetchClient, + type CloudAgentNextFetchClient, +} from '@kilocode/worker-utils'; import type { Env, CodeReview, @@ -64,6 +68,9 @@ export class CodeReviewOrchestrator extends DurableObject { /** In-memory cache of current review state */ private state!: CodeReview; + /** Shared typed client for cloud-agent-next tRPC endpoints */ + private cloudAgentNextClient: CloudAgentNextFetchClient | undefined; + /** Maximum time to wait for SSE stream (20 minutes) */ private static readonly STREAM_TIMEOUT_MS = 20 * 60 * 1000; @@ -89,6 +96,11 @@ export class CodeReviewOrchestrator extends DurableObject { super(ctx, env); } + private getCloudAgentNextClient(): CloudAgentNextFetchClient { + this.cloudAgentNextClient ??= createCloudAgentNextFetchClient(this.env.CLOUD_AGENT_NEXT_URL); + return this.cloudAgentNextClient; + } + /** * Alarm handler for scheduled cleanup tasks. * Only used for cleanup after review completion (7 days later). @@ -361,6 +373,7 @@ export class CodeReviewOrchestrator extends DurableObject { }; skipBalanceCheck?: boolean; agentVersion?: string; + previousCloudAgentSessionId?: string; }): Promise<{ status: CodeReviewStatus }> { this.state = { reviewId: params.reviewId, @@ -371,6 +384,7 @@ export class CodeReviewOrchestrator extends DurableObject { updatedAt: new Date().toISOString(), skipBalanceCheck: params.skipBalanceCheck, agentVersion: params.agentVersion, + previousCloudAgentSessionId: params.previousCloudAgentSessionId, }; await this.saveState(); @@ -467,22 +481,24 @@ export class CodeReviewOrchestrator extends DurableObject { * Routes to the correct backend based on agentVersion. */ private async interruptCloudAgentSession(sessionId: string): Promise { - const baseUrl = - this.state.agentVersion === 'v2' ? this.env.CLOUD_AGENT_NEXT_URL : this.env.CLOUD_AGENT_URL; - const cloudAgentUrl = `${baseUrl}/trpc/interruptSession`; + const headers: Record = { + Authorization: `Bearer ${this.state.authToken}`, + }; - const response = await fetch(cloudAgentUrl, { - method: 'POST', - headers: { - Authorization: `Bearer ${this.state.authToken}`, - 'Content-Type': 'application/json', - }, - body: JSON.stringify({ sessionId }), - }); + if (this.state.agentVersion === 'v2') { + await this.getCloudAgentNextClient().interruptSession(headers, { sessionId }); + } else { + // Legacy cloud-agent path — raw fetch (SSE-based service, not covered by shared client) + const response = await fetch(`${this.env.CLOUD_AGENT_URL}/trpc/interruptSession`, { + method: 'POST', + headers: { ...headers, 'Content-Type': 'application/json' }, + body: JSON.stringify({ sessionId }), + }); - if (!response.ok) { - const errorText = await response.text(); - throw new Error(`Cloud agent returned ${response.status}: ${errorText}`); + if (!response.ok) { + const errorText = await response.text(); + throw new Error(`Cloud agent returned ${response.status}: ${errorText}`); + } } } @@ -527,7 +543,11 @@ export class CodeReviewOrchestrator extends DurableObject { }); if (agentVersion === 'v2') { - await this.runWithCloudAgentNext(); + if (this.state.previousCloudAgentSessionId) { + await this.runWithCloudAgentNextFollowup(); + } else { + await this.runWithCloudAgentNext(); + } } else { await this.runWithCloudAgent(); } @@ -545,6 +565,7 @@ export class CodeReviewOrchestrator extends DurableObject { */ private async runWithCloudAgentNext(): Promise { const runStartTime = Date.now(); + const client = this.getCloudAgentNextClient(); try { await this.updateStatus('running'); @@ -555,62 +576,39 @@ export class CodeReviewOrchestrator extends DurableObject { }); // Build common headers for prepareSession (internalApiProtectedProcedure) - const headers: Record = { + const internalHeaders: Record = { Authorization: `Bearer ${this.state.authToken}`, - 'Content-Type': 'application/json', 'x-internal-api-key': this.env.INTERNAL_API_SECRET, }; if (this.state.skipBalanceCheck) { - headers['x-skip-balance-check'] = 'true'; + internalHeaders['x-skip-balance-check'] = 'true'; } // Step 1: Prepare session with callback target + const callbackTarget = { + url: `${this.env.API_URL}/api/internal/code-review-status/${this.state.reviewId}`, + headers: { + 'X-Internal-Secret': this.env.INTERNAL_API_SECRET, + }, + }; + const prepareInput = { ...this.state.sessionInput, - createdOnPlatform: 'code-review', - callbackTarget: { - url: `${this.env.API_URL}/api/internal/code-review-status/${this.state.reviewId}`, - headers: { - 'X-Internal-Secret': this.env.INTERNAL_API_SECRET, - }, - }, + createdOnPlatform: 'code-review' as const, + callbackTarget, }; console.log('[CodeReviewOrchestrator] Calling prepareSession', { reviewId: this.state.reviewId, - callbackUrl: prepareInput.callbackTarget.url, + callbackUrl: callbackTarget.url, createdOnPlatform: prepareInput.createdOnPlatform, skipBalanceCheck: this.state.skipBalanceCheck, }); - const prepareResponse = await fetch(`${this.env.CLOUD_AGENT_NEXT_URL}/trpc/prepareSession`, { - method: 'POST', - headers, - body: JSON.stringify(prepareInput), - }); - - if (!prepareResponse.ok) { - const errorText = await prepareResponse.text(); - throw new Error(`prepareSession failed (${prepareResponse.status}): ${errorText}`); - } - - const prepareResult: Record = await prepareResponse.json(); - const prepareData = (prepareResult?.result as Record)?.data as - | Record - | undefined; - if ( - !prepareData || - typeof prepareData.cloudAgentSessionId !== 'string' || - typeof prepareData.kiloSessionId !== 'string' - ) { - throw new Error( - `Unexpected prepareSession response shape: ${JSON.stringify(prepareResult).slice(0, 500)}` - ); - } - const { cloudAgentSessionId, kiloSessionId } = prepareData as { - cloudAgentSessionId: string; - kiloSessionId: string; - }; + const { cloudAgentSessionId, kiloSessionId } = await client.prepareSession( + internalHeaders, + prepareInput + ); console.log('[CodeReviewOrchestrator] Session prepared', { reviewId: this.state.reviewId, @@ -626,12 +624,11 @@ export class CodeReviewOrchestrator extends DurableObject { // Step 2: Initiate execution // initiateFromKilocodeSessionV2 is a protectedProcedure (Bearer token only) - const initiateHeaders: Record = { + const userHeaders: Record = { Authorization: `Bearer ${this.state.authToken}`, - 'Content-Type': 'application/json', }; if (this.state.skipBalanceCheck) { - initiateHeaders['x-skip-balance-check'] = 'true'; + userHeaders['x-skip-balance-check'] = 'true'; } console.log('[CodeReviewOrchestrator] Calling initiateFromKilocodeSessionV2', { @@ -639,37 +636,15 @@ export class CodeReviewOrchestrator extends DurableObject { cloudAgentSessionId, }); - const initiateResponse = await fetch( - `${this.env.CLOUD_AGENT_NEXT_URL}/trpc/initiateFromKilocodeSessionV2`, - { - method: 'POST', - headers: initiateHeaders, - body: JSON.stringify({ cloudAgentSessionId }), - } - ); - - if (!initiateResponse.ok) { - const errorText = await initiateResponse.text(); - throw new Error( - `initiateFromKilocodeSessionV2 failed (${initiateResponse.status}): ${errorText}` - ); - } - - const initiateResult: Record = await initiateResponse.json(); - const initiateData = (initiateResult?.result as Record)?.data as - | Record - | undefined; - if (!initiateData || typeof initiateData.executionId !== 'string') { - throw new Error( - `Unexpected initiateFromKilocodeSessionV2 response shape: ${JSON.stringify(initiateResult).slice(0, 500)}` - ); - } + const initiateResult = await client.initiateFromPreparedSession(userHeaders, { + cloudAgentSessionId, + }); console.log('[CodeReviewOrchestrator] Execution started', { reviewId: this.state.reviewId, cloudAgentSessionId, - executionId: initiateData.executionId, - status: initiateData.status, + executionId: initiateResult.executionId, + status: initiateResult.status, }); // Done — cloud-agent-next callback will deliver terminal status @@ -703,6 +678,121 @@ export class CodeReviewOrchestrator extends DurableObject { } } + // --------------------------------------------------------------------------- + // cloud-agent-next follow-up flow (session continuation) + // Uses sendMessageV2 to reuse an existing session from a previous review. + // Falls back to fresh session (prepareSession + initiate) on failure. + // --------------------------------------------------------------------------- + + /** + * Orchestration via cloud-agent-next with session continuation. + * Calls sendMessageV2 on an existing session from a previous review. + * On failure (404, 409, etc.), falls back to runWithCloudAgentNext() for a fresh session. + */ + private async runWithCloudAgentNextFollowup(): Promise { + const previousSessionId = this.state.previousCloudAgentSessionId; + if (!previousSessionId) { + throw new Error('runWithCloudAgentNextFollowup called without previousCloudAgentSessionId'); + } + const client = this.getCloudAgentNextClient(); + + console.log('[CodeReviewOrchestrator] Attempting session continuation via sendMessageV2', { + reviewId: this.state.reviewId, + previousCloudAgentSessionId: previousSessionId, + }); + + try { + await this.updateStatus('running'); + + // Build internal headers (internalApiProtectedProcedure — API key + Bearer token) + const internalHeaders: Record = { + Authorization: `Bearer ${this.state.authToken}`, + 'x-internal-api-key': this.env.INTERNAL_API_SECRET, + }; + if (this.state.skipBalanceCheck) { + internalHeaders['x-skip-balance-check'] = 'true'; + } + + // Step 1: Update callback target via updateSession (internal-only endpoint). + // callbackTarget must be set through an internal procedure, not the + // user-facing sendMessageV2, to prevent SSRF via arbitrary callback URLs. + const callbackTarget = { + url: `${this.env.API_URL}/api/internal/code-review-status/${this.state.reviewId}`, + headers: { + 'X-Internal-Secret': this.env.INTERNAL_API_SECRET, + }, + }; + + await client.updateSession(internalHeaders, { + cloudAgentSessionId: previousSessionId, + callbackTarget, + }); + + // Step 2: Send follow-up message (user-facing, no callbackTarget) + const userHeaders: Record = { + Authorization: `Bearer ${this.state.authToken}`, + }; + if (this.state.skipBalanceCheck) { + userHeaders['x-skip-balance-check'] = 'true'; + } + + console.log('[CodeReviewOrchestrator] Calling sendMessageV2', { + reviewId: this.state.reviewId, + cloudAgentSessionId: previousSessionId, + callbackUrl: callbackTarget.url, + }); + + const sendResult = await client.sendMessageV2(userHeaders, { + cloudAgentSessionId: previousSessionId, + prompt: this.state.sessionInput.prompt, + mode: this.state.sessionInput.mode, + model: this.state.sessionInput.model, + variant: this.state.sessionInput.variant, + githubToken: this.state.sessionInput.githubToken, + gitToken: this.state.sessionInput.gitToken, + }); + + // Store session ID (reusing the previous one) and execution ID + await this.updateStatus('running', { + sessionId: previousSessionId, + }); + + console.log('[CodeReviewOrchestrator] Follow-up execution started via sendMessageV2', { + reviewId: this.state.reviewId, + cloudAgentSessionId: previousSessionId, + executionId: sendResult.executionId, + status: sendResult.status, + }); + + // Done — cloud-agent-next callback will deliver terminal status + } catch (error) { + const errorMessage = error instanceof Error ? error.message : 'Unknown error'; + + console.warn('[CodeReviewOrchestrator] sendMessageV2 failed, falling back to fresh session', { + reviewId: this.state.reviewId, + previousCloudAgentSessionId: previousSessionId, + error: errorMessage, + }); + + // Reset status to running (it may have been set to running already, but ensure clean state) + // Clear previousCloudAgentSessionId so the fresh session path doesn't try followup again + this.state.previousCloudAgentSessionId = undefined; + + try { + await this.runWithCloudAgentNext(); + } catch (freshError) { + // runWithCloudAgentNext handles its own error/status updates, so this catch + // is only for unexpected throws that bypass its internal error handling + const freshErrorMessage = + freshError instanceof Error ? freshError.message : 'Unknown error'; + console.error('[CodeReviewOrchestrator] Fresh session fallback also failed', { + reviewId: this.state.reviewId, + error: freshErrorMessage, + }); + } + } + } + // --------------------------------------------------------------------------- // cloud-agent flow (default / legacy) // Uses SSE streaming via initiateSessionAsync. diff --git a/cloudflare-code-review-infra/src/index.ts b/cloudflare-code-review-infra/src/index.ts index 168c6afe79..086efd59fe 100644 --- a/cloudflare-code-review-infra/src/index.ts +++ b/cloudflare-code-review-infra/src/index.ts @@ -91,6 +91,7 @@ app.post('/review', async (c: Context) => { owner: body.owner, skipBalanceCheck: body.skipBalanceCheck, agentVersion: body.agentVersion, + previousCloudAgentSessionId: body.previousCloudAgentSessionId, }), 'start' ); diff --git a/cloudflare-code-review-infra/src/types.ts b/cloudflare-code-review-infra/src/types.ts index 2964fea272..70cedc0981 100644 --- a/cloudflare-code-review-infra/src/types.ts +++ b/cloudflare-code-review-infra/src/types.ts @@ -65,6 +65,8 @@ export interface CodeReview { skipBalanceCheck?: boolean; // Skip balance validation in cloud agent (for OSS sponsorship) /** Which cloud agent backend to use: 'v1' (cloud-agent SSE) or 'v2' (cloud-agent-next) */ agentVersion?: string; + /** Cloud-agent session ID from a previous completed review, for session continuation */ + previousCloudAgentSessionId?: string; } export interface CodeReviewStatusResponse { @@ -93,6 +95,8 @@ export interface CodeReviewRequest { skipBalanceCheck?: boolean; /** Which cloud agent backend to use: 'v1' (cloud-agent SSE) or 'v2' (cloud-agent-next) */ agentVersion?: string; + /** Cloud-agent session ID from a previous completed review, for session continuation */ + previousCloudAgentSessionId?: string; } export interface CodeReviewResponse { diff --git a/packages/worker-utils/src/cloud-agent-next-client.ts b/packages/worker-utils/src/cloud-agent-next-client.ts new file mode 100644 index 0000000000..f25585af12 --- /dev/null +++ b/packages/worker-utils/src/cloud-agent-next-client.ts @@ -0,0 +1,228 @@ +/** + * Lightweight, fetch-based client for cloud-agent-next tRPC endpoints. + * + * Designed to work in Cloudflare Workers (no Node.js dependencies) so both + * the code-review orchestrator DO and the Next.js server can share the same + * typed interface. The Next.js `CloudAgentNextClient` wraps a full tRPC client + * with Sentry and credit-error handling; this module covers only the raw HTTP + * transport layer and response parsing. + */ + +// --------------------------------------------------------------------------- +// Types — aligned with cloud-agent-next tRPC router contracts +// --------------------------------------------------------------------------- + +export type CallbackTarget = { + url: string; + headers?: Record; +}; + +export type CloudAgentPrepareSessionInput = { + prompt: string; + mode: string; + model: string; + variant?: string; + githubRepo?: string; + githubToken?: string; + gitUrl?: string; + gitToken?: string; + platform?: 'github' | 'gitlab'; + kilocodeOrganizationId?: string; + envVars?: Record; + mcpServers?: Record; + upstreamBranch?: string; + callbackTarget?: CallbackTarget; + createdOnPlatform?: string; + gateThreshold?: 'off' | 'all' | 'warning' | 'critical'; +}; + +export type CloudAgentPrepareSessionOutput = { + cloudAgentSessionId: string; + kiloSessionId: string; +}; + +export type CloudAgentInitiateInput = { + cloudAgentSessionId: string; +}; + +export type CloudAgentInitiateOutput = { + executionId: string; + status?: string; +}; + +export type CloudAgentUpdateSessionInput = { + cloudAgentSessionId: string; + callbackTarget?: CallbackTarget | null; + [key: string]: unknown; +}; + +export type CloudAgentSendMessageInput = { + cloudAgentSessionId: string; + prompt: string; + mode: string; + model: string; + variant?: string; + githubToken?: string; + gitToken?: string; +}; + +export type CloudAgentSendMessageOutput = { + executionId: string; + status?: string; +}; + +export type CloudAgentInterruptInput = { + sessionId: string; +}; + +export type CloudAgentInterruptOutput = { + success: boolean; + message: string; + processesFound: boolean; +}; + +// --------------------------------------------------------------------------- +// tRPC HTTP helpers +// --------------------------------------------------------------------------- + +class CloudAgentNextError extends Error { + readonly status: number; + constructor(procedure: string, status: number, body: string) { + super(`${procedure} failed (${status}): ${body}`); + this.name = 'CloudAgentNextError'; + this.status = status; + } +} + +/** + * Parse a tRPC JSON-RPC envelope and return `result.data`, throwing on + * non-200 responses or unexpected shapes. + */ +async function trpcPost( + url: string, + headers: Record, + body: unknown, + procedure: string +): Promise { + const response = await fetch(url, { + method: 'POST', + headers: { 'Content-Type': 'application/json', ...headers }, + body: JSON.stringify(body), + }); + + if (!response.ok) { + const errorText = await response.text(); + throw new CloudAgentNextError(procedure, response.status, errorText); + } + + const json = (await response.json()) as Record; + const data = (json?.result as Record | undefined)?.data; + if (data === undefined) { + throw new Error( + `Unexpected ${procedure} response shape: ${JSON.stringify(json).slice(0, 500)}` + ); + } + return data as T; +} + +// --------------------------------------------------------------------------- +// Client factory +// --------------------------------------------------------------------------- + +export type CloudAgentNextFetchClient = { + prepareSession( + headers: Record, + input: CloudAgentPrepareSessionInput + ): Promise; + + initiateFromPreparedSession( + headers: Record, + input: CloudAgentInitiateInput + ): Promise; + + updateSession( + headers: Record, + input: CloudAgentUpdateSessionInput + ): Promise; + + sendMessageV2( + headers: Record, + input: CloudAgentSendMessageInput + ): Promise; + + interruptSession( + headers: Record, + input: CloudAgentInterruptInput + ): Promise; +}; + +/** + * Create a typed, fetch-based client for cloud-agent-next tRPC endpoints. + * + * The caller is responsible for assembling the correct headers (Bearer token, + * internal API key, skip-balance-check, etc.) because different procedures + * require different auth levels. + */ +export function createCloudAgentNextFetchClient(baseUrl: string): CloudAgentNextFetchClient { + const trpc = (procedure: string) => `${baseUrl}/trpc/${procedure}`; + + return { + async prepareSession(headers, input) { + const data = await trpcPost>( + trpc('prepareSession'), + headers, + input, + 'prepareSession' + ); + if (typeof data.cloudAgentSessionId !== 'string' || typeof data.kiloSessionId !== 'string') { + throw new Error( + `Unexpected prepareSession response shape: ${JSON.stringify(data).slice(0, 500)}` + ); + } + return data as unknown as CloudAgentPrepareSessionOutput; + }, + + async initiateFromPreparedSession(headers, input) { + const data = await trpcPost>( + trpc('initiateFromKilocodeSessionV2'), + headers, + input, + 'initiateFromKilocodeSessionV2' + ); + if (typeof data.executionId !== 'string') { + throw new Error( + `Unexpected initiateFromKilocodeSessionV2 response shape: ${JSON.stringify(data).slice(0, 500)}` + ); + } + return data as unknown as CloudAgentInitiateOutput; + }, + + async updateSession(headers, input) { + await trpcPost(trpc('updateSession'), headers, input, 'updateSession'); + }, + + async sendMessageV2(headers, input) { + const data = await trpcPost>( + trpc('sendMessageV2'), + headers, + input, + 'sendMessageV2' + ); + if (typeof data.executionId !== 'string') { + throw new Error( + `Unexpected sendMessageV2 response shape: ${JSON.stringify(data).slice(0, 500)}` + ); + } + return data as unknown as CloudAgentSendMessageOutput; + }, + + async interruptSession(headers, input) { + return trpcPost( + trpc('interruptSession'), + headers, + input, + 'interruptSession' + ); + }, + }; +} diff --git a/packages/worker-utils/src/index.ts b/packages/worker-utils/src/index.ts index 22b89a2142..e7fa98058f 100644 --- a/packages/worker-utils/src/index.ts +++ b/packages/worker-utils/src/index.ts @@ -23,6 +23,21 @@ export { createNotFoundHandler } from './not-found-handler.js'; export type { Owner, MCPServerConfig } from './types.js'; +export { createCloudAgentNextFetchClient } from './cloud-agent-next-client.js'; +export type { + CloudAgentNextFetchClient, + CallbackTarget, + CloudAgentPrepareSessionInput, + CloudAgentPrepareSessionOutput, + CloudAgentInitiateInput, + CloudAgentInitiateOutput, + CloudAgentUpdateSessionInput, + CloudAgentSendMessageInput, + CloudAgentSendMessageOutput, + CloudAgentInterruptInput, + CloudAgentInterruptOutput, +} from './cloud-agent-next-client.js'; + export { signKiloToken, verifyKiloToken, diff --git a/src/app/api/internal/code-review-status/[reviewId]/route.ts b/src/app/api/internal/code-review-status/[reviewId]/route.ts index d41ba52668..86567e4a6b 100644 --- a/src/app/api/internal/code-review-status/[reviewId]/route.ts +++ b/src/app/api/internal/code-review-status/[reviewId]/route.ts @@ -401,6 +401,28 @@ export async function POST( }); } + // Defense-in-depth: reject callbacks from superseded sessions. + // When the orchestrator retries with a fresh session after a failed + // continuation attempt, a stale failure callback from the old session + // may arrive and corrupt the new review's state. If the review already + // has a session_id and the callback carries a different sessionId, the + // callback belongs to a previous (superseded) session — ignore it. + if (sessionId && review.session_id && sessionId !== review.session_id) { + logExceptInTest( + '[code-review-status] Stale callback from superseded session, skipping update', + { + reviewId, + callbackSessionId: sessionId, + reviewSessionId: review.session_id, + requestedStatus: status, + } + ); + return NextResponse.json({ + success: true, + message: 'Stale callback from superseded session', + }); + } + // Valid transitions: // - queued -> running (orchestrator starting) // - running -> running (sessionId update) diff --git a/src/lib/code-reviews/core/constants.ts b/src/lib/code-reviews/core/constants.ts index 27480ec65b..64ebd3865c 100644 --- a/src/lib/code-reviews/core/constants.ts +++ b/src/lib/code-reviews/core/constants.ts @@ -32,6 +32,13 @@ export function isActiveReviewPromo(botId: string | undefined, model: string): b return Date.now() < Date.parse(REVIEW_PROMO_END); } +// ============================================================================ +// Feature Flags +// ============================================================================ + +/** PostHog flag that gates incremental (diff-only) reviews on follow-up pushes */ +export const FEATURE_FLAG_INCREMENTAL_REVIEW = 'code-review-incremental'; + // ============================================================================ // Pagination // ============================================================================ diff --git a/src/lib/code-reviews/db/code-reviews.test.ts b/src/lib/code-reviews/db/code-reviews.test.ts new file mode 100644 index 0000000000..82589bfb3f --- /dev/null +++ b/src/lib/code-reviews/db/code-reviews.test.ts @@ -0,0 +1,116 @@ +import { db } from '@/lib/drizzle'; +import { cloud_agent_code_reviews, kilocode_users } from '@kilocode/db/schema'; +import { eq } from 'drizzle-orm'; +import { insertTestUser } from '@/tests/helpers/user.helper'; +import type { User } from '@kilocode/db/schema'; +import { + createCodeReview, + updateCodeReviewStatus, + findPreviousCompletedReview, +} from './code-reviews'; + +const REPO = `test-org/session-continuation-${Date.now()}`; + +describe('findPreviousCompletedReview', () => { + let testUser: User; + const createdReviewIds: string[] = []; + + beforeAll(async () => { + testUser = await insertTestUser(); + }); + + afterAll(async () => { + for (const id of createdReviewIds) { + await db.delete(cloud_agent_code_reviews).where(eq(cloud_agent_code_reviews.id, id)); + } + await db.delete(kilocode_users).where(eq(kilocode_users.id, testUser.id)); + }); + + async function createReview(headSha: string) { + const id = await createCodeReview({ + owner: { type: 'user', id: testUser.id, userId: testUser.id }, + repoFullName: REPO, + prNumber: 42, + prUrl: `https://github.com/${REPO}/pull/42`, + prTitle: 'test PR', + prAuthor: 'octocat', + baseRef: 'main', + headRef: 'feature/test', + headSha, + platform: 'github', + }); + createdReviewIds.push(id); + return id; + } + + it('returns null when no previous completed review exists', async () => { + const result = await findPreviousCompletedReview(REPO, 42, 'abc123'); + expect(result).toBeNull(); + }); + + it('returns head_sha and session_id: null for a completed review without session', async () => { + const id = await createReview('sha-no-session'); + await updateCodeReviewStatus(id, 'completed'); + + const result = await findPreviousCompletedReview(REPO, 42, 'other-sha'); + expect(result).not.toBeNull(); + expect(result!.head_sha).toBe('sha-no-session'); + expect(result!.session_id).toBeNull(); + }); + + it('returns head_sha and session_id for a completed review with session', async () => { + const id = await createReview('sha-with-session'); + await updateCodeReviewStatus(id, 'completed', { + sessionId: 'agent_test123', + }); + + const result = await findPreviousCompletedReview(REPO, 42, 'other-sha'); + expect(result).not.toBeNull(); + expect(result!.head_sha).toBe('sha-with-session'); + expect(result!.session_id).toBe('agent_test123'); + }); + + it('excludes the current SHA', async () => { + const result = await findPreviousCompletedReview(REPO, 42, 'sha-with-session'); + // Should skip "sha-with-session" and fall back to "sha-no-session" + expect(result).not.toBeNull(); + expect(result!.head_sha).toBe('sha-no-session'); + }); + + it('returns the most recent completed review', async () => { + const id = await createReview('sha-newer'); + await updateCodeReviewStatus(id, 'completed', { + sessionId: 'agent_newer', + }); + + const result = await findPreviousCompletedReview(REPO, 42, 'other-sha'); + expect(result).not.toBeNull(); + expect(result!.head_sha).toBe('sha-newer'); + expect(result!.session_id).toBe('agent_newer'); + }); + + it('ignores non-completed reviews', async () => { + const id = await createReview('sha-running'); + await updateCodeReviewStatus(id, 'running', { + sessionId: 'agent_running', + }); + + // Should still return the most recent *completed* one + const result = await findPreviousCompletedReview(REPO, 42, 'other-sha'); + expect(result).not.toBeNull(); + expect(result!.head_sha).toBe('sha-newer'); + expect(result!.session_id).toBe('agent_newer'); + }); + + it('ensures session_id and head_sha come from the same row', async () => { + // Create a completed review with no session (simulates v1 legacy) + const legacyId = await createReview('sha-legacy-newest'); + await updateCodeReviewStatus(legacyId, 'completed'); + + const result = await findPreviousCompletedReview(REPO, 42, 'other-sha'); + expect(result).not.toBeNull(); + // The newest completed review has no session — both fields from same row + expect(result!.head_sha).toBe('sha-legacy-newest'); + expect(result!.session_id).toBeNull(); + }); +}); diff --git a/src/lib/code-reviews/db/code-reviews.ts b/src/lib/code-reviews/db/code-reviews.ts index 7f26900499..0905c2d5d5 100644 --- a/src/lib/code-reviews/db/code-reviews.ts +++ b/src/lib/code-reviews/db/code-reviews.ts @@ -424,6 +424,48 @@ export async function findActiveReviewsForPR( } } +/** + * Finds the most recent completed review for the same PR with a different SHA. + * Used for incremental reviews: returns the previous HEAD SHA so the agent + * can diff against it instead of re-reviewing the entire PR. + * Also returns session_id (nullable) so the caller can derive both the + * incremental diff base and the session continuation target from a single row. + */ +export async function findPreviousCompletedReview( + repoFullName: string, + prNumber: number, + excludeSha: string, + platform: string = 'github' +): Promise<{ head_sha: string; session_id: string | null } | null> { + try { + const [review] = await db + .select({ + head_sha: cloud_agent_code_reviews.head_sha, + session_id: cloud_agent_code_reviews.session_id, + }) + .from(cloud_agent_code_reviews) + .where( + and( + eq(cloud_agent_code_reviews.repo_full_name, repoFullName), + eq(cloud_agent_code_reviews.pr_number, prNumber), + eq(cloud_agent_code_reviews.platform, platform), + ne(cloud_agent_code_reviews.head_sha, excludeSha), + eq(cloud_agent_code_reviews.status, 'completed') + ) + ) + .orderBy(desc(cloud_agent_code_reviews.created_at)) + .limit(1); + + return review || null; + } catch (error) { + captureException(error, { + tags: { operation: 'findPreviousCompletedReview' }, + extra: { repoFullName, prNumber, excludeSha, platform }, + }); + throw error; + } +} + /** * Stores the GitHub Check Run ID on a code review record. * Called after creating the initial check run so we can update it later. diff --git a/src/lib/code-reviews/prompts/default-prompt-template-gitlab.json b/src/lib/code-reviews/prompts/default-prompt-template-gitlab.json index eb4bc213b3..0a7df3e2b5 100644 --- a/src/lib/code-reviews/prompts/default-prompt-template-gitlab.json +++ b/src/lib/code-reviews/prompts/default-prompt-template-gitlab.json @@ -11,6 +11,7 @@ "summaryCommandCreate": "## Summary Command: CREATE new note\n\nUse `glab api` to add a new comment to the MR:\n\n```bash\nglab api --method POST \"projects/{PROJECT_PATH_ENCODED}/merge_requests/{MR_IID}/notes\" \\\n -H \"Content-Type: application/json\" --input - << 'EOF'\n{\n \"body\": \"\\n## Code Review Summary\\n\\n**Status:** X Issues Found | **Recommendation:** Address before merge\\n\\n...rest of summary...\"\n}\nEOF\n```", "summaryCommandUpdate": "## Summary Command: UPDATE existing note\n\nNote ID: `{NOTE_ID}`\n\nUse `glab api` to update an existing note:\n\n```bash\nglab api --method PUT \"projects/{PROJECT_PATH_ENCODED}/merge_requests/{MR_IID}/notes/{NOTE_ID}\" \\\n -H \"Content-Type: application/json\" --input - << 'EOF'\n{\n \"body\": \"\\n## Code Review Summary\\n\\n...updated content...\"\n}\nEOF\n```", "inlineCommentsApi": "# COMMANDS\n\n## Inline Comments - MUST USE `glab api`\n\n⚠️ **CRITICAL:** To post inline comments on specific lines, you MUST use `glab api` with the discussions endpoint. The `glab mr note` command CANNOT post inline comments - it only posts general MR comments!\n\n### Required Command Format\n\nFor EVERY inline comment, use this EXACT format with JSON body via heredoc:\n\n```bash\nglab api --method POST \"projects/{PROJECT_PATH_ENCODED}/merge_requests/{MR_IID}/discussions\" \\\n -H \"Content-Type: application/json\" --input - << 'EOF'\n{\n \"body\": \"YOUR_COMMENT_HERE\",\n \"position\": {\n \"base_sha\": \"{BASE_SHA}\",\n \"start_sha\": \"{START_SHA}\",\n \"head_sha\": \"{HEAD_SHA}\",\n \"position_type\": \"text\",\n \"new_path\": \"PATH_TO_FILE\",\n \"new_line\": LINE_NUMBER\n }\n}\nEOF\n```\n\n**Replace these values:**\n- `YOUR_COMMENT_HERE`: Your comment body with `\\n` for newlines. Include suggestion blocks like: `**CRITICAL:** Issue\\n\\n```suggestion:-0+0\\nfixed line\\n```\n- `PATH_TO_FILE`: The file path (e.g., `src/utils.ts`)\n- `LINE_NUMBER`: The line number as integer (no quotes)\n\n**DO NOT replace these - they are pre-filled by the system:**\n- `{BASE_SHA}`, `{START_SHA}`, `{HEAD_SHA}` - Copy exactly as shown\n- `{PROJECT_PATH_ENCODED}`, `{MR_IID}` - Copy exactly as shown\n\n### Example 1: Fix a typo (with suggestion)\n\n```bash\nglab api --method POST \"projects/{PROJECT_PATH_ENCODED}/merge_requests/{MR_IID}/discussions\" \\\n -H \"Content-Type: application/json\" --input - << 'EOF'\n{\n \"body\": \"**CRITICAL:** Variable name typo - `searchTem` should be `searchTerm`\\n\\n```suggestion:-0+0\\n return searchTerm ? `${baseUrl}&name=${searchTerm}` : baseUrl;\\n```\",\n \"position\": {\n \"base_sha\": \"{BASE_SHA}\",\n \"start_sha\": \"{START_SHA}\",\n \"head_sha\": \"{HEAD_SHA}\",\n \"position_type\": \"text\",\n \"new_path\": \"src/utils.ts\",\n \"new_line\": 42\n }\n}\nEOF\n```\n\n### Example 2: Remove a line (empty suggestion)\n\n```bash\nglab api --method POST \"projects/{PROJECT_PATH_ENCODED}/merge_requests/{MR_IID}/discussions\" \\\n -H \"Content-Type: application/json\" --input - << 'EOF'\n{\n \"body\": \"**CRITICAL:** Invalid code - this line should be removed\\n\\n```suggestion:-0+0\\n```\",\n \"position\": {\n \"base_sha\": \"{BASE_SHA}\",\n \"start_sha\": \"{START_SHA}\",\n \"head_sha\": \"{HEAD_SHA}\",\n \"position_type\": \"text\",\n \"new_path\": \"src/index.js\",\n \"new_line\": 7\n }\n}\nEOF\n```\n\n### Example 3: Comment WITHOUT suggestion\n\n```bash\nglab api --method POST \"projects/{PROJECT_PATH_ENCODED}/merge_requests/{MR_IID}/discussions\" \\\n -H \"Content-Type: application/json\" --input - << 'EOF'\n{\n \"body\": \"**WARNING:** Potential null pointer exception\\n\\nThe variable `user` could be null here. Consider adding a null check.\",\n \"position\": {\n \"base_sha\": \"{BASE_SHA}\",\n \"start_sha\": \"{START_SHA}\",\n \"head_sha\": \"{HEAD_SHA}\",\n \"position_type\": \"text\",\n \"new_path\": \"src/handlers.ts\",\n \"new_line\": 15\n }\n}\nEOF\n```\n\n**Note:** Post each inline comment separately (GitLab doesn't support batch).", + "incrementalReviewWorkflow": "# INCREMENTAL REVIEW MODE\n\nThis MR was previously reviewed at commit `{PREVIOUS_SHA}`. The previous review findings are provided below so you can focus on what changed.\n\n## Previous Review Summary\n{PREVIOUS_SUMMARY}\n\n## Previous Inline Comments ({ACTIVE_COMMENT_COUNT} active)\nSee the Existing Inline Comments table below.\n\n## Incremental Workflow\n\n### Step 1: View ONLY the incremental diff\n```bash\ngit pull\ngit diff {PREVIOUS_SHA}..HEAD\n```\n\nIf this command fails (e.g., force push rewrote history), FALL BACK to full review:\nuse `glab mr diff {MR_IID}` and ignore previous context entirely.\n\n### Step 2: Analyze changed files\nFor EACH file in the incremental diff:\n- Read the FULL file for context\n- Check for ALL issue types: bugs, security problems, typos, logic errors, missing error handling, edge cases\n- Re-verify any previous issues on changed lines (they may be fixed)\n\nFor files NOT in the incremental diff: carry forward previous findings.\nDo NOT re-read or re-analyze unchanged files.\n\n### Step 3: Verify ALL Issues\n\nFor EACH potential issue you collected:\n1. **Read the actual line** - Use the Read tool\n2. **Confirm the issue exists** - The problem must be visible in the code\n3. **Check it's not already commented** - See Existing Comments table\n\n**Anti-hallucination:** ALWAYS read the actual line before commenting. If you think line 66 has a typo, READ line 66 first — the issue may not exist there.\n\n### Step 4: Submit inline comments\nOnly post NEW issues. Do NOT duplicate existing comments.\nPost each inline comment separately using the `glab api` format in the COMMANDS section.\n\n**Skip this step if no NEW issues found.**\n\n### Step 5: Update Summary\nUpdate the summary to reflect ALL issues:\n- NEW issues from changed files\n- Previous issues on unchanged files (carried forward)\n- Mark resolved issues that were fixed in new commits", "fixLinkTemplate": "## Fix Link (include if issues found)\n\n[Fix these issues in Kilo Cloud]({FIX_LINK})", "styleGuidance": { "strict": "# STRICT REVIEW MODE\n\nYou are a thorough, detail-oriented code reviewer. Your job is to catch everything — not just obvious bugs, but edge cases, potential future issues, and code smell.\n\n## Rules\n\n1. **Flag ALL potential issues**, not just high-confidence ones. Lower your reporting threshold.\n2. **Err on the side of caution.** If something *might* be a problem, report it.\n3. **Prioritize quality and security above all.** Missing error handling, missing input validation, and missing type safety are all worth flagging.\n4. **Include edge cases and future risks.** If a pattern could break under reasonable future changes, mention it.\n5. **Be direct and professional.** No sugar-coating — state the issue clearly and explain the risk.", diff --git a/src/lib/code-reviews/prompts/default-prompt-template.json b/src/lib/code-reviews/prompts/default-prompt-template.json index 3616a6f4cb..6df714ee4d 100644 --- a/src/lib/code-reviews/prompts/default-prompt-template.json +++ b/src/lib/code-reviews/prompts/default-prompt-template.json @@ -11,6 +11,7 @@ "summaryCommandCreate": "## Summary Command: CREATE new comment\n\n```bash\ngh api repos/{REPO}/issues/{PR}/comments --input - << 'EOF'\n{\n \"body\": \"\\n## Code Review Summary\\n\\n...\"\n}\nEOF\n```", "summaryCommandUpdate": "## Summary Command: UPDATE existing comment\n\nComment ID: `{COMMENT_ID}`\n\n```bash\ngh api repos/{REPO}/issues/comments/{COMMENT_ID} -X PATCH --input - << 'EOF'\n{\n \"body\": \"\\n## Code Review Summary\\n\\n...\"\n}\nEOF\n```", "inlineCommentsApi": "## Inline Comments API Call\n\n```bash\ngh api repos/{REPO}/pulls/{PR}/reviews --input - << 'EOF'\n{\n \"event\": \"COMMENT\",\n \"body\": \"\",\n \"comments\": [\n {\"path\": \"src/file.ts\", \"line\": 42, \"side\": \"RIGHT\", \"body\": \"**CRITICAL:** Issue\"}\n ]\n}\nEOF\n```", + "incrementalReviewWorkflow": "# INCREMENTAL REVIEW MODE\n\nThis PR was previously reviewed at commit `{PREVIOUS_SHA}`. The previous review findings are provided below so you can focus on what changed.\n\n## Previous Review Summary\n{PREVIOUS_SUMMARY}\n\n## Previous Inline Comments ({ACTIVE_COMMENT_COUNT} active)\nSee the Existing Inline Comments table below.\n\n## Incremental Workflow\n\n### Step 1: View ONLY the incremental diff\n```bash\ngit pull\ngit diff {PREVIOUS_SHA}..HEAD\n```\n\nIf this command fails (e.g., force push rewrote history), FALL BACK to full review:\nuse `gh pr diff {PR_NUMBER}` and ignore previous context entirely.\n\n### Step 2: Analyze changed files\nFor EACH file in the incremental diff:\n- Read the FULL file for context\n- Check for ALL issue types: bugs, security problems, typos, logic errors, missing error handling, edge cases\n- Re-verify any previous issues on changed lines (they may be fixed)\n\nFor files NOT in the incremental diff: carry forward previous findings.\nDo NOT re-read or re-analyze unchanged files.\n\n### Step 3: Verify ALL Issues\n\nFor EACH potential issue you collected:\n1. **Read the actual line** - Use the Read tool\n2. **Confirm the issue exists** - The problem must be visible in the code\n3. **Check it's not already commented** - See Existing Comments table\n\n**Anti-hallucination:** ALWAYS read the actual line before commenting. If you think line 66 has a typo, READ line 66 first — the issue may not exist there.\n\n### Step 4: Submit inline comments (single API call)\nOnly post NEW issues. Do NOT duplicate existing comments.\nUse the Inline Comments API format in the COMMANDS section.\n\n**Skip this step if no NEW issues found.**\n\n### Step 5: Update Summary\nUpdate the summary to reflect ALL issues:\n- NEW issues from changed files\n- Previous issues on unchanged files (carried forward)\n- Mark resolved issues that were fixed in new commits", "fixLinkTemplate": "## Fix Link (include if issues found)\n\n[Fix these issues in Kilo Cloud]({FIX_LINK})", "styleGuidance": { "strict": "# STRICT REVIEW MODE\n\nYou are a thorough, detail-oriented code reviewer. Your job is to catch everything — not just obvious bugs, but edge cases, potential future issues, and code smell.\n\n## Rules\n\n1. **Flag ALL potential issues**, not just high-confidence ones. Lower your reporting threshold.\n2. **Err on the side of caution.** If something *might* be a problem, report it.\n3. **Prioritize quality and security above all.** Missing error handling, missing input validation, and missing type safety are all worth flagging.\n4. **Include edge cases and future risks.** If a pattern could break under reasonable future changes, mention it.\n5. **Be direct and professional.** No sugar-coating — state the issue clearly and explain the risk.", diff --git a/src/lib/code-reviews/prompts/generate-prompt.test.ts b/src/lib/code-reviews/prompts/generate-prompt.test.ts index 37f4cac49a..529f93ee40 100644 --- a/src/lib/code-reviews/prompts/generate-prompt.test.ts +++ b/src/lib/code-reviews/prompts/generate-prompt.test.ts @@ -1,6 +1,6 @@ import type { CodeReviewAgentConfig } from '@/lib/agent-config/core/types'; import { resolveTemplate, generateReviewPrompt } from './generate-prompt'; -import type { PromptTemplate } from './generate-prompt'; +import type { PromptTemplate, ExistingReviewState } from './generate-prompt'; // --- Fixtures --- @@ -204,3 +204,115 @@ describe('generateReviewPrompt', () => { expect(prompt).not.toContain('ROAST MODE ACTIVATED'); }); }); + +// --- Incremental review --- + +const existingReviewStateWithSummary: ExistingReviewState = { + summaryComment: { + commentId: 123, + body: '\n## Code Review Summary\n\n**Status:** 2 Issues Found', + }, + inlineComments: [ + { id: 1, path: 'src/foo.ts', line: 10, body: '**WARNING:** Issue one', isOutdated: false }, + { id: 2, path: 'src/bar.ts', line: 20, body: '**CRITICAL:** Issue two', isOutdated: true }, + ], + previousStatus: 'issues-found', + headCommitSha: 'currentsha123', +}; + +const existingReviewStateNoSummary: ExistingReviewState = { + summaryComment: null, + inlineComments: [], + previousStatus: 'no-review', + headCommitSha: 'currentsha123', +}; + +describe('generateReviewPrompt (incremental review)', () => { + it('uses incremental workflow when previousHeadSha and summary comment are provided', async () => { + const { prompt } = await generateReviewPrompt(baseConfig, 'owner/repo', 42, { + reviewId: 'review-123', + existingReviewState: existingReviewStateWithSummary, + previousHeadSha: 'abc123prev', + }); + + expect(prompt).toContain('INCREMENTAL REVIEW MODE'); + expect(prompt).toContain('abc123prev'); + expect(prompt).toContain('git diff abc123prev..HEAD'); + expect(prompt).toContain('2 Issues Found'); + // Should contain the active comment count (1 active, 1 outdated) + expect(prompt).toContain('1 active'); + // Should NOT contain the standard workflow step 1 + expect(prompt).not.toContain('gh pr diff 42\n```'); + }); + + it('uses standard workflow when previousHeadSha is null', async () => { + const { prompt } = await generateReviewPrompt(baseConfig, 'owner/repo', 42, { + reviewId: 'review-123', + existingReviewState: existingReviewStateWithSummary, + previousHeadSha: null, + }); + + expect(prompt).not.toContain('INCREMENTAL REVIEW MODE'); + expect(prompt).toContain('gh pr diff 42'); + }); + + it('uses standard workflow when previousHeadSha is provided but no summary comment', async () => { + const { prompt } = await generateReviewPrompt(baseConfig, 'owner/repo', 42, { + reviewId: 'review-123', + existingReviewState: existingReviewStateNoSummary, + previousHeadSha: 'abc123prev', + }); + + expect(prompt).not.toContain('INCREMENTAL REVIEW MODE'); + expect(prompt).toContain('gh pr diff 42'); + }); + + it('uses standard workflow when existingReviewState is null', async () => { + const { prompt } = await generateReviewPrompt(baseConfig, 'owner/repo', 42, { + reviewId: 'review-123', + existingReviewState: null, + previousHeadSha: 'abc123prev', + }); + + expect(prompt).not.toContain('INCREMENTAL REVIEW MODE'); + expect(prompt).toContain('gh pr diff 42'); + }); + + it('still includes existing inline comments table in incremental mode', async () => { + const { prompt } = await generateReviewPrompt(baseConfig, 'owner/repo', 42, { + reviewId: 'review-123', + existingReviewState: existingReviewStateWithSummary, + previousHeadSha: 'abc123prev', + }); + + // The inline comments table should still be present (section 10 in generate-prompt.ts) + expect(prompt).toContain('Existing Inline Comments'); + expect(prompt).toContain('src/foo.ts'); + }); + + it('uses UPDATE summary command in incremental mode', async () => { + const { prompt } = await generateReviewPrompt(baseConfig, 'owner/repo', 42, { + reviewId: 'review-123', + existingReviewState: existingReviewStateWithSummary, + previousHeadSha: 'abc123prev', + }); + + // Summary command should be UPDATE (since summaryComment exists) + expect(prompt).toContain('UPDATE existing comment'); + expect(prompt).toContain('123'); // commentId + }); + + it('works with GitLab platform in incremental mode', async () => { + const { prompt } = await generateReviewPrompt(baseConfig, 'group/project', 10, { + reviewId: 'review-456', + existingReviewState: existingReviewStateWithSummary, + platform: 'gitlab', + gitlabContext: { baseSha: 'base123', startSha: 'start123', headSha: 'head123' }, + previousHeadSha: 'prevsha456', + }); + + expect(prompt).toContain('INCREMENTAL REVIEW MODE'); + expect(prompt).toContain('prevsha456'); + expect(prompt).toContain('glab mr diff'); + }); +}); diff --git a/src/lib/code-reviews/prompts/generate-prompt.ts b/src/lib/code-reviews/prompts/generate-prompt.ts index bf050088db..aff24aaa6e 100644 --- a/src/lib/code-reviews/prompts/generate-prompt.ts +++ b/src/lib/code-reviews/prompts/generate-prompt.ts @@ -65,6 +65,8 @@ const PromptTemplateSchema = z.object({ summaryCommandUpdate: z.string(), inlineCommentsApi: z.string(), fixLinkTemplate: z.string(), + // Incremental review workflow (used instead of `workflow` when a previous review exists) + incrementalReviewWorkflow: z.string().optional(), // Per-style overrides (optional — only needed for non-default styles like roast) styleGuidance: z.record(z.string(), z.string()).optional(), commentFormatOverrides: z.record(z.string(), z.string()).optional(), @@ -122,6 +124,8 @@ export function resolveTemplate( return { template: { ...remoteTemplate, + incrementalReviewWorkflow: + remoteTemplate.incrementalReviewWorkflow ?? localTemplate.incrementalReviewWorkflow, styleGuidance: mergeStyleOverrides(localTemplate.styleGuidance, remoteTemplate.styleGuidance), commentFormatOverrides: mergeStyleOverrides( localTemplate.commentFormatOverrides, @@ -171,26 +175,43 @@ export type GitLabDiffContext = { headSha: string; }; +/** + * Optional parameters for prompt generation + */ +export type GenerateReviewPromptOptions = { + /** Code review ID for generating fix link */ + reviewId?: string; + /** Complete review state for intelligent decisions */ + existingReviewState?: ExistingReviewState | null; + /** Platform type (defaults to 'github') */ + platform?: CodeReviewPlatform; + /** GitLab-specific diff context for inline comments */ + gitlabContext?: GitLabDiffContext; + /** HEAD SHA from a previous completed review (enables incremental mode) */ + previousHeadSha?: string | null; +}; + /** * Generates a code review prompt based on configuration * @param config Agent configuration with review settings * @param repository Repository in format "owner/repo" (GitHub) or "namespace/project" (GitLab) * @param prNumber Pull request number (GitHub) or merge request IID (GitLab) - * @param reviewId Code review ID for generating fix link (optional) - * @param existingReviewState Complete review state for intelligent decisions (optional) - * @param platform Platform type (defaults to 'github' for backward compatibility) - * @param gitlabContext GitLab-specific diff context for inline comments (optional) + * @param options Optional parameters for review context, platform, and incremental mode * @returns Generated prompt with version and source info */ export async function generateReviewPrompt( config: CodeReviewAgentConfig, repository: string, prNumber?: number, - reviewId?: string, - existingReviewState?: ExistingReviewState | null, - platform: CodeReviewPlatform = 'github', - gitlabContext?: GitLabDiffContext + options: GenerateReviewPromptOptions = {} ): Promise<{ prompt: string; version: string; source: 'posthog' | 'local' }> { + const { + reviewId, + existingReviewState, + platform = 'github', + gitlabContext, + previousHeadSha, + } = options; // Load template from PostHog (remote) or local fallback const { template, source } = await loadPromptTemplate(platform); const platformConfig = getPlatformConfig(platform); @@ -240,7 +261,21 @@ export async function generateReviewPrompt( prompt += template.hardConstraints + '\n\n'; // 5. Workflow with placeholders replaced - prompt += replacePlaceholders(template.workflow) + '\n\n'; + // Use incremental workflow when we have a previous completed review SHA and a summary comment + if ( + previousHeadSha && + template.incrementalReviewWorkflow && + existingReviewState?.summaryComment + ) { + const activeCount = existingReviewState.inlineComments?.filter(c => !c.isOutdated).length ?? 0; + const incrementalWorkflow = template.incrementalReviewWorkflow + .replace(/{PREVIOUS_SHA}/g, previousHeadSha) + .replace(/{PREVIOUS_SUMMARY}/g, existingReviewState.summaryComment.body) + .replace(/{ACTIVE_COMMENT_COUNT}/g, String(activeCount)); + prompt += replacePlaceholders(incrementalWorkflow) + '\n\n'; + } else { + prompt += replacePlaceholders(template.workflow) + '\n\n'; + } // 6. What to review prompt += template.whatToReview + '\n\n'; diff --git a/src/lib/code-reviews/triggers/prepare-review-payload.ts b/src/lib/code-reviews/triggers/prepare-review-payload.ts index 405597c530..e0ad075c8d 100644 --- a/src/lib/code-reviews/triggers/prepare-review-payload.ts +++ b/src/lib/code-reviews/triggers/prepare-review-payload.ts @@ -36,10 +36,12 @@ import type { GitLabDiffContext, } from '../prompts/generate-prompt'; import { getIntegrationById } from '@/lib/integrations/db/platform-integrations'; -import { getCodeReviewById } from '../db/code-reviews'; +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, isActiveReviewPromo, REVIEW_PROMO_START, REVIEW_PROMO_END, @@ -50,7 +52,6 @@ import type { CodeReviewAgentConfig } from '@/lib/agent-config/core/types'; import { logExceptInTest, errorExceptInTest } from '@/lib/utils.server'; import type { CodeReviewPlatform } from '../core/schemas'; import { PLATFORM } from '@/lib/integrations/core/constants'; -import { isFeatureFlagEnabled } from '@/lib/posthog-feature-flags'; export type PreparePayloadParams = { reviewId: string; @@ -93,6 +94,8 @@ export type CodeReviewPayload = { skipBalanceCheck?: boolean; /** Which cloud agent backend to use: 'v1' (cloud-agent SSE) or 'v2' (cloud-agent-next) */ agentVersion?: string; + /** Cloud-agent session ID from a previous completed review, for session continuation */ + previousCloudAgentSessionId?: string; }; /** @@ -314,18 +317,62 @@ export async function prepareReviewPayload( } } - // 4. Generate auth token for cloud agent with bot identifier + // 4. Check for previous completed review (incremental review optimization) + // Both previousHeadSha (for diff base) and previousCloudAgentSessionId (for session + // 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); + + 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, + } + ); + } + } 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 const authToken = generateApiToken(user, { botId: 'reviewer' }); - // 5. Generate dynamic review prompt (include reviewId for fix link and review state) + // 6. Generate dynamic review prompt const { prompt, version, source } = await generateReviewPrompt( agentConfig.config as CodeReviewAgentConfig, review.repo_full_name, review.pr_number, - reviewId, - existingReviewState, - platform, - gitlabContext + { + reviewId, + existingReviewState, + platform, + gitlabContext, + previousHeadSha, + } ); logExceptInTest('[prepareReviewPayload] Generated prompt:', { @@ -336,7 +383,7 @@ export async function prepareReviewPayload( promptLength: prompt.length, }); - // 6. Prepare session input + // 7. Prepare session input // Note: cloud-agent automatically sets GH_TOKEN/GITLAB_TOKEN from token parameters const config = agentConfig.config as CodeReviewAgentConfig; @@ -406,12 +453,13 @@ export async function prepareReviewPayload( }); } - // 7. Build complete payload + // 8. Build complete payload const payload: CodeReviewPayload = { reviewId, authToken, sessionInput, owner, + previousCloudAgentSessionId, }; logExceptInTest('[prepareReviewPayload] Prepared payload', {