diff --git a/src/cursor/review-schema.ts b/src/cursor/review-schema.ts index 1ecbcc7..17c7d83 100644 --- a/src/cursor/review-schema.ts +++ b/src/cursor/review-schema.ts @@ -1,7 +1,18 @@ -import { z } from "zod"; +import { z, ZodError } from "zod"; -export const reviewSchema = z.object({ - verdict: z.enum(["approve", "request_changes", "comment"]), +export { ZodError }; + +const VERDICTS = ["approve", "request_changes", "comment"] as const; +type Verdict = (typeof VERDICTS)[number]; + +const EVENT_BY_VERDICT: Record = { + approve: "APPROVE", + request_changes: "REQUEST_CHANGES", + comment: "COMMENT" +}; + +const reviewSchema = z.object({ + verdict: z.enum(VERDICTS), event: z.enum(["APPROVE", "REQUEST_CHANGES", "COMMENT"]), body: z.string().min(1), comments: z.array( @@ -16,5 +27,74 @@ export const reviewSchema = z.object({ export type ReviewResult = z.infer; 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; + 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 = { + 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]; } diff --git a/src/gitea/comments-api.ts b/src/gitea/comments-api.ts index ea31573..ae701dc 100644 --- a/src/gitea/comments-api.ts +++ b/src/gitea/comments-api.ts @@ -37,28 +37,27 @@ export async function deleteProgressComment(input: { }); } -export const TIMEOUT_FAILURE_MARKER = ""; +export const FAILURE_COMMENT_MARKER = ""; -export function buildTimeoutFailureBody(timeoutMs: number): string { - const timeoutMinutes = Math.max(1, Math.round(timeoutMs / 60_000)); +export function buildReviewFailureBody(reason: string): string { return ( - `${TIMEOUT_FAILURE_MARKER}\n\n` + - `🤖 **PR review failed:** the automated review timed out after ${timeoutMinutes} minute(s). ` + + `${FAILURE_COMMENT_MARKER}\n\n` + + `🤖 **PR review failed:** ${reason} ` + "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; owner: string; repo: string; prNumber: number; - timeoutMs: number; + reason: string; }): Promise { await input.gitea.createIssueComment( input.owner, input.repo, input.prNumber, - buildTimeoutFailureBody(input.timeoutMs) + buildReviewFailureBody(input.reason) ); } diff --git a/src/prompt/build-review-prompt.ts b/src/prompt/build-review-prompt.ts index e01a609..3f7a9db 100644 --- a/src/prompt/build-review-prompt.ts +++ b/src/prompt/build-review-prompt.ts @@ -22,7 +22,8 @@ export function buildReviewPrompt(input: { return [ "You are a senior code reviewer for this pull request.", "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.`, "Inline comments MUST have a changed file path and new_position >= 1.", "", diff --git a/src/run/review-runner.ts b/src/run/review-runner.ts index e10f0b1..2eb11e2 100644 --- a/src/run/review-runner.ts +++ b/src/run/review-runner.ts @@ -1,13 +1,14 @@ import { Env } from "../config/env.js"; import { loadRepoConfig } from "../config/load-repo-config.js"; import { runCursorReview } from "../cursor/review-agent.js"; +import { ZodError } from "../cursor/review-schema.js"; import { DedupeStore } from "../domain/dedupe-store.js"; import { shouldProcessEvent } from "../domain/should-process-event.js"; import { GiteaClient } from "../gitea/client.js"; import { deleteProgressComment, postProgressComment, - postTimeoutFailureComment + postReviewFailureComment } from "../gitea/comments-api.js"; import { deletePriorBotReviews, postReview } from "../gitea/review-api.js"; import { removeBotFromReviewers } from "../gitea/reviewer-api.js"; @@ -164,7 +165,7 @@ async function executeReview(params: { initialDelayMs: 500, operationName: "runCursorReview", correlationId: input.correlationId, - shouldRetry: (error) => !isTimeoutError(error) + shouldRetry: (error) => !isNonRetryableCursorError(error) }); await deletePriorBotReviews({ @@ -220,17 +221,16 @@ async function executeReview(params: { }); return "success"; } catch (error) { - if (isTimeoutError(error)) { - await handleTimeoutFailure({ - gitea, - owner, - repo, - prNumber, - botLogin: input.env.GITEA_BOT_LOGIN, - timeoutMs: input.env.REVIEW_TIMEOUT_MS, - correlationId: input.correlationId - }); - } + await handleReviewFailure({ + gitea, + owner, + repo, + prNumber, + botLogin: input.env.GITEA_BOT_LOGIN, + timeoutMs: input.env.REVIEW_TIMEOUT_MS, + correlationId: input.correlationId, + error + }); throw error; } finally { if (progressCommentId !== undefined) { @@ -249,7 +249,28 @@ function isTimeoutError(error: unknown): boolean { 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; owner: string; repo: string; @@ -257,24 +278,27 @@ async function handleTimeoutFailure(input: { botLogin: string; timeoutMs: number; correlationId: string; + error: unknown; }): Promise { + const reason = formatFailureReason(input.error, input.timeoutMs); + try { - await postTimeoutFailureComment({ + await postReviewFailureComment({ gitea: input.gitea, owner: input.owner, repo: input.repo, 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, pr_number: input.prNumber }); - } catch (error) { - log("warn", "Failed to post timeout failure comment on PR", { + } catch (postError) { + log("warn", "Failed to post review failure comment on PR", { correlation_id: input.correlationId, 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, botLogin: input.botLogin }); - log("info", "Removed bot from reviewers after timeout", { + log("info", "Removed bot from reviewers after review failure", { correlation_id: input.correlationId, pr_number: input.prNumber }); - } catch (error) { - log("warn", "Failed to remove bot from reviewers after timeout", { + } catch (removeError) { + log("warn", "Failed to remove bot from reviewers after review failure", { correlation_id: input.correlationId, pr_number: input.prNumber, - error: error instanceof Error ? error.message : String(error) + error: removeError instanceof Error ? removeError.message : String(removeError) }); } }