Normalize Cursor verdict values and post PR comment on review failures.
Coerce common verdict/event variants before validation, skip retries on schema errors, and leave a failure comment plus reviewer removal for any failed run. Co-authored-by: Cursor <cursoragent@cursor.com>
This commit is contained in:
@@ -1,7 +1,18 @@
|
|||||||
import { z } from "zod";
|
import { z, ZodError } from "zod";
|
||||||
|
|
||||||
export const reviewSchema = z.object({
|
export { ZodError };
|
||||||
verdict: z.enum(["approve", "request_changes", "comment"]),
|
|
||||||
|
const VERDICTS = ["approve", "request_changes", "comment"] as const;
|
||||||
|
type Verdict = (typeof VERDICTS)[number];
|
||||||
|
|
||||||
|
const EVENT_BY_VERDICT: Record<Verdict, "APPROVE" | "REQUEST_CHANGES" | "COMMENT"> = {
|
||||||
|
approve: "APPROVE",
|
||||||
|
request_changes: "REQUEST_CHANGES",
|
||||||
|
comment: "COMMENT"
|
||||||
|
};
|
||||||
|
|
||||||
|
const reviewSchema = z.object({
|
||||||
|
verdict: z.enum(VERDICTS),
|
||||||
event: z.enum(["APPROVE", "REQUEST_CHANGES", "COMMENT"]),
|
event: z.enum(["APPROVE", "REQUEST_CHANGES", "COMMENT"]),
|
||||||
body: z.string().min(1),
|
body: z.string().min(1),
|
||||||
comments: z.array(
|
comments: z.array(
|
||||||
@@ -16,5 +27,74 @@ export const reviewSchema = z.object({
|
|||||||
export type ReviewResult = z.infer<typeof reviewSchema>;
|
export type ReviewResult = z.infer<typeof reviewSchema>;
|
||||||
|
|
||||||
export function parseReviewResult(payload: unknown): ReviewResult {
|
export function parseReviewResult(payload: unknown): ReviewResult {
|
||||||
return reviewSchema.parse(payload);
|
return reviewSchema.parse(normalizeReviewPayload(payload));
|
||||||
|
}
|
||||||
|
|
||||||
|
function normalizeReviewPayload(payload: unknown): unknown {
|
||||||
|
if (!payload || typeof payload !== "object") {
|
||||||
|
return payload;
|
||||||
|
}
|
||||||
|
|
||||||
|
const obj = payload as Record<string, unknown>;
|
||||||
|
const verdict = normalizeVerdict(obj.verdict) ?? normalizeVerdict(obj.event);
|
||||||
|
if (!verdict) {
|
||||||
|
return payload;
|
||||||
|
}
|
||||||
|
|
||||||
|
return {
|
||||||
|
...obj,
|
||||||
|
verdict,
|
||||||
|
event: normalizeEvent(obj.event, verdict),
|
||||||
|
comments: Array.isArray(obj.comments) ? obj.comments : []
|
||||||
|
};
|
||||||
|
}
|
||||||
|
|
||||||
|
function normalizeVerdict(raw: unknown): Verdict | undefined {
|
||||||
|
if (typeof raw !== "string") {
|
||||||
|
return undefined;
|
||||||
|
}
|
||||||
|
|
||||||
|
const normalized = raw.trim().toLowerCase().replace(/[\s-]+/g, "_");
|
||||||
|
const aliases: Record<string, Verdict> = {
|
||||||
|
approve: "approve",
|
||||||
|
approved: "approve",
|
||||||
|
lgtm: "approve",
|
||||||
|
request_changes: "request_changes",
|
||||||
|
requestchange: "request_changes",
|
||||||
|
changes_requested: "request_changes",
|
||||||
|
changes: "request_changes",
|
||||||
|
reject: "request_changes",
|
||||||
|
comment: "comment",
|
||||||
|
commented: "comment",
|
||||||
|
neutral: "comment"
|
||||||
|
};
|
||||||
|
|
||||||
|
if ((VERDICTS as readonly string[]).includes(normalized)) {
|
||||||
|
return normalized as Verdict;
|
||||||
|
}
|
||||||
|
|
||||||
|
const fromAlias = aliases[normalized];
|
||||||
|
if (fromAlias) {
|
||||||
|
return fromAlias;
|
||||||
|
}
|
||||||
|
|
||||||
|
const upper = raw.trim().toUpperCase();
|
||||||
|
if (upper === "APPROVE") return "approve";
|
||||||
|
if (upper === "REQUEST_CHANGES") return "request_changes";
|
||||||
|
if (upper === "COMMENT") return "comment";
|
||||||
|
|
||||||
|
return undefined;
|
||||||
|
}
|
||||||
|
|
||||||
|
function normalizeEvent(
|
||||||
|
raw: unknown,
|
||||||
|
verdict: Verdict
|
||||||
|
): "APPROVE" | "REQUEST_CHANGES" | "COMMENT" {
|
||||||
|
if (typeof raw === "string") {
|
||||||
|
const upper = raw.trim().toUpperCase();
|
||||||
|
if (upper === "APPROVE" || upper === "REQUEST_CHANGES" || upper === "COMMENT") {
|
||||||
|
return upper;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
return EVENT_BY_VERDICT[verdict];
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -37,28 +37,27 @@ export async function deleteProgressComment(input: {
|
|||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
|
||||||
export const TIMEOUT_FAILURE_MARKER = "<!-- gitea-pr-review-bot-timeout -->";
|
export const FAILURE_COMMENT_MARKER = "<!-- gitea-pr-review-bot-failure -->";
|
||||||
|
|
||||||
export function buildTimeoutFailureBody(timeoutMs: number): string {
|
export function buildReviewFailureBody(reason: string): string {
|
||||||
const timeoutMinutes = Math.max(1, Math.round(timeoutMs / 60_000));
|
|
||||||
return (
|
return (
|
||||||
`${TIMEOUT_FAILURE_MARKER}\n\n` +
|
`${FAILURE_COMMENT_MARKER}\n\n` +
|
||||||
`🤖 **PR review failed:** the automated review timed out after ${timeoutMinutes} minute(s). ` +
|
`🤖 **PR review failed:** ${reason} ` +
|
||||||
"You can request the bot again as reviewer or review the changes manually."
|
"You can request the bot again as reviewer or review the changes manually."
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
export async function postTimeoutFailureComment(input: {
|
export async function postReviewFailureComment(input: {
|
||||||
gitea: GiteaClient;
|
gitea: GiteaClient;
|
||||||
owner: string;
|
owner: string;
|
||||||
repo: string;
|
repo: string;
|
||||||
prNumber: number;
|
prNumber: number;
|
||||||
timeoutMs: number;
|
reason: string;
|
||||||
}): Promise<void> {
|
}): Promise<void> {
|
||||||
await input.gitea.createIssueComment(
|
await input.gitea.createIssueComment(
|
||||||
input.owner,
|
input.owner,
|
||||||
input.repo,
|
input.repo,
|
||||||
input.prNumber,
|
input.prNumber,
|
||||||
buildTimeoutFailureBody(input.timeoutMs)
|
buildReviewFailureBody(input.reason)
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -22,7 +22,8 @@ export function buildReviewPrompt(input: {
|
|||||||
return [
|
return [
|
||||||
"You are a senior code reviewer for this pull request.",
|
"You are a senior code reviewer for this pull request.",
|
||||||
"Return strict JSON only with fields: verdict,event,body,comments.",
|
"Return strict JSON only with fields: verdict,event,body,comments.",
|
||||||
"Use event one of APPROVE, REQUEST_CHANGES, COMMENT.",
|
'verdict must be exactly one of: "approve", "request_changes", "comment" (lowercase).',
|
||||||
|
"event must be one of: APPROVE, REQUEST_CHANGES, COMMENT (must match verdict).",
|
||||||
`At most ${input.maxInlineComments} inline comments.`,
|
`At most ${input.maxInlineComments} inline comments.`,
|
||||||
"Inline comments MUST have a changed file path and new_position >= 1.",
|
"Inline comments MUST have a changed file path and new_position >= 1.",
|
||||||
"",
|
"",
|
||||||
|
|||||||
+41
-17
@@ -1,13 +1,14 @@
|
|||||||
import { Env } from "../config/env.js";
|
import { Env } from "../config/env.js";
|
||||||
import { loadRepoConfig } from "../config/load-repo-config.js";
|
import { loadRepoConfig } from "../config/load-repo-config.js";
|
||||||
import { runCursorReview } from "../cursor/review-agent.js";
|
import { runCursorReview } from "../cursor/review-agent.js";
|
||||||
|
import { ZodError } from "../cursor/review-schema.js";
|
||||||
import { DedupeStore } from "../domain/dedupe-store.js";
|
import { DedupeStore } from "../domain/dedupe-store.js";
|
||||||
import { shouldProcessEvent } from "../domain/should-process-event.js";
|
import { shouldProcessEvent } from "../domain/should-process-event.js";
|
||||||
import { GiteaClient } from "../gitea/client.js";
|
import { GiteaClient } from "../gitea/client.js";
|
||||||
import {
|
import {
|
||||||
deleteProgressComment,
|
deleteProgressComment,
|
||||||
postProgressComment,
|
postProgressComment,
|
||||||
postTimeoutFailureComment
|
postReviewFailureComment
|
||||||
} from "../gitea/comments-api.js";
|
} from "../gitea/comments-api.js";
|
||||||
import { deletePriorBotReviews, postReview } from "../gitea/review-api.js";
|
import { deletePriorBotReviews, postReview } from "../gitea/review-api.js";
|
||||||
import { removeBotFromReviewers } from "../gitea/reviewer-api.js";
|
import { removeBotFromReviewers } from "../gitea/reviewer-api.js";
|
||||||
@@ -164,7 +165,7 @@ async function executeReview(params: {
|
|||||||
initialDelayMs: 500,
|
initialDelayMs: 500,
|
||||||
operationName: "runCursorReview",
|
operationName: "runCursorReview",
|
||||||
correlationId: input.correlationId,
|
correlationId: input.correlationId,
|
||||||
shouldRetry: (error) => !isTimeoutError(error)
|
shouldRetry: (error) => !isNonRetryableCursorError(error)
|
||||||
});
|
});
|
||||||
|
|
||||||
await deletePriorBotReviews({
|
await deletePriorBotReviews({
|
||||||
@@ -220,17 +221,16 @@ async function executeReview(params: {
|
|||||||
});
|
});
|
||||||
return "success";
|
return "success";
|
||||||
} catch (error) {
|
} catch (error) {
|
||||||
if (isTimeoutError(error)) {
|
await handleReviewFailure({
|
||||||
await handleTimeoutFailure({
|
|
||||||
gitea,
|
gitea,
|
||||||
owner,
|
owner,
|
||||||
repo,
|
repo,
|
||||||
prNumber,
|
prNumber,
|
||||||
botLogin: input.env.GITEA_BOT_LOGIN,
|
botLogin: input.env.GITEA_BOT_LOGIN,
|
||||||
timeoutMs: input.env.REVIEW_TIMEOUT_MS,
|
timeoutMs: input.env.REVIEW_TIMEOUT_MS,
|
||||||
correlationId: input.correlationId
|
correlationId: input.correlationId,
|
||||||
|
error
|
||||||
});
|
});
|
||||||
}
|
|
||||||
throw error;
|
throw error;
|
||||||
} finally {
|
} finally {
|
||||||
if (progressCommentId !== undefined) {
|
if (progressCommentId !== undefined) {
|
||||||
@@ -249,7 +249,28 @@ function isTimeoutError(error: unknown): boolean {
|
|||||||
return error instanceof Error && error.message.toLowerCase().includes("timed out");
|
return error instanceof Error && error.message.toLowerCase().includes("timed out");
|
||||||
}
|
}
|
||||||
|
|
||||||
async function handleTimeoutFailure(input: {
|
function isNonRetryableCursorError(error: unknown): boolean {
|
||||||
|
return isTimeoutError(error) || error instanceof ZodError;
|
||||||
|
}
|
||||||
|
|
||||||
|
function formatFailureReason(error: unknown, timeoutMs: number): string {
|
||||||
|
if (isTimeoutError(error)) {
|
||||||
|
const timeoutMinutes = Math.max(1, Math.round(timeoutMs / 60_000));
|
||||||
|
return `the automated review timed out after ${timeoutMinutes} minute(s).`;
|
||||||
|
}
|
||||||
|
if (error instanceof ZodError) {
|
||||||
|
return "the review response had an invalid format (could not parse verdict/event).";
|
||||||
|
}
|
||||||
|
if (error instanceof Error) {
|
||||||
|
if (error.message.length > 180) {
|
||||||
|
return "an unexpected error occurred while generating the review.";
|
||||||
|
}
|
||||||
|
return error.message;
|
||||||
|
}
|
||||||
|
return "an unexpected error occurred while generating the review.";
|
||||||
|
}
|
||||||
|
|
||||||
|
async function handleReviewFailure(input: {
|
||||||
gitea: GiteaClient;
|
gitea: GiteaClient;
|
||||||
owner: string;
|
owner: string;
|
||||||
repo: string;
|
repo: string;
|
||||||
@@ -257,24 +278,27 @@ async function handleTimeoutFailure(input: {
|
|||||||
botLogin: string;
|
botLogin: string;
|
||||||
timeoutMs: number;
|
timeoutMs: number;
|
||||||
correlationId: string;
|
correlationId: string;
|
||||||
|
error: unknown;
|
||||||
}): Promise<void> {
|
}): Promise<void> {
|
||||||
|
const reason = formatFailureReason(input.error, input.timeoutMs);
|
||||||
|
|
||||||
try {
|
try {
|
||||||
await postTimeoutFailureComment({
|
await postReviewFailureComment({
|
||||||
gitea: input.gitea,
|
gitea: input.gitea,
|
||||||
owner: input.owner,
|
owner: input.owner,
|
||||||
repo: input.repo,
|
repo: input.repo,
|
||||||
prNumber: input.prNumber,
|
prNumber: input.prNumber,
|
||||||
timeoutMs: input.timeoutMs
|
reason
|
||||||
});
|
});
|
||||||
log("info", "Posted timeout failure comment on PR", {
|
log("info", "Posted review failure comment on PR", {
|
||||||
correlation_id: input.correlationId,
|
correlation_id: input.correlationId,
|
||||||
pr_number: input.prNumber
|
pr_number: input.prNumber
|
||||||
});
|
});
|
||||||
} catch (error) {
|
} catch (postError) {
|
||||||
log("warn", "Failed to post timeout failure comment on PR", {
|
log("warn", "Failed to post review failure comment on PR", {
|
||||||
correlation_id: input.correlationId,
|
correlation_id: input.correlationId,
|
||||||
pr_number: input.prNumber,
|
pr_number: input.prNumber,
|
||||||
error: error instanceof Error ? error.message : String(error)
|
error: postError instanceof Error ? postError.message : String(postError)
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -286,15 +310,15 @@ async function handleTimeoutFailure(input: {
|
|||||||
prNumber: input.prNumber,
|
prNumber: input.prNumber,
|
||||||
botLogin: input.botLogin
|
botLogin: input.botLogin
|
||||||
});
|
});
|
||||||
log("info", "Removed bot from reviewers after timeout", {
|
log("info", "Removed bot from reviewers after review failure", {
|
||||||
correlation_id: input.correlationId,
|
correlation_id: input.correlationId,
|
||||||
pr_number: input.prNumber
|
pr_number: input.prNumber
|
||||||
});
|
});
|
||||||
} catch (error) {
|
} catch (removeError) {
|
||||||
log("warn", "Failed to remove bot from reviewers after timeout", {
|
log("warn", "Failed to remove bot from reviewers after review failure", {
|
||||||
correlation_id: input.correlationId,
|
correlation_id: input.correlationId,
|
||||||
pr_number: input.prNumber,
|
pr_number: input.prNumber,
|
||||||
error: error instanceof Error ? error.message : String(error)
|
error: removeError instanceof Error ? removeError.message : String(removeError)
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user