From 42c6d017a3917ba9a00e41a5c8ee24f0f015ad41 Mon Sep 17 00:00:00 2001 From: Alex Alecu Date: Mon, 9 Mar 2026 10:48:24 +0200 Subject: [PATCH 01/24] add findPreviousCompletedReview DB query --- src/lib/code-reviews/db/code-reviews.ts | 35 +++++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/src/lib/code-reviews/db/code-reviews.ts b/src/lib/code-reviews/db/code-reviews.ts index c87a411b3b..b0cc7f1f47 100644 --- a/src/lib/code-reviews/db/code-reviews.ts +++ b/src/lib/code-reviews/db/code-reviews.ts @@ -419,6 +419,41 @@ 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. + */ +export async function findPreviousCompletedReview( + repoFullName: string, + prNumber: number, + excludeSha: string +): Promise<{ head_sha: string } | null> { + try { + const [review] = await db + .select({ head_sha: cloud_agent_code_reviews.head_sha }) + .from(cloud_agent_code_reviews) + .where( + and( + eq(cloud_agent_code_reviews.repo_full_name, repoFullName), + eq(cloud_agent_code_reviews.pr_number, prNumber), + ne(cloud_agent_code_reviews.head_sha, excludeSha), + eq(cloud_agent_code_reviews.status, 'completed') + ) + ) + .orderBy(desc(cloud_agent_code_reviews.completed_at)) + .limit(1); + + return review || null; + } catch (error) { + captureException(error, { + tags: { operation: 'findPreviousCompletedReview' }, + extra: { repoFullName, prNumber, excludeSha }, + }); + throw error; + } +} + /** * Verifies that a user owns (or is a member of the org that owns) a code review * Returns true if the user has access, false otherwise From 5d502ab1380fb0cd2d38a2ad00d6f35ec217b475 Mon Sep 17 00:00:00 2001 From: Alex Alecu Date: Mon, 9 Mar 2026 10:48:27 +0200 Subject: [PATCH 02/24] add incremental workflow to prompt templates --- src/lib/code-reviews/prompts/default-prompt-template-gitlab.json | 1 + src/lib/code-reviews/prompts/default-prompt-template.json | 1 + 2 files changed, 2 insertions(+) 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 0c78c37f22..d6fa075c92 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 fetch origin\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 3be1ed2baa..0b3bc25619 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 fetch origin\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.", From 42633565f1fd38d7ff2b254ee2c73fde56d7f6b3 Mon Sep 17 00:00:00 2001 From: Alex Alecu Date: Mon, 9 Mar 2026 10:48:32 +0200 Subject: [PATCH 03/24] add feature flag constant for incremental reviews --- src/lib/code-reviews/core/constants.ts | 7 +++++++ 1 file changed, 7 insertions(+) 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 // ============================================================================ From ea839cdc7eea163744f5b959d6123993966af37d Mon Sep 17 00:00:00 2001 From: Alex Alecu Date: Mon, 9 Mar 2026 10:49:16 +0200 Subject: [PATCH 04/24] refactor generateReviewPrompt to use options object --- .../code-reviews/prompts/generate-prompt.ts | 33 ++++++++++++++----- 1 file changed, 25 insertions(+), 8 deletions(-) diff --git a/src/lib/code-reviews/prompts/generate-prompt.ts b/src/lib/code-reviews/prompts/generate-prompt.ts index bf050088db..8682aaf568 100644 --- a/src/lib/code-reviews/prompts/generate-prompt.ts +++ b/src/lib/code-reviews/prompts/generate-prompt.ts @@ -171,26 +171,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); From 02e0b113cc143fb8914d1b112d99f229ae7c8b9f Mon Sep 17 00:00:00 2001 From: Alex Alecu Date: Mon, 9 Mar 2026 10:49:22 +0200 Subject: [PATCH 05/24] add incremental workflow to prompt generation --- .../code-reviews/prompts/generate-prompt.ts | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/src/lib/code-reviews/prompts/generate-prompt.ts b/src/lib/code-reviews/prompts/generate-prompt.ts index 8682aaf568..3c519ca8b8 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(), @@ -257,7 +259,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'; From d909df0f62c59026f639b100e8c1c1104f984a82 Mon Sep 17 00:00:00 2001 From: Alex Alecu Date: Mon, 9 Mar 2026 10:49:28 +0200 Subject: [PATCH 06/24] wire up incremental review in payload preparation --- .../triggers/prepare-review-payload.ts | 48 ++++++++++++++++--- 1 file changed, 41 insertions(+), 7 deletions(-) diff --git a/src/lib/code-reviews/triggers/prepare-review-payload.ts b/src/lib/code-reviews/triggers/prepare-review-payload.ts index f1f3ed3e7e..efd40d67a4 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, @@ -311,18 +313,50 @@ export async function prepareReviewPayload( } } - // 4. Generate auth token for cloud agent with bot identifier + // 4. Check for previous completed review (incremental review optimization) + let previousHeadSha: string | null = null; + const incrementalEnabled = await isFeatureFlagEnabled(FEATURE_FLAG_INCREMENTAL_REVIEW); + + if (incrementalEnabled) { + try { + const previousReview = await findPreviousCompletedReview( + review.repo_full_name, + review.pr_number, + review.head_sha + ); + previousHeadSha = previousReview?.head_sha ?? null; + + if (previousHeadSha) { + logExceptInTest('[prepareReviewPayload] Found previous completed review for incremental mode', { + reviewId, + previousHeadSha: previousHeadSha.substring(0, 8), + currentHeadSha: review.head_sha.substring(0, 8), + }); + } + } 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 (include reviewId for fix link and review state) 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:', { From f787f241aa57f3517bd82f767467e2c368d6ab65 Mon Sep 17 00:00:00 2001 From: Alex Alecu Date: Mon, 9 Mar 2026 10:49:32 +0200 Subject: [PATCH 07/24] add tests for incremental review prompt generation --- .../prompts/generate-prompt.test.ts | 114 +++++++++++++++++- 1 file changed, 113 insertions(+), 1 deletion(-) 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'); + }); +}); From b2bff33f36e49ef61f31b647b96964e006d6bfd9 Mon Sep 17 00:00:00 2001 From: Alex Alecu Date: Mon, 9 Mar 2026 10:59:54 +0200 Subject: [PATCH 08/24] backfill incrementalReviewWorkflow from local template when remote omits it --- src/lib/code-reviews/prompts/generate-prompt.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/lib/code-reviews/prompts/generate-prompt.ts b/src/lib/code-reviews/prompts/generate-prompt.ts index 3c519ca8b8..aff24aaa6e 100644 --- a/src/lib/code-reviews/prompts/generate-prompt.ts +++ b/src/lib/code-reviews/prompts/generate-prompt.ts @@ -124,6 +124,8 @@ export function resolveTemplate( return { template: { ...remoteTemplate, + incrementalReviewWorkflow: + remoteTemplate.incrementalReviewWorkflow ?? localTemplate.incrementalReviewWorkflow, styleGuidance: mergeStyleOverrides(localTemplate.styleGuidance, remoteTemplate.styleGuidance), commentFormatOverrides: mergeStyleOverrides( localTemplate.commentFormatOverrides, From 9ab2dbbff8a7e3098f02c1d1beec2fdafe26a245 Mon Sep 17 00:00:00 2001 From: Alex Alecu Date: Mon, 9 Mar 2026 11:00:14 +0200 Subject: [PATCH 09/24] filter by platform in findPreviousCompletedReview --- src/lib/code-reviews/db/code-reviews.ts | 6 ++++-- src/lib/code-reviews/triggers/prepare-review-payload.ts | 3 ++- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/src/lib/code-reviews/db/code-reviews.ts b/src/lib/code-reviews/db/code-reviews.ts index b0cc7f1f47..5030edce5a 100644 --- a/src/lib/code-reviews/db/code-reviews.ts +++ b/src/lib/code-reviews/db/code-reviews.ts @@ -427,7 +427,8 @@ export async function findActiveReviewsForPR( export async function findPreviousCompletedReview( repoFullName: string, prNumber: number, - excludeSha: string + excludeSha: string, + platform: string = 'github' ): Promise<{ head_sha: string } | null> { try { const [review] = await db @@ -437,6 +438,7 @@ export async function findPreviousCompletedReview( 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') ) @@ -448,7 +450,7 @@ export async function findPreviousCompletedReview( } catch (error) { captureException(error, { tags: { operation: 'findPreviousCompletedReview' }, - extra: { repoFullName, prNumber, excludeSha }, + extra: { repoFullName, prNumber, excludeSha, platform }, }); throw error; } diff --git a/src/lib/code-reviews/triggers/prepare-review-payload.ts b/src/lib/code-reviews/triggers/prepare-review-payload.ts index efd40d67a4..3728510fed 100644 --- a/src/lib/code-reviews/triggers/prepare-review-payload.ts +++ b/src/lib/code-reviews/triggers/prepare-review-payload.ts @@ -322,7 +322,8 @@ export async function prepareReviewPayload( const previousReview = await findPreviousCompletedReview( review.repo_full_name, review.pr_number, - review.head_sha + review.head_sha, + platform ); previousHeadSha = previousReview?.head_sha ?? null; From 8940acdf71669a889b9ed6d28ea4d045f1a8e9ae Mon Sep 17 00:00:00 2001 From: Alex Alecu Date: Mon, 9 Mar 2026 11:00:23 +0200 Subject: [PATCH 10/24] order by created_at instead of completed_at in previous review lookup --- src/lib/code-reviews/db/code-reviews.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lib/code-reviews/db/code-reviews.ts b/src/lib/code-reviews/db/code-reviews.ts index 5030edce5a..f68e427a87 100644 --- a/src/lib/code-reviews/db/code-reviews.ts +++ b/src/lib/code-reviews/db/code-reviews.ts @@ -443,7 +443,7 @@ export async function findPreviousCompletedReview( eq(cloud_agent_code_reviews.status, 'completed') ) ) - .orderBy(desc(cloud_agent_code_reviews.completed_at)) + .orderBy(desc(cloud_agent_code_reviews.created_at)) .limit(1); return review || null; From 48f84552204b952fa0abfe631d5914756d6c78de Mon Sep 17 00:00:00 2001 From: Alex Alecu Date: Mon, 9 Mar 2026 11:00:46 +0200 Subject: [PATCH 11/24] use git pull instead of git fetch in incremental workflow --- .../code-reviews/prompts/default-prompt-template-gitlab.json | 2 +- src/lib/code-reviews/prompts/default-prompt-template.json | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) 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 d6fa075c92..23f3154f51 100644 --- a/src/lib/code-reviews/prompts/default-prompt-template-gitlab.json +++ b/src/lib/code-reviews/prompts/default-prompt-template-gitlab.json @@ -11,7 +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 fetch origin\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", + "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 0b3bc25619..9f748b0aa0 100644 --- a/src/lib/code-reviews/prompts/default-prompt-template.json +++ b/src/lib/code-reviews/prompts/default-prompt-template.json @@ -11,7 +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 fetch origin\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", + "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.", From 2f1a7b7f99d036c4ea4ccb3147cf5cc5442e11c7 Mon Sep 17 00:00:00 2001 From: Alex Alecu Date: Mon, 9 Mar 2026 11:00:54 +0200 Subject: [PATCH 12/24] exclude live PR head SHA instead of stored review SHA --- src/lib/code-reviews/triggers/prepare-review-payload.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lib/code-reviews/triggers/prepare-review-payload.ts b/src/lib/code-reviews/triggers/prepare-review-payload.ts index 3728510fed..17065ddc76 100644 --- a/src/lib/code-reviews/triggers/prepare-review-payload.ts +++ b/src/lib/code-reviews/triggers/prepare-review-payload.ts @@ -322,7 +322,7 @@ export async function prepareReviewPayload( const previousReview = await findPreviousCompletedReview( review.repo_full_name, review.pr_number, - review.head_sha, + existingReviewState?.headCommitSha ?? review.head_sha, platform ); previousHeadSha = previousReview?.head_sha ?? null; From ce6cc9d47ffd54bd6093e712892c96394c7b856a Mon Sep 17 00:00:00 2001 From: Alex Alecu Date: Mon, 9 Mar 2026 11:01:13 +0200 Subject: [PATCH 13/24] fix duplicate step numbering in prepareReviewPayload --- src/lib/code-reviews/triggers/prepare-review-payload.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/lib/code-reviews/triggers/prepare-review-payload.ts b/src/lib/code-reviews/triggers/prepare-review-payload.ts index 17065ddc76..53e3beaa72 100644 --- a/src/lib/code-reviews/triggers/prepare-review-payload.ts +++ b/src/lib/code-reviews/triggers/prepare-review-payload.ts @@ -346,7 +346,7 @@ export async function prepareReviewPayload( // 5. Generate auth token for cloud agent with bot identifier const authToken = generateApiToken(user, { botId: 'reviewer' }); - // 6. 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, @@ -368,7 +368,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; @@ -430,7 +430,7 @@ export async function prepareReviewPayload( }); } - // 7. Build complete payload + // 8. Build complete payload const payload: CodeReviewPayload = { reviewId, authToken, From 622ed7e3488da6e3ff70942c3bf85bba39f1daf6 Mon Sep 17 00:00:00 2001 From: Alex Alecu Date: Mon, 9 Mar 2026 13:52:15 +0200 Subject: [PATCH 14/24] format prepare-review-payload.ts --- .../triggers/prepare-review-payload.ts | 24 ++++++++++++------- 1 file changed, 15 insertions(+), 9 deletions(-) diff --git a/src/lib/code-reviews/triggers/prepare-review-payload.ts b/src/lib/code-reviews/triggers/prepare-review-payload.ts index 53e3beaa72..601eb35105 100644 --- a/src/lib/code-reviews/triggers/prepare-review-payload.ts +++ b/src/lib/code-reviews/triggers/prepare-review-payload.ts @@ -328,18 +328,24 @@ export async function prepareReviewPayload( previousHeadSha = previousReview?.head_sha ?? null; if (previousHeadSha) { - logExceptInTest('[prepareReviewPayload] Found previous completed review for incremental mode', { - reviewId, - previousHeadSha: previousHeadSha.substring(0, 8), - currentHeadSha: review.head_sha.substring(0, 8), - }); + logExceptInTest( + '[prepareReviewPayload] Found previous completed review for incremental mode', + { + reviewId, + previousHeadSha: previousHeadSha.substring(0, 8), + currentHeadSha: review.head_sha.substring(0, 8), + } + ); } } catch (error) { // Non-critical - fall back to full review - logExceptInTest('[prepareReviewPayload] Failed to fetch previous review, falling back to full review:', { - reviewId, - error, - }); + logExceptInTest( + '[prepareReviewPayload] Failed to fetch previous review, falling back to full review:', + { + reviewId, + error, + } + ); } } From 8ed952f60096a4e0f2012cfe21422eee2f36d6c2 Mon Sep 17 00:00:00 2001 From: Alex Alecu Date: Mon, 9 Mar 2026 19:23:15 +0200 Subject: [PATCH 15/24] accept callbackTarget in sendMessageV2 Allow follow-up executions to override the stored callback URL. The DO updates its metadata before executing, so completion notifications route to the new review's endpoint. --- cloud-agent-next/src/execution/types.ts | 2 ++ .../src/persistence/CloudAgentSession.ts | 23 +++++++++++++++++++ .../src/router/handlers/session-execution.ts | 1 + cloud-agent-next/src/router/schemas.ts | 3 +++ 4 files changed, 29 insertions(+) diff --git a/cloud-agent-next/src/execution/types.ts b/cloud-agent-next/src/execution/types.ts index f91fcb2f47..735589a851 100644 --- a/cloud-agent-next/src/execution/types.ts +++ b/cloud-agent-next/src/execution/types.ts @@ -12,6 +12,7 @@ import type { AgentMode } from '../schema.js'; import type { Images, EncryptedSecrets as SchemaEncryptedSecrets } from '../router/schemas.js'; import type { EncryptedSecrets } from '../utils/encryption.js'; import type { MCPServerConfig } from '../persistence/types.js'; +import type { CallbackTarget } from '../callbacks/types.js'; // --------------------------------------------------------------------------- // Execution Modes @@ -155,6 +156,7 @@ type FollowupExecutionRequest = BaseExecutionRequest & { githubToken?: string; gitToken?: string; }; + callbackTarget?: CallbackTarget; }; /** diff --git a/cloud-agent-next/src/persistence/CloudAgentSession.ts b/cloud-agent-next/src/persistence/CloudAgentSession.ts index cf71e428dd..42d4ce525d 100644 --- a/cloud-agent-next/src/persistence/CloudAgentSession.ts +++ b/cloud-agent-next/src/persistence/CloudAgentSession.ts @@ -621,6 +621,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. @@ -1811,6 +1830,10 @@ export class CloudAgentSession extends DurableObject { await this.updateGitToken(request.tokenOverrides.gitToken); metadata.gitToken = request.tokenOverrides.gitToken; } + if (request.callbackTarget) { + await this.updateCallbackTarget(request.callbackTarget); + metadata.callbackTarget = request.callbackTarget; + } const mode = (request.mode ?? metadata.mode ?? 'code') as ExecutionMode; const model = normalizeKilocodeModel(request.model ?? metadata.model); diff --git a/cloud-agent-next/src/router/handlers/session-execution.ts b/cloud-agent-next/src/router/handlers/session-execution.ts index 3366d74703..7dcb3fd86d 100644 --- a/cloud-agent-next/src/router/handlers/session-execution.ts +++ b/cloud-agent-next/src/router/handlers/session-execution.ts @@ -177,6 +177,7 @@ export function createSessionExecutionV2Handlers() { githubToken: input.githubToken, gitToken: input.gitToken, }, + callbackTarget: input.callbackTarget, }; const startResult = await withDORetry< diff --git a/cloud-agent-next/src/router/schemas.ts b/cloud-agent-next/src/router/schemas.ts index 24f3633a79..ced0e314fd 100644 --- a/cloud-agent-next/src/router/schemas.ts +++ b/cloud-agent-next/src/router/schemas.ts @@ -109,6 +109,9 @@ export const SendMessageV2Input = z images: ImagesSchema.optional().describe( 'Optional image attachments to download from R2 to the sandbox' ), + callbackTarget: CallbackTargetSchema.optional().describe( + 'Optional callback target override - updates the stored callback before executing' + ), }) .extend(PromptPayload.shape) .refine(rejectCustomMode, { From 752e04234032799e181a2bceb3d030a15a588cd2 Mon Sep 17 00:00:00 2001 From: Alex Alecu Date: Mon, 9 Mar 2026 19:23:27 +0200 Subject: [PATCH 16/24] look up previous session ID for session continuation Add findPreviousCompletedReviewSession query that returns the cloud-agent session ID from the most recent completed review for the same PR. Include previousCloudAgentSessionId in the CodeReviewPayload so the orchestrator can reuse it. --- src/lib/code-reviews/db/code-reviews.ts | 44 ++++++++++++++++++- .../triggers/prepare-review-payload.ts | 43 +++++++++++++++++- 2 files changed, 85 insertions(+), 2 deletions(-) diff --git a/src/lib/code-reviews/db/code-reviews.ts b/src/lib/code-reviews/db/code-reviews.ts index f68e427a87..175a49a6e9 100644 --- a/src/lib/code-reviews/db/code-reviews.ts +++ b/src/lib/code-reviews/db/code-reviews.ts @@ -7,7 +7,7 @@ import { db } from '@/lib/drizzle'; import { cloud_agent_code_reviews } from '@kilocode/db/schema'; -import { eq, and, desc, count, ne, inArray } from 'drizzle-orm'; +import { eq, and, desc, count, ne, inArray, isNotNull } from 'drizzle-orm'; import { captureException } from '@sentry/nextjs'; import type { CreateReviewParams, CodeReviewStatus, ListReviewsParams, Owner } from '../core'; import type { CloudAgentCodeReview } from '@kilocode/db/schema'; @@ -456,6 +456,48 @@ export async function findPreviousCompletedReview( } } +/** + * Like findPreviousCompletedReview, but also returns the session_id. + * Used for session continuation: reuses the cloud-agent session on follow-up pushes. + * Only returns reviews that have a non-null session_id. + */ +export async function findPreviousCompletedReviewSession( + repoFullName: string, + prNumber: number, + excludeSha: string, + platform: string = 'github' +): Promise<{ head_sha: string; session_id: string } | 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'), + isNotNull(cloud_agent_code_reviews.session_id) + ) + ) + .orderBy(desc(cloud_agent_code_reviews.created_at)) + .limit(1); + + if (!review || !review.session_id) return null; + return { head_sha: review.head_sha, session_id: review.session_id }; + } catch (error) { + captureException(error, { + tags: { operation: 'findPreviousCompletedReviewSession' }, + extra: { repoFullName, prNumber, excludeSha, platform }, + }); + throw error; + } +} + /** * Verifies that a user owns (or is a member of the org that owns) a code review * Returns true if the user has access, false otherwise diff --git a/src/lib/code-reviews/triggers/prepare-review-payload.ts b/src/lib/code-reviews/triggers/prepare-review-payload.ts index 601eb35105..e863128978 100644 --- a/src/lib/code-reviews/triggers/prepare-review-payload.ts +++ b/src/lib/code-reviews/triggers/prepare-review-payload.ts @@ -36,7 +36,11 @@ import type { GitLabDiffContext, } from '../prompts/generate-prompt'; import { getIntegrationById } from '@/lib/integrations/db/platform-integrations'; -import { getCodeReviewById, findPreviousCompletedReview } from '../db/code-reviews'; +import { + getCodeReviewById, + findPreviousCompletedReview, + findPreviousCompletedReviewSession, +} from '../db/code-reviews'; import { isFeatureFlagEnabled } from '@/lib/posthog-feature-flags'; import { DEFAULT_CODE_REVIEW_MODEL, @@ -92,6 +96,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; }; /** @@ -349,6 +355,40 @@ export async function prepareReviewPayload( } } + // 4b. Look up previous session ID for session continuation + let previousCloudAgentSessionId: string | undefined; + if (incrementalEnabled) { + try { + const previousSession = await findPreviousCompletedReviewSession( + review.repo_full_name, + review.pr_number, + existingReviewState?.headCommitSha ?? review.head_sha, + platform + ); + previousCloudAgentSessionId = previousSession?.session_id; + + if (previousCloudAgentSessionId) { + logExceptInTest( + '[prepareReviewPayload] Found previous session for continuation', + { + reviewId, + previousCloudAgentSessionId, + previousHeadSha: previousSession?.head_sha.substring(0, 8), + } + ); + } + } catch (error) { + // Non-critical - fall back to fresh session + logExceptInTest( + '[prepareReviewPayload] Failed to fetch previous session, falling back to fresh session:', + { + reviewId, + error, + } + ); + } + } + // 5. Generate auth token for cloud agent with bot identifier const authToken = generateApiToken(user, { botId: 'reviewer' }); @@ -442,6 +482,7 @@ export async function prepareReviewPayload( authToken, sessionInput, owner, + previousCloudAgentSessionId, }; logExceptInTest('[prepareReviewPayload] Prepared payload', { From d286a6f6020e8b9666233422001854bc1f2b9f69 Mon Sep 17 00:00:00 2001 From: Alex Alecu Date: Mon, 9 Mar 2026 19:23:39 +0200 Subject: [PATCH 17/24] add sendMessageV2 follow-up path to orchestrator On follow-up pushes, reuse the existing cloud-agent session via sendMessageV2 instead of creating a fresh one. This preserves conversation context and avoids re-cloning the repo. Falls back to prepareSession + initiate if sendMessageV2 fails (session expired, sandbox evicted, execution in progress, etc). --- .../src/code-review-orchestrator.ts | 131 +++++++++++++++++- cloudflare-code-review-infra/src/index.ts | 1 + cloudflare-code-review-infra/src/types.ts | 4 + 3 files changed, 135 insertions(+), 1 deletion(-) diff --git a/cloudflare-code-review-infra/src/code-review-orchestrator.ts b/cloudflare-code-review-infra/src/code-review-orchestrator.ts index ffbbaa63bb..ad471180b4 100644 --- a/cloudflare-code-review-infra/src/code-review-orchestrator.ts +++ b/cloudflare-code-review-infra/src/code-review-orchestrator.ts @@ -361,6 +361,7 @@ export class CodeReviewOrchestrator extends DurableObject { }; skipBalanceCheck?: boolean; agentVersion?: string; + previousCloudAgentSessionId?: string; }): Promise<{ status: CodeReviewStatus }> { this.state = { reviewId: params.reviewId, @@ -371,6 +372,7 @@ export class CodeReviewOrchestrator extends DurableObject { updatedAt: new Date().toISOString(), skipBalanceCheck: params.skipBalanceCheck, agentVersion: params.agentVersion, + previousCloudAgentSessionId: params.previousCloudAgentSessionId, }; await this.saveState(); @@ -527,7 +529,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(); } @@ -703,6 +709,129 @@ 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!; + + console.log('[CodeReviewOrchestrator] Attempting session continuation via sendMessageV2', { + reviewId: this.state.reviewId, + previousCloudAgentSessionId: previousSessionId, + }); + + try { + await this.updateStatus('running'); + + // Build headers for sendMessageV2 (protectedProcedure — Bearer token only) + const headers: Record = { + Authorization: `Bearer ${this.state.authToken}`, + 'Content-Type': 'application/json', + }; + if (this.state.skipBalanceCheck) { + headers['x-skip-balance-check'] = 'true'; + } + + 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 sendMessageInput = { + 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, + callbackTarget, + }; + + console.log('[CodeReviewOrchestrator] Calling sendMessageV2', { + reviewId: this.state.reviewId, + cloudAgentSessionId: previousSessionId, + callbackUrl: callbackTarget.url, + }); + + const response = await fetch(`${this.env.CLOUD_AGENT_NEXT_URL}/trpc/sendMessageV2`, { + method: 'POST', + headers, + body: JSON.stringify(sendMessageInput), + }); + + if (!response.ok) { + const errorText = await response.text(); + throw new Error(`sendMessageV2 failed (${response.status}): ${errorText}`); + } + + const result: Record = await response.json(); + const data = (result?.result as Record)?.data as + | Record + | undefined; + if (!data || typeof data.executionId !== 'string') { + throw new Error( + `Unexpected sendMessageV2 response shape: ${JSON.stringify(result).slice(0, 500)}` + ); + } + + // 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: data.executionId, + status: data.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 c2348dacea..07ee6a7f4d 100644 --- a/cloudflare-code-review-infra/src/types.ts +++ b/cloudflare-code-review-infra/src/types.ts @@ -63,6 +63,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 { @@ -91,6 +93,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 { From aaaea85af755b9c67b12f784c2bc7e11783cee6a Mon Sep 17 00:00:00 2001 From: Alex Alecu Date: Mon, 9 Mar 2026 19:23:47 +0200 Subject: [PATCH 18/24] add tests for findPreviousCompletedReviewSession --- src/lib/code-reviews/db/code-reviews.test.ts | 124 +++++++++++++++++++ 1 file changed, 124 insertions(+) create mode 100644 src/lib/code-reviews/db/code-reviews.test.ts 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..df80112581 --- /dev/null +++ b/src/lib/code-reviews/db/code-reviews.test.ts @@ -0,0 +1,124 @@ +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, + findPreviousCompletedReviewSession, +} from './code-reviews'; + +const REPO = `test-org/session-continuation-${Date.now()}`; + +describe('findPreviousCompletedReviewSession', () => { + let testUser: User; + const createdReviewIds: string[] = []; + + beforeAll(async () => { + testUser = await insertTestUser(); + }); + + afterAll(async () => { + // Clean up reviews then user + 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, + }); + createdReviewIds.push(id); + return id; + } + + it('returns null when no previous completed review exists', async () => { + const result = await findPreviousCompletedReviewSession(REPO, 42, 'abc123'); + expect(result).toBeNull(); + }); + + it('returns null when previous review is completed but has no session_id', async () => { + const id = await createReview('sha-no-session'); + await updateCodeReviewStatus(id, 'completed'); + + const result = await findPreviousCompletedReviewSession(REPO, 42, 'other-sha'); + expect(result).toBeNull(); + }); + + it('returns session_id and head_sha for a completed review with session_id', async () => { + const id = await createReview('sha-with-session'); + await updateCodeReviewStatus(id, 'completed', { + sessionId: 'agent_test123', + }); + + const result = await findPreviousCompletedReviewSession(REPO, 42, 'other-sha'); + expect(result).not.toBeNull(); + expect(result!.session_id).toBe('agent_test123'); + expect(result!.head_sha).toBe('sha-with-session'); + }); + + it('excludes the current SHA', async () => { + const result = await findPreviousCompletedReviewSession( + REPO, + 42, + 'sha-with-session' + ); + // Should not find itself — only reviews with a different SHA + // The "sha-no-session" review has no session_id, so only "sha-with-session" would match + // but it's excluded by excludeSha + expect(result).toBeNull(); + }); + + it('returns the most recent completed review with session_id', async () => { + const id = await createReview('sha-newer'); + await updateCodeReviewStatus(id, 'completed', { + sessionId: 'agent_newer', + }); + + const result = await findPreviousCompletedReviewSession(REPO, 42, 'other-sha'); + expect(result).not.toBeNull(); + expect(result!.session_id).toBe('agent_newer'); + expect(result!.head_sha).toBe('sha-newer'); + }); + + it('ignores non-completed reviews with session_id', 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 findPreviousCompletedReviewSession(REPO, 42, 'other-sha'); + expect(result).not.toBeNull(); + expect(result!.session_id).toBe('agent_newer'); + }); + + it('is consistent with findPreviousCompletedReview on head_sha', async () => { + const sessionResult = await findPreviousCompletedReviewSession( + REPO, + 42, + 'other-sha' + ); + const shaResult = await findPreviousCompletedReview(REPO, 42, 'other-sha'); + + // Both should find a completed review (sessionResult is more restrictive) + expect(shaResult).not.toBeNull(); + expect(sessionResult).not.toBeNull(); + // The head_sha from the session query should also appear in the sha-only query + expect(sessionResult!.head_sha).toBe(shaResult!.head_sha); + }); +}); From de20fc8c76a1fd092d26fbcc925fc0f621f485df Mon Sep 17 00:00:00 2001 From: Alex Alecu Date: Wed, 11 Mar 2026 12:09:23 +0200 Subject: [PATCH 19/24] fix: remove callbackTarget from user-facing sendMessageV2 endpoint callbackTarget on the protectedProcedure sendMessageV2 endpoint allowed any authenticated user to override the session callback URL, creating an SSRF vector. Move callback setup to the internal-only updateSession endpoint (requires x-internal-api-key). The orchestrator now calls updateSession before sendMessageV2 to set the callback securely. Also fixes merge conflicts: duplicate isFeatureFlagEnabled import and missing platform field in test helper. --- cloud-agent-next/src/execution/types.ts | 2 - .../src/persistence/CloudAgentSession.ts | 15 +++-- .../src/router/handlers/session-execution.ts | 1 - cloud-agent-next/src/router/schemas.ts | 3 - .../src/code-review-orchestrator.ts | 60 ++++++++++++------- src/lib/code-reviews/db/code-reviews.test.ts | 17 ++---- .../triggers/prepare-review-payload.ts | 14 ++--- 7 files changed, 58 insertions(+), 54 deletions(-) diff --git a/cloud-agent-next/src/execution/types.ts b/cloud-agent-next/src/execution/types.ts index 735589a851..f91fcb2f47 100644 --- a/cloud-agent-next/src/execution/types.ts +++ b/cloud-agent-next/src/execution/types.ts @@ -12,7 +12,6 @@ import type { AgentMode } from '../schema.js'; import type { Images, EncryptedSecrets as SchemaEncryptedSecrets } from '../router/schemas.js'; import type { EncryptedSecrets } from '../utils/encryption.js'; import type { MCPServerConfig } from '../persistence/types.js'; -import type { CallbackTarget } from '../callbacks/types.js'; // --------------------------------------------------------------------------- // Execution Modes @@ -156,7 +155,6 @@ type FollowupExecutionRequest = BaseExecutionRequest & { githubToken?: string; gitToken?: string; }; - callbackTarget?: CallbackTarget; }; /** diff --git a/cloud-agent-next/src/persistence/CloudAgentSession.ts b/cloud-agent-next/src/persistence/CloudAgentSession.ts index b79e189f88..239b75c3ad 100644 --- a/cloud-agent-next/src/persistence/CloudAgentSession.ts +++ b/cloud-agent-next/src/persistence/CloudAgentSession.ts @@ -804,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' }; } @@ -1951,11 +1959,6 @@ export class CloudAgentSession extends DurableObject { await this.updateGitToken(request.tokenOverrides.gitToken); metadata.gitToken = request.tokenOverrides.gitToken; } - if (request.callbackTarget) { - await this.updateCallbackTarget(request.callbackTarget); - metadata.callbackTarget = request.callbackTarget; - } - const mode = (request.mode ?? metadata.mode ?? 'code') as ExecutionMode; const model = normalizeKilocodeModel(request.model ?? metadata.model); const variant = request.variant ?? metadata.variant; diff --git a/cloud-agent-next/src/router/handlers/session-execution.ts b/cloud-agent-next/src/router/handlers/session-execution.ts index 7dcb3fd86d..3366d74703 100644 --- a/cloud-agent-next/src/router/handlers/session-execution.ts +++ b/cloud-agent-next/src/router/handlers/session-execution.ts @@ -177,7 +177,6 @@ export function createSessionExecutionV2Handlers() { githubToken: input.githubToken, gitToken: input.gitToken, }, - callbackTarget: input.callbackTarget, }; const startResult = await withDORetry< diff --git a/cloud-agent-next/src/router/schemas.ts b/cloud-agent-next/src/router/schemas.ts index 8659cf60e7..96991d37cc 100644 --- a/cloud-agent-next/src/router/schemas.ts +++ b/cloud-agent-next/src/router/schemas.ts @@ -109,9 +109,6 @@ export const SendMessageV2Input = z images: ImagesSchema.optional().describe( 'Optional image attachments to download from R2 to the sandbox' ), - callbackTarget: CallbackTargetSchema.optional().describe( - 'Optional callback target override - updates the stored callback before executing' - ), }) .extend(PromptPayload.shape) .refine(rejectCustomMode, { diff --git a/cloudflare-code-review-infra/src/code-review-orchestrator.ts b/cloudflare-code-review-infra/src/code-review-orchestrator.ts index ad471180b4..f0a07725c8 100644 --- a/cloudflare-code-review-infra/src/code-review-orchestrator.ts +++ b/cloudflare-code-review-infra/src/code-review-orchestrator.ts @@ -731,15 +731,19 @@ export class CodeReviewOrchestrator extends DurableObject { try { await this.updateStatus('running'); - // Build headers for sendMessageV2 (protectedProcedure — Bearer token only) - const headers: Record = { + // Build internal headers (internalApiProtectedProcedure — API key + Bearer token) + 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: 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: { @@ -747,6 +751,29 @@ export class CodeReviewOrchestrator extends DurableObject { }, }; + const updateResponse = await fetch(`${this.env.CLOUD_AGENT_NEXT_URL}/trpc/updateSession`, { + method: 'POST', + headers: internalHeaders, + body: JSON.stringify({ + cloudAgentSessionId: previousSessionId, + callbackTarget, + }), + }); + + if (!updateResponse.ok) { + const errorText = await updateResponse.text(); + throw new Error(`updateSession (callback) failed (${updateResponse.status}): ${errorText}`); + } + + // Step 2: Send follow-up message (user-facing, no callbackTarget) + const userHeaders: Record = { + Authorization: `Bearer ${this.state.authToken}`, + 'Content-Type': 'application/json', + }; + if (this.state.skipBalanceCheck) { + userHeaders['x-skip-balance-check'] = 'true'; + } + const sendMessageInput = { cloudAgentSessionId: previousSessionId, prompt: this.state.sessionInput.prompt, @@ -755,7 +782,6 @@ export class CodeReviewOrchestrator extends DurableObject { variant: this.state.sessionInput.variant, githubToken: this.state.sessionInput.githubToken, gitToken: this.state.sessionInput.gitToken, - callbackTarget, }; console.log('[CodeReviewOrchestrator] Calling sendMessageV2', { @@ -766,7 +792,7 @@ export class CodeReviewOrchestrator extends DurableObject { const response = await fetch(`${this.env.CLOUD_AGENT_NEXT_URL}/trpc/sendMessageV2`, { method: 'POST', - headers, + headers: userHeaders, body: JSON.stringify(sendMessageInput), }); @@ -801,14 +827,11 @@ export class CodeReviewOrchestrator extends DurableObject { } 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, - } - ); + 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 @@ -821,13 +844,10 @@ export class CodeReviewOrchestrator extends DurableObject { // 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, - } - ); + console.error('[CodeReviewOrchestrator] Fresh session fallback also failed', { + reviewId: this.state.reviewId, + error: freshErrorMessage, + }); } } } diff --git a/src/lib/code-reviews/db/code-reviews.test.ts b/src/lib/code-reviews/db/code-reviews.test.ts index df80112581..1fb7c51a59 100644 --- a/src/lib/code-reviews/db/code-reviews.test.ts +++ b/src/lib/code-reviews/db/code-reviews.test.ts @@ -23,9 +23,7 @@ describe('findPreviousCompletedReviewSession', () => { afterAll(async () => { // Clean up reviews then user for (const id of createdReviewIds) { - await db - .delete(cloud_agent_code_reviews) - .where(eq(cloud_agent_code_reviews.id, id)); + 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)); }); @@ -41,6 +39,7 @@ describe('findPreviousCompletedReviewSession', () => { baseRef: 'main', headRef: 'feature/test', headSha, + platform: 'github', }); createdReviewIds.push(id); return id; @@ -72,11 +71,7 @@ describe('findPreviousCompletedReviewSession', () => { }); it('excludes the current SHA', async () => { - const result = await findPreviousCompletedReviewSession( - REPO, - 42, - 'sha-with-session' - ); + const result = await findPreviousCompletedReviewSession(REPO, 42, 'sha-with-session'); // Should not find itself — only reviews with a different SHA // The "sha-no-session" review has no session_id, so only "sha-with-session" would match // but it's excluded by excludeSha @@ -108,11 +103,7 @@ describe('findPreviousCompletedReviewSession', () => { }); it('is consistent with findPreviousCompletedReview on head_sha', async () => { - const sessionResult = await findPreviousCompletedReviewSession( - REPO, - 42, - 'other-sha' - ); + const sessionResult = await findPreviousCompletedReviewSession(REPO, 42, 'other-sha'); const shaResult = await findPreviousCompletedReview(REPO, 42, 'other-sha'); // Both should find a completed review (sessionResult is more restrictive) diff --git a/src/lib/code-reviews/triggers/prepare-review-payload.ts b/src/lib/code-reviews/triggers/prepare-review-payload.ts index 4e92cea5e6..3da2a6a9c0 100644 --- a/src/lib/code-reviews/triggers/prepare-review-payload.ts +++ b/src/lib/code-reviews/triggers/prepare-review-payload.ts @@ -56,7 +56,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; @@ -371,14 +370,11 @@ export async function prepareReviewPayload( previousCloudAgentSessionId = previousSession?.session_id; if (previousCloudAgentSessionId) { - logExceptInTest( - '[prepareReviewPayload] Found previous session for continuation', - { - reviewId, - previousCloudAgentSessionId, - previousHeadSha: previousSession?.head_sha.substring(0, 8), - } - ); + logExceptInTest('[prepareReviewPayload] Found previous session for continuation', { + reviewId, + previousCloudAgentSessionId, + previousHeadSha: previousSession?.head_sha.substring(0, 8), + }); } } catch (error) { // Non-critical - fall back to fresh session From e5f3233277f7e4fce971c5a65b525173d690c281 Mon Sep 17 00:00:00 2001 From: Alex Alecu Date: Wed, 11 Mar 2026 14:08:42 +0200 Subject: [PATCH 20/24] fix(code-reviews): derive incremental diff base and session ID from same review row Previously findPreviousCompletedReview and findPreviousCompletedReviewSession were separate DB queries that could return different review rows when the most recent completed review lacked a session_id. This caused the incremental diff to target one SHA while session continuation resumed a workspace from a different, older review. Consolidate into a single findPreviousCompletedReview that returns both head_sha and session_id (nullable), ensuring both values always come from the same row. --- src/lib/code-reviews/db/code-reviews.test.ts | 57 ++++++++++--------- src/lib/code-reviews/db/code-reviews.ts | 51 +++-------------- .../triggers/prepare-review-payload.ts | 42 ++------------ 3 files changed, 42 insertions(+), 108 deletions(-) diff --git a/src/lib/code-reviews/db/code-reviews.test.ts b/src/lib/code-reviews/db/code-reviews.test.ts index 1fb7c51a59..82589bfb3f 100644 --- a/src/lib/code-reviews/db/code-reviews.test.ts +++ b/src/lib/code-reviews/db/code-reviews.test.ts @@ -7,12 +7,11 @@ import { createCodeReview, updateCodeReviewStatus, findPreviousCompletedReview, - findPreviousCompletedReviewSession, } from './code-reviews'; const REPO = `test-org/session-continuation-${Date.now()}`; -describe('findPreviousCompletedReviewSession', () => { +describe('findPreviousCompletedReview', () => { let testUser: User; const createdReviewIds: string[] = []; @@ -21,7 +20,6 @@ describe('findPreviousCompletedReviewSession', () => { }); afterAll(async () => { - // Clean up reviews then user for (const id of createdReviewIds) { await db.delete(cloud_agent_code_reviews).where(eq(cloud_agent_code_reviews.id, id)); } @@ -46,70 +44,73 @@ describe('findPreviousCompletedReviewSession', () => { } it('returns null when no previous completed review exists', async () => { - const result = await findPreviousCompletedReviewSession(REPO, 42, 'abc123'); + const result = await findPreviousCompletedReview(REPO, 42, 'abc123'); expect(result).toBeNull(); }); - it('returns null when previous review is completed but has no session_id', async () => { + 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 findPreviousCompletedReviewSession(REPO, 42, 'other-sha'); - expect(result).toBeNull(); + 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 session_id and head_sha for a completed review with session_id', async () => { + 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 findPreviousCompletedReviewSession(REPO, 42, 'other-sha'); + const result = await findPreviousCompletedReview(REPO, 42, 'other-sha'); expect(result).not.toBeNull(); - expect(result!.session_id).toBe('agent_test123'); expect(result!.head_sha).toBe('sha-with-session'); + expect(result!.session_id).toBe('agent_test123'); }); it('excludes the current SHA', async () => { - const result = await findPreviousCompletedReviewSession(REPO, 42, 'sha-with-session'); - // Should not find itself — only reviews with a different SHA - // The "sha-no-session" review has no session_id, so only "sha-with-session" would match - // but it's excluded by excludeSha - expect(result).toBeNull(); + 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 with session_id', async () => { + it('returns the most recent completed review', async () => { const id = await createReview('sha-newer'); await updateCodeReviewStatus(id, 'completed', { sessionId: 'agent_newer', }); - const result = await findPreviousCompletedReviewSession(REPO, 42, 'other-sha'); + const result = await findPreviousCompletedReview(REPO, 42, 'other-sha'); expect(result).not.toBeNull(); - expect(result!.session_id).toBe('agent_newer'); expect(result!.head_sha).toBe('sha-newer'); + expect(result!.session_id).toBe('agent_newer'); }); - it('ignores non-completed reviews with session_id', async () => { + 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 findPreviousCompletedReviewSession(REPO, 42, 'other-sha'); + 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('is consistent with findPreviousCompletedReview on head_sha', async () => { - const sessionResult = await findPreviousCompletedReviewSession(REPO, 42, 'other-sha'); - const shaResult = await findPreviousCompletedReview(REPO, 42, 'other-sha'); + 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'); - // Both should find a completed review (sessionResult is more restrictive) - expect(shaResult).not.toBeNull(); - expect(sessionResult).not.toBeNull(); - // The head_sha from the session query should also appear in the sha-only query - expect(sessionResult!.head_sha).toBe(shaResult!.head_sha); + 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 22e7ff3077..0905c2d5d5 100644 --- a/src/lib/code-reviews/db/code-reviews.ts +++ b/src/lib/code-reviews/db/code-reviews.ts @@ -11,7 +11,7 @@ import { microdollar_usage, microdollar_usage_metadata, } from '@kilocode/db/schema'; -import { eq, and, desc, count, ne, inArray, isNotNull, sql, sum, gte } from 'drizzle-orm'; +import { eq, and, desc, count, ne, inArray, sql, sum, gte } from 'drizzle-orm'; import { captureException } from '@sentry/nextjs'; import type { CreateReviewParams, CodeReviewStatus, ListReviewsParams, Owner } from '../core'; import type { CloudAgentCodeReview } from '@kilocode/db/schema'; @@ -428,50 +428,15 @@ 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 } | null> { - try { - const [review] = await db - .select({ head_sha: cloud_agent_code_reviews.head_sha }) - .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; - } -} - -/** - * Like findPreviousCompletedReview, but also returns the session_id. - * Used for session continuation: reuses the cloud-agent session on follow-up pushes. - * Only returns reviews that have a non-null session_id. - */ -export async function findPreviousCompletedReviewSession( - repoFullName: string, - prNumber: number, - excludeSha: string, - platform: string = 'github' -): Promise<{ head_sha: string; session_id: string } | null> { +): Promise<{ head_sha: string; session_id: string | null } | null> { try { const [review] = await db .select({ @@ -485,18 +450,16 @@ export async function findPreviousCompletedReviewSession( 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'), - isNotNull(cloud_agent_code_reviews.session_id) + eq(cloud_agent_code_reviews.status, 'completed') ) ) .orderBy(desc(cloud_agent_code_reviews.created_at)) .limit(1); - if (!review || !review.session_id) return null; - return { head_sha: review.head_sha, session_id: review.session_id }; + return review || null; } catch (error) { captureException(error, { - tags: { operation: 'findPreviousCompletedReviewSession' }, + tags: { operation: 'findPreviousCompletedReview' }, extra: { repoFullName, prNumber, excludeSha, platform }, }); throw error; diff --git a/src/lib/code-reviews/triggers/prepare-review-payload.ts b/src/lib/code-reviews/triggers/prepare-review-payload.ts index 3da2a6a9c0..e0ad075c8d 100644 --- a/src/lib/code-reviews/triggers/prepare-review-payload.ts +++ b/src/lib/code-reviews/triggers/prepare-review-payload.ts @@ -36,11 +36,7 @@ import type { GitLabDiffContext, } from '../prompts/generate-prompt'; import { getIntegrationById } from '@/lib/integrations/db/platform-integrations'; -import { - getCodeReviewById, - findPreviousCompletedReview, - findPreviousCompletedReviewSession, -} from '../db/code-reviews'; +import { getCodeReviewById, findPreviousCompletedReview } from '../db/code-reviews'; import { isFeatureFlagEnabled } from '@/lib/posthog-feature-flags'; import { DEFAULT_CODE_REVIEW_MODEL, @@ -322,7 +318,10 @@ export async function prepareReviewPayload( } // 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) { @@ -334,6 +333,7 @@ export async function prepareReviewPayload( platform ); previousHeadSha = previousReview?.head_sha ?? null; + previousCloudAgentSessionId = previousReview?.session_id ?? undefined; if (previousHeadSha) { logExceptInTest( @@ -342,6 +342,7 @@ export async function prepareReviewPayload( reviewId, previousHeadSha: previousHeadSha.substring(0, 8), currentHeadSha: review.head_sha.substring(0, 8), + previousCloudAgentSessionId, } ); } @@ -357,37 +358,6 @@ export async function prepareReviewPayload( } } - // 4b. Look up previous session ID for session continuation - let previousCloudAgentSessionId: string | undefined; - if (incrementalEnabled) { - try { - const previousSession = await findPreviousCompletedReviewSession( - review.repo_full_name, - review.pr_number, - existingReviewState?.headCommitSha ?? review.head_sha, - platform - ); - previousCloudAgentSessionId = previousSession?.session_id; - - if (previousCloudAgentSessionId) { - logExceptInTest('[prepareReviewPayload] Found previous session for continuation', { - reviewId, - previousCloudAgentSessionId, - previousHeadSha: previousSession?.head_sha.substring(0, 8), - }); - } - } catch (error) { - // Non-critical - fall back to fresh session - logExceptInTest( - '[prepareReviewPayload] Failed to fetch previous session, falling back to fresh session:', - { - reviewId, - error, - } - ); - } - } - // 5. Generate auth token for cloud agent with bot identifier const authToken = generateApiToken(user, { botId: 'reviewer' }); From a96a876bd3dfcefd87cb0483d95d821d7e9d9467 Mon Sep 17 00:00:00 2001 From: Alex Alecu Date: Wed, 11 Mar 2026 14:13:07 +0200 Subject: [PATCH 21/24] refactor(code-reviews): extract shared cloud-agent-next fetch client into worker-utils The code-review orchestrator Durable Object used raw fetch() calls to cloud-agent-next tRPC endpoints, duplicating request/response parsing that the Next.js CloudAgentNextClient already handled. Any API shape change required updating both codebases independently. Add a lightweight, fetch-based createCloudAgentNextFetchClient() to @kilocode/worker-utils with typed methods for prepareSession, initiateFromPreparedSession, updateSession, sendMessageV2, and interruptSession. Refactor the orchestrator to use the shared client, eliminating ~120 lines of manual tRPC envelope parsing. --- .../src/code-review-orchestrator.ts | 192 +++++---------- .../src/cloud-agent-next-client.ts | 228 ++++++++++++++++++ packages/worker-utils/src/index.ts | 15 ++ 3 files changed, 308 insertions(+), 127 deletions(-) create mode 100644 packages/worker-utils/src/cloud-agent-next-client.ts diff --git a/cloudflare-code-review-infra/src/code-review-orchestrator.ts b/cloudflare-code-review-infra/src/code-review-orchestrator.ts index f0a07725c8..ed0ccdcac8 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). @@ -469,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}`); + } } } @@ -551,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'); @@ -561,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, @@ -632,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', { @@ -645,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 @@ -722,6 +691,7 @@ export class CodeReviewOrchestrator extends DurableObject { */ private async runWithCloudAgentNextFollowup(): Promise { const previousSessionId = this.state.previousCloudAgentSessionId!; + const client = this.getCloudAgentNextClient(); console.log('[CodeReviewOrchestrator] Attempting session continuation via sendMessageV2', { reviewId: this.state.reviewId, @@ -734,7 +704,6 @@ export class CodeReviewOrchestrator extends DurableObject { // Build internal headers (internalApiProtectedProcedure — API key + Bearer token) 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) { @@ -751,30 +720,26 @@ export class CodeReviewOrchestrator extends DurableObject { }, }; - const updateResponse = await fetch(`${this.env.CLOUD_AGENT_NEXT_URL}/trpc/updateSession`, { - method: 'POST', - headers: internalHeaders, - body: JSON.stringify({ - cloudAgentSessionId: previousSessionId, - callbackTarget, - }), + await client.updateSession(internalHeaders, { + cloudAgentSessionId: previousSessionId, + callbackTarget, }); - if (!updateResponse.ok) { - const errorText = await updateResponse.text(); - throw new Error(`updateSession (callback) failed (${updateResponse.status}): ${errorText}`); - } - // Step 2: Send follow-up message (user-facing, no callbackTarget) const userHeaders: Record = { Authorization: `Bearer ${this.state.authToken}`, - 'Content-Type': 'application/json', }; if (this.state.skipBalanceCheck) { userHeaders['x-skip-balance-check'] = 'true'; } - const sendMessageInput = { + 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, @@ -782,35 +747,8 @@ export class CodeReviewOrchestrator extends DurableObject { variant: this.state.sessionInput.variant, githubToken: this.state.sessionInput.githubToken, gitToken: this.state.sessionInput.gitToken, - }; - - console.log('[CodeReviewOrchestrator] Calling sendMessageV2', { - reviewId: this.state.reviewId, - cloudAgentSessionId: previousSessionId, - callbackUrl: callbackTarget.url, }); - const response = await fetch(`${this.env.CLOUD_AGENT_NEXT_URL}/trpc/sendMessageV2`, { - method: 'POST', - headers: userHeaders, - body: JSON.stringify(sendMessageInput), - }); - - if (!response.ok) { - const errorText = await response.text(); - throw new Error(`sendMessageV2 failed (${response.status}): ${errorText}`); - } - - const result: Record = await response.json(); - const data = (result?.result as Record)?.data as - | Record - | undefined; - if (!data || typeof data.executionId !== 'string') { - throw new Error( - `Unexpected sendMessageV2 response shape: ${JSON.stringify(result).slice(0, 500)}` - ); - } - // Store session ID (reusing the previous one) and execution ID await this.updateStatus('running', { sessionId: previousSessionId, @@ -819,8 +757,8 @@ export class CodeReviewOrchestrator extends DurableObject { console.log('[CodeReviewOrchestrator] Follow-up execution started via sendMessageV2', { reviewId: this.state.reviewId, cloudAgentSessionId: previousSessionId, - executionId: data.executionId, - status: data.status, + executionId: sendResult.executionId, + status: sendResult.status, }); // Done — cloud-agent-next callback will deliver terminal status 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..0effb07801 --- /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: Record = await response.json(); + 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, From 8c85efeb73b903f192df006f3c382fcb65642b77 Mon Sep 17 00:00:00 2001 From: Alex Alecu Date: Wed, 11 Mar 2026 14:20:14 +0200 Subject: [PATCH 22/24] fix: resolve lint errors in cloud-agent-next client and orchestrator --- cloudflare-code-review-infra/src/code-review-orchestrator.ts | 5 ++++- packages/worker-utils/src/cloud-agent-next-client.ts | 2 +- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/cloudflare-code-review-infra/src/code-review-orchestrator.ts b/cloudflare-code-review-infra/src/code-review-orchestrator.ts index ed0ccdcac8..273ad19f18 100644 --- a/cloudflare-code-review-infra/src/code-review-orchestrator.ts +++ b/cloudflare-code-review-infra/src/code-review-orchestrator.ts @@ -690,7 +690,10 @@ export class CodeReviewOrchestrator extends DurableObject { * On failure (404, 409, etc.), falls back to runWithCloudAgentNext() for a fresh session. */ private async runWithCloudAgentNextFollowup(): Promise { - const previousSessionId = this.state.previousCloudAgentSessionId!; + 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', { diff --git a/packages/worker-utils/src/cloud-agent-next-client.ts b/packages/worker-utils/src/cloud-agent-next-client.ts index 0effb07801..f25585af12 100644 --- a/packages/worker-utils/src/cloud-agent-next-client.ts +++ b/packages/worker-utils/src/cloud-agent-next-client.ts @@ -115,7 +115,7 @@ async function trpcPost( throw new CloudAgentNextError(procedure, response.status, errorText); } - const json: Record = await response.json(); + const json = (await response.json()) as Record; const data = (json?.result as Record | undefined)?.data; if (data === undefined) { throw new Error( From d07e224b55feae30329a38da972fd09ae4e40e86 Mon Sep 17 00:00:00 2001 From: Alex Alecu Date: Wed, 11 Mar 2026 15:59:20 +0200 Subject: [PATCH 23/24] fix(cloud-agent-next): suppress failure callback for followup executions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When sendMessageV2 fails during session continuation (e.g. workspace restore), executeDirectly was enqueuing a failure callback using the already-updated callback target — pointing at the new review. The orchestrator catches this error synchronously and falls back to a fresh session, but the stale failure callback races ahead and marks the new review as failed permanently. Thread a suppressCallbackOnError flag through executeDirectly → failExecution → updateExecutionStatus so that followup-path failures still persist terminal state in the DO but skip the callback enqueue. The initiate and initiatePrepared paths are unchanged since they have no synchronous caller. --- .../src/persistence/CloudAgentSession.ts | 47 ++++++++++++++----- 1 file changed, 36 insertions(+), 11 deletions(-) diff --git a/cloud-agent-next/src/persistence/CloudAgentSession.ts b/cloud-agent-next/src/persistence/CloudAgentSession.ts index 239b75c3ad..3868ae65ae 100644 --- a/cloud-agent-next/src/persistence/CloudAgentSession.ts +++ b/cloud-agent-next/src/persistence/CloudAgentSession.ts @@ -1235,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, @@ -1370,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; @@ -1381,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 @@ -2010,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)) { @@ -2044,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'); @@ -2092,6 +2116,7 @@ export class CloudAgentSession extends DurableObject { status: 'failed', error: errorMessage, streamEventType: 'error', + suppressCallback: opts?.suppressCallbackOnError, }); throw error; From e33dedfeefeff01c620d717207e92d4829a96438 Mon Sep 17 00:00:00 2001 From: Alex Alecu Date: Wed, 11 Mar 2026 15:59:26 +0200 Subject: [PATCH 24/24] fix(code-reviews): reject stale callbacks from superseded sessions Defense-in-depth guard in the code-review-status handler: before accepting a status update, check whether the callback's sessionId differs from the review's current session_id. If both are non-null and don't match, the callback belongs to a superseded session and is silently discarded. This prevents race conditions where a failure callback from an old session corrupts a review that has already been picked up by a new fallback session. --- .../code-review-status/[reviewId]/route.ts | 22 +++++++++++++++++++ 1 file changed, 22 insertions(+) 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)