feat(code-reviews): add manual jobs#4273
Conversation
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
| ); | ||
|
|
||
| export const organizationReviewAgentRouter = createTRPCRouter({ | ||
| createManualReviewJob: organizationMemberMutationProcedure |
There was a problem hiding this comment.
WARNING: Manual review creation skips organization billing enforcement
This new mutation uses organizationMemberMutationProcedure, unlike the other organization code-review write paths in this router that go through organizationBillingMutationProcedure. As a result, members can start manual review jobs even when the organization is blocked by subscription or billing checks, which is a permission regression for a capacity-consuming action.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
There was a problem hiding this comment.
No change needed. This path still checks that the org has an active plan, and using the billing-only path would stop regular members from starting jobs.
| @@ -0,0 +1,8 @@ | |||
| DROP INDEX "UQ_cloud_agent_code_reviews_repo_pr_sha";--> statement-breakpoint | |||
| ALTER TABLE "cloud_agent_code_reviews" ADD COLUMN "manual_config" jsonb;--> statement-breakpoint | |||
| CREATE UNIQUE INDEX "UQ_cloud_agent_code_reviews_webhook_integration_repo_pr_sha" ON "cloud_agent_code_reviews" USING btree ("platform_integration_id","repo_full_name","pr_number","head_sha") WHERE "cloud_agent_code_reviews"."manual_config" IS NULL;--> statement-breakpoint | |||
There was a problem hiding this comment.
WARNING: Non-concurrent indexes will block writes on a hot table
cloud_agent_code_reviews is already populated and written continuously by the review queue. Building these four unique indexes with plain CREATE UNIQUE INDEX takes a write-blocking lock for the duration of each build, so deploying this migration can stall or fail live review creation and status updates. These indexes need to be created concurrently in a separate migration step.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
Code Review SummaryStatus: 1 Issue Found | Recommendation: Address before merge Fix these issues in Kilo Cloud Overview
Issue Details (click to expand)WARNING
Files Reviewed (9 files)
Previous Review Summaries (2 snapshots, latest commit caeef16)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit caeef16)Status: 1 Issue Found | Recommendation: Address before merge Fix these issues in Kilo Cloud Overview
Issue Details (click to expand)WARNING
Files Reviewed (5 files)
Previous review (commit 72fcba5)Status: 2 Issues Found | Recommendation: Address before merge Fix these issues in Kilo Cloud Overview
Issue Details (click to expand)WARNING
Files Reviewed (31 files)
Reviewed by gpt-5.4-20260305 · Input: 94.7K · Output: 12.4K · Cached: 474.9K Review guidance: REVIEW.md from base branch |
| CREATE UNIQUE INDEX "UQ_cloud_agent_code_reviews_manual_user_repo_pr_sha" ON "cloud_agent_code_reviews" USING btree ("owned_by_user_id","repo_full_name","pr_number","head_sha") WHERE "cloud_agent_code_reviews"."manual_config" IS NOT NULL AND "cloud_agent_code_reviews"."owned_by_user_id" IS NOT NULL;--> statement-breakpoint | ||
| CREATE UNIQUE INDEX "UQ_cloud_agent_code_reviews_manual_org_repo_pr_sha" ON "cloud_agent_code_reviews" USING btree ("owned_by_organization_id","repo_full_name","pr_number","head_sha") WHERE "cloud_agent_code_reviews"."manual_config" IS NOT NULL AND "cloud_agent_code_reviews"."owned_by_organization_id" IS NOT NULL;--> statement-breakpoint | ||
| CREATE UNIQUE INDEX "UQ_cloud_agent_code_reviews_active_provider_publisher" ON "cloud_agent_code_reviews" USING btree ("platform_integration_id","repo_full_name","pr_number") WHERE "cloud_agent_code_reviews"."platform_integration_id" IS NOT NULL |
There was a problem hiding this comment.
@kilocode-bot do we really need new indexes just because we added manual_config ?
There was a problem hiding this comment.
Yes, the new indexes are genuinely needed — but it's deeper than just "a new column was added." Here's the breakdown:
Why the old index had to be dropped
The original UQ_cloud_agent_code_reviews_repo_pr_sha was a global unique key on (repo_full_name, pr_number, head_sha). That breaks once manual jobs exist, because:
- A webhook-driven review and a manual review of the same commit can legitimately coexist.
- Different users can each manually review the same public PR at the same SHA (local dev mode). The old index would cause the second user's job to collide.
Why 4 partial indexes instead of 1
The feature introduces multiple distinct identity scopes, and the unique key column differs per scope:
| Index | Enforces |
|---|---|
..._webhook_integration_repo_pr_sha |
One webhook review per integration + repo/pr/sha |
..._manual_user_repo_pr_sha |
One manual review per user + repo/pr/sha |
..._manual_org_repo_pr_sha |
One manual review per org + repo/pr/sha |
..._active_provider_publisher |
At most one active review posting back to the provider per integration + repo/PR (no sha — prevents concurrent comment threads) |
The 4th index is the most important for correctness — it prevents two jobs from simultaneously writing to the same GitHub/GitLab PR comment thread, even at different SHAs.
One minor simplification possible
The two manual indexes (user + org) could be merged into a single WHERE manual_config IS NOT NULL index using COALESCE(owned_by_user_id::text, owned_by_organization_id::text) as the key, since the owner_check constraint already guarantees only one is set. The current two-index form is more explicit and readable though — reasonable tradeoff.
Bottom line: The split is justified by the feature's semantics, not index bloat. The only real simplification would be merging the two manual owner indexes into one.
| @@ -0,0 +1,10 @@ | |||
| DROP INDEX "UQ_cloud_agent_code_reviews_repo_pr_sha";--> statement-breakpoint | |||
| ALTER TABLE "cloud_agent_code_reviews" ADD COLUMN "manual_config" jsonb;--> statement-breakpoint | |||
| COMMIT;--> statement-breakpoint | |||
There was a problem hiding this comment.
WARNING: Committing before the replacement unique indexes finish opens a duplicate window
This COMMIT makes the drop of UQ_cloud_agent_code_reviews_repo_pr_sha visible before any of the new scoped unique indexes exist. Because cloud_agent_code_reviews is still accepting writes while lines 4-7 build concurrently, duplicate review rows can slip in for the same webhook/manual identity, which can then make one of the new unique index builds fail or leave bad duplicates behind if the migration is retried.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
1 participant
Summary
Manual Code Reviewer jobs can now be started from the Jobs card for personal users and organizations. In normal environments, jobs use the connected GitHub or GitLab account and post results back to the change. In local development, jobs can review public GitHub and GitLab changes without provider credentials and show the result in Kilo only.
Verification
Visual Changes
N/A
Reviewer Notes
Manual jobs store their model, effort, instructions, and output mode on the job so retries keep the same behavior. Local jobs are intentionally limited to explicit local development and public github.com or gitlab.com changes.