action: retry transient GitHub 422 "internal error" on review submission (#610)

* action: retry transient GitHub 422 "internal error" on review submission

GitHub sometimes 422s POST /pulls/{n}/reviews with body
"An internal error occurred, please try again." — a server-side hiccup
that the existing 422 handler framed with the generic
"likely causes (1)(2)(3)" prompt listing affected comments. the agent
dutifully refetched the diff, dropped comments, and resubmitted, hitting
the same transient error on a shifting affected-comments list until
GitHub accepted. some runs logged 8+ spurious retries with ~11 minutes
of wall-clock, dropping valid inline comments along the way.

detect the transient body explicitly, retry in-tool twice with 1s/3s
backoff, and surface a distinct error on exhaustion that tells the agent
this is a GitHub-side issue — do not modify inline comments, wait and
retry or fall back to a body-only review. closes #584.

* action: use retry util for transient review 422, drop isTransientReviewError tests

---------

Co-authored-by: pullfrog[bot] <226033991+pullfrog[bot]@users.noreply.github.com>

mcp/review.ts

@@ -11,6 +11,7 @@ import {
 } from "../utils/diffCoverage.ts";
 import { fixDoubleEscapedString } from "../utils/fixDoubleEscapedString.ts";
 import { patchWorkflowRunFields } from "../utils/patchWorkflowRunFields.ts";
+import { retry } from "../utils/retry.ts";
 import { deleteProgressComment } from "./comment.ts";
 import type { ToolContext } from "./server.ts";
 import { execute, tool } from "./shared.ts";
@@ -21,6 +22,29 @@ function getHttpStatus(err: unknown): number | undefined {
   return typeof status === "number" ? status : undefined;
 }
 
+/**
+ * detect GitHub's generic server-side 422 ("An internal error occurred,
+ * please try again.") that sometimes fires on `POST /pulls/{n}/reviews`.
+ *
+ * the body is stable across occurrences and distinct from every other 422
+ * cause we care about (anchor validation, body length, malformed suggestion
+ * blocks) — those all cite the specific problem. treating this as a
+ * transient server error unlocks bounded in-tool retry instead of surfacing
+ * it to the agent with the generic "likely causes (1)(2)(3)" prompt, which
+ * induces whack-a-mole comment dropping on content that was never the issue.
+ */
+export function isTransientReviewError(err: unknown): boolean {
+  if (getHttpStatus(err) !== 422) return false;
+  const msg = err instanceof Error ? err.message : String(err);
+  return /internal error occurred, please try again/i.test(msg);
+}
+
+// backoff schedule for transient GitHub 422 "internal error" responses on the
+// reviews endpoint. 3 attempts total (initial + 2 retries) with 1s/3s delays
+// — most transient GH errors clear within a few seconds, and longer delays
+// push review submission past agent-perceived responsiveness.
+export const TRANSIENT_REVIEW_RETRY_DELAYS_MS = [1_000, 3_000];
+
 type PullFile = RestEndpointMethodTypes["pulls"]["listFiles"]["response"]["data"][number];
 export type CommentableLines = { RIGHT: Set<number>; LEFT: Set<number> };
 
@@ -483,16 +507,50 @@ export function CreatePullRequestReviewTool(ctx: ToolContext) {
 
       // no body → single-step createReview (no footer needed)
       // has body → pending + submit so we can build footer with Fix links using review ID
+      //
+      // wrap the submission in `retry` so GitHub's transient 422 "internal
+      // error" body (distinct from anchor / body-length / suggestion 422s,
+      // which all cite the specific cause) clears on its own instead of
+      // surfacing through the generic 422 handler — that framing sent the
+      // agent dropping valid inline comments chasing a non-issue.
+      // `shouldRetry` scopes retries to the transient body only, so real
+      // validation 422s still fail fast.
       let result;
       try {
-        result = body
+        result = await retry(
-          ? await createAndSubmitWithFooter(ctx, params, {
+          () =>
-              body,
+            body
-              approved: approved ?? false,
+              ? createAndSubmitWithFooter(ctx, params, {
-              hasComments: (params.comments?.length ?? 0) > 0,
+                  body,
-            })
+                  approved: approved ?? false,
-          : await createReviewWithStrandedRecovery(ctx, params);
+                  hasComments: (params.comments?.length ?? 0) > 0,
+                })
+              : createReviewWithStrandedRecovery(ctx, params),
+          {
+            delaysMs: TRANSIENT_REVIEW_RETRY_DELAYS_MS,
+            shouldRetry: isTransientReviewError,
+            label: "review submission",
+          }
+        );
       } catch (err: unknown) {
+        // GitHub's transient 422 "internal error" is distinct from anchor /
+        // body-length / suggestion validation failures — framing it with the
+        // generic "likely causes (1)(2)(3)" prompt sends the agent dropping
+        // comments that were never the problem. after bounded in-tool retry
+        // we surface a dedicated message that tells the agent to wait-and-
+        // retry or fall back to a body-only review.
+        if (isTransientReviewError(err)) {
+          const rawMsg = err instanceof Error ? err.message : String(err);
+          throw new Error(
+            `GitHub returned a transient 422 "internal error" on the reviews endpoint after ${TRANSIENT_REVIEW_RETRY_DELAYS_MS.length + 1} attempts. ` +
+              `This is a GitHub-side issue, not a problem with your review content. ` +
+              `Do NOT modify or drop inline comments — their content is not the cause. ` +
+              `Wait ~30 seconds and call this tool once more with the SAME arguments. ` +
+              `If it still fails, submit a body-only review (move all inline feedback into \`body\` as text) so nothing is lost. ` +
+              `GitHub said: ${rawMsg}`,
+            { cause: err }
+          );
+        }
         if (getHttpStatus(err) !== 422 || !params.comments?.length) throw err;
 
         const details = params.comments.map((c) => {

utils/retry.ts

@@ -4,6 +4,12 @@ import { log } from "./cli.ts";
 export type RetryOptions = {
   maxAttempts?: number;
   delayMs?: number;
+  /**
+   * explicit delay schedule — one entry per retry (length N ⇒ N+1 attempts).
+   * when set, overrides `maxAttempts` and `delayMs`. e.g. `[1_000, 3_000]`
+   * means up to 3 attempts, sleeping 1s before retry 2 and 3s before retry 3.
+   */
+  delaysMs?: readonly number[];
   shouldRetry?: (error: unknown) => boolean;
   label?: string;
 };
@@ -20,10 +26,15 @@ const defaultShouldRetry = (error: unknown): boolean => {
 };
 
 export async function retry<T>(fn: () => Promise<T>, options: RetryOptions = {}): Promise<T> {
-  const maxAttempts = options.maxAttempts ?? 3;
-  const delayMs = options.delayMs ?? 1000;
   const shouldRetry = options.shouldRetry ?? defaultShouldRetry;
   const label = options.label ?? "operation";
+  const delays = options.delaysMs
+    ? Array.from(options.delaysMs)
+    : Array.from(
+        { length: (options.maxAttempts ?? 3) - 1 },
+        (_, i) => (options.delayMs ?? 1000) * (i + 1)
+      );
+  const maxAttempts = delays.length + 1;
 
   let lastError: unknown;
 
@@ -37,7 +48,7 @@ export async function retry<T>(fn: () => Promise<T>, options: RetryOptions = {})
         throw error;
       }
 
-      const delay = delayMs * attempt;
+      const delay = delays[attempt - 1]!;
       log.info(`» ${label} failed (attempt ${attempt}/${maxAttempts}), retrying in ${delay}ms...`);
       await sleep(delay);
     }