Skip to content

fix(code-reviews): guard retrigger races#4352

Open
alex-alecu wants to merge 1 commit into
mainfrom
fix/code-review-retrigger-active
Open

fix(code-reviews): guard retrigger races#4352
alex-alecu wants to merge 1 commit into
mainfrom
fix/code-review-retrigger-active

Conversation

@alex-alecu

Copy link
Copy Markdown
Contributor

Summary

Fixes Code Reviewer retry so retrying an old failed review no longer crashes when another review is already running for the same pull request. The retry now stops with a clear message if a newer review is already active. If an older review looks stuck, the user is asked before Kilo cancels it and retries the newer failed review.

Verification

  1. Open a failed Code Reviewer job for a pull request that also has a newer running review.
  2. Click Retry and confirm Kilo shows a clear conflict message instead of crashing.
  3. Open a newer failed job for a pull request that has an older stuck review.
  4. Click Retry and confirm Kilo asks before cancelling the old review and retrying.

Visual Changes

N/A

Reviewer Notes

The DB-backed router test could not be run locally because Docker was unavailable. The main risk area is the handoff where Kilo cancels an older active review before retrying the failed review.

}

function isPostgresUniqueViolation(error: unknown): boolean {
return error instanceof Error && 'code' in error && error.code === '23505';

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

WARNING: Unique-violation detection likely never fires because it only checks the top-level error.code

This codebase's own isUniqueViolation helper in apps/web/src/routers/admin/credit-campaigns-router.ts documents that Drizzle (this repo pins drizzle-orm@0.45.2) wraps the underlying pg error in a DrizzleQueryError, so the Postgres error code lives on error.cause.code, not error.code. That helper explicitly checks both the top-level error and error.cause for this reason.

isPostgresUniqueViolation here only checks error.code, so a real 23505 raised by UQ_cloud_agent_code_reviews_active_provider_publisher when resetCodeReviewForRetry races another dispatch will likely fall through to the generic throw error; path instead of the intended friendly CONFLICT — undermining the race-guard this PR is meant to add. This path also has no test coverage exercising the actual unique-violation catch.

Suggested change
return error instanceof Error && 'code' in error && error.code === '23505';
return error instanceof Error && 'code' in error && (error.code === '23505' || (error.cause instanceof Error && 'code' in error.cause && error.cause.code === '23505'));

Reply with @kilocode-bot fix it to have Kilo Code address this issue.

@kilo-code-bot

kilo-code-bot Bot commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

Code Review Summary

Status: 1 Issue Found | Recommendation: Address before merge

Executive Summary

The new isPostgresUniqueViolation guard in code-reviews-router.ts checks only the top-level error.code, which is unlikely to match the Drizzle-wrapped Postgres error for the 23505 unique-violation case it's meant to catch.

Overview

Severity Count
CRITICAL 0
WARNING 1
SUGGESTION 0
Issue Details (click to expand)

WARNING

File Line Issue
apps/web/src/routers/code-reviews/code-reviews-router.ts 232 isPostgresUniqueViolation only checks error.code, but Drizzle wraps the pg error so the code likely lives on error.cause.code (per the existing pattern in credit-campaigns-router.ts), so this race guard likely never fires
Files Reviewed (6 files)
  • apps/web/src/app/(app)/code-reviews/[reviewId]/CodeReviewDetailClient.tsx
  • apps/web/src/components/code-reviews/CodeReviewJobsCard.tsx
  • apps/web/src/components/code-reviews/useRetriggerReview.ts
  • apps/web/src/lib/code-reviews/core/schemas.ts
  • apps/web/src/routers/code-reviews-router.test.ts
  • apps/web/src/routers/code-reviews/code-reviews-router.ts

Fix these issues in Kilo Cloud


Reviewed by claude-sonnet-5-20260630 · Input: 80 · Output: 33.2K · Cached: 4.5M

Review guidance: REVIEW.md from base branch main

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants