refactor progress comments into a bundled type + helper module (#567 )

* refactor progress comments into a single bundled type + helper module introduce ProgressComment ({ id, type: "issue" | "review" }) as the canonical handle for the GitHub comment a run uses to report progress, and route every read/update/delete/create through a single helper module (action/utils/progressComment.ts). previously every site that touched the progress comment hardcoded octokit.rest.issues.*Comment, which made adding a second comment type (review-thread replies) require duplicating the same branch in 6+ places — the same shape that bit pullfrog/app#445. new capability: when the address-reviews trigger fires for a one-off review comment, the "Leaping into action" comment is now posted as a reply in that review thread instead of as a top-level PR timeline comment. the helper handles failure (e.g. parent comment deleted) by silently falling back to a top-level issue comment, so the run never loses its progress surface. changes: - action/utils/progressComment.ts (new) — ProgressComment type + getProgressComment, updateProgressComment, deleteProgressCommentApi, createLeapingProgressComment. uses a structural Octokit interface to bridge the @octokit/rest version mismatch between the action package (v22) and the root project (v21). - action/internal/index.ts — re-export the new types and helpers for cross-boundary use. - action/external.ts, action/utils/payload.ts — replace progressCommentId: string with progressComment: { id: string, type: "issue" | "review" } in WriteablePayload + JsonPayload. wire-format breaking, no legacy fallback (in-flight runs across the deploy lose their progress comment, fine). - action/mcp/server.ts — ToolState.progressCommentId becomes progressComment: ProgressComment | null | undefined (same tristate semantics). - action/main.ts, action/mcp/comment.ts, action/utils/errorReport.ts, action/utils/postCleanup.ts — every issues.*Comment call against the progress comment routes through the helper module. zero hardcoded API branching outside the helper. - utils/github/triggerWorkflow.ts — drop createLeapingComment + updateCommentToLeaping; dispatchAndTrackWorkflow gains a resolution chain (existingComment → replyToReviewComment → triggeringIssue → none) and an existingComment: ProgressComment param plus replyToReviewComment: { pullNumber, commentId }. - utils/webhooks/handleWebhook.ts — dispatch closure threads replyToReviewComment through; the one-off review comment branch passes it and skips the now-redundant eyes reaction on the comment we're about to reply to. - app/trigger/[owner]/[repo]/[number]/page.tsx, utils/github/runActionLocal.ts, app/api/cli/dispatch/route.ts, app/api/dispatch-workflow/route.ts — call sites updated to new shape. no schema or DB column changes. the existing WorkflowRun.progressCommentId column is still written by id only; type lives only on the in-flight payload, which is sufficient for runtime since it's the only thing that needs to know which API to call. * anneal pass 1: fallback visibility + stale doc/comment updates - progressComment.ts: when reviewReply→issue fallback fires, prepend a [!NOTE] callout with a permalink back to the original review comment. without this, the parent comment showed no eyes reaction (deliberately skipped) and no reply, leaving the user with no signal that anything happened. - wiki/post-cleanup.md: update progressCommentId references to progressComment, document the new helper-based dispatch by type. - wiki/main.md: update initToolState({ progressCommentId }) → ({ progressComment }) in the resolver-chain diagram. - action/main.ts, action/mcp/review.ts: update two stale comments that referenced the old field name. * anneal pass 2: post-cleanup detection through fallback notice + log cleanup - isLeapingIntoActionCommentBody: strip a leading GFM blockquote/alert before testing the leaping prefix. without this, the [!NOTE] callout that the reviewReply→issue fallback prepends would prevent post-cleanup from recognizing the stuck "Leaping into action..." comment, leaving it permanently on the PR timeline if the workflow died before any progress update. - progressComment helper: switch from log.warning (action-flavored, emits a ::warning:: GitHub Actions annotation) to console.warn so the helper doesn't pollute Vercel logs when invoked from the webhook context. - triggerWorkflow.ts: drop the duplicate caller-side log on review-reply failure — the helper already speaks loudly. Reword the catch-branch log to reflect that it now only fires when both the reply AND the helper's internal fallback failed. - progressComment.ts: document that the [!NOTE] fallback notice is overwritten on the first report_progress call, and explain the trade-off vs persisting it through the action payload + ToolState. * debloat: drop the [!NOTE] fallback callout Reverting two pieces from the prior anneal pass: - progressComment.ts: drop the [!NOTE] callout that the reviewReply→issue fallback prepended to the leaping body. It disappeared on the agent's first report_progress call, which made it half-committed to visibility — worse than either properly persisting it (real engineering) or leaving the fallback silent (current choice). The console.warn diagnostic and the workflow-run footer link in the leaping comment itself give us enough signal for the rare case where both API endpoints fail at once. - isLeapingIntoActionCommentBody: revert the leading-blockquote stripping; only needed to compensate for the [!NOTE] callout. Keeping: the console.warn-vs-log.warning fix (real cross-runtime concern), the duplicate-log drop in triggerWorkflow.ts, the wiki updates, and the two stale source-comment fixes. * fix: prevent stranded task list overwriting post-cleanup message When a run is cancelled, the action's todoTracker may have an HTTP write in flight to GitHub when SIGTERM lands. The action process dies, but the request data has already left the socket — GitHub processes it and updates the comment body to the (stale) task list. Meanwhile post-cleanup, running in a separate process, writes the "This run was cancelled 🛑" message. If the tracker's in-flight write happens to land *after* post-cleanup's write, the user never sees the cancellation message. Two-layer fix: - Action side: cancel the tracker in the SIGTERM signal handler so no new debounced writes get scheduled. This shrinks the race window but can't un-send a request already on the wire. - Post-cleanup side: after writing, verify the body landed and re-issue if another write clobbered ours. Loops up to 3× with a 3s settle delay so delayed in-flight writes from the dying action have time to arrive before our read-back check decides whether to retry. * lint: import createLeapingProgressComment from pullfrog/internal in test script * address bot review findings: reply-target root, version bump, GET error handling Three real findings from the bot reviews on #567 plus a small DRY pass: 1. handleWebhook reply-target: `newComments[0]` may be a reply, not a top-level review comment. `getReviewCommentsWithReplies` returns root + replies for any thread the review touched, and `pull_request_review_id` filtering only narrows by *which review submitted*, not *root vs reply*. When a user submits a single reply as their entire review (e.g. replying to someone else's comment to ping @pullfrog), the reply ID flowed through to `createReplyForReviewComment`, which 422s on replies-to-replies and degraded to a top-level issue comment — exactly the polluted-PR-timeline behavior this PR was built to remove. Walk up `in_reply_to` from the already-fetched thread data to find the root and reply there instead. 2. action/package.json: bumped 0.0.202 → 0.0.204. main is at 0.0.203 and our wire format changed; without a bump validateCompatibility can't surface the mismatch on the deploy boundary, and the merge would have gone backwards. 3. postCleanup writeAndVerify: distinguish a thrown verify-GET from a "body got overwritten" mismatch. Treating a transient 5xx/rate-limit GET the same as a clobber wasted PUT attempts and printed a misleading "in-flight writes kept clobbering us" warning. We trust our PUT (which returned 200) and exit instead of amplifying writes against a flaky API. 4. Small DRY: extracted parseProgressComment for the `{ id: string; type } -> ProgressComment` parse that had drifted across server.ts and postCleanup.ts.
bump xai/grok to 4.3 and grok-fast to 4-1-fast
2026-05-06 01:50:58 +00:00 · 2026-05-05 23:40:00 +00:00 · 2026-05-05 23:25:46 +00:00 · 2026-05-05 21:59:40 +00:00 · 2026-05-05 19:33:46 +00:00
27 changed files with 1430 additions and 289 deletions
@@ -279,8 +279,8 @@ export interface WriteablePayload {
  timeout?: string | undefined;
  /** working directory for the agent */
  cwd?: string | undefined;
-  /** pre-created progress comment ID for updating status */
-  progressCommentId?: string | undefined;
+  /** pre-created progress comment (ID + type) for updating status */
+  progressComment?: { id: string; type: "issue" | "review" } | undefined;
 }

 // immutable payload type for agent execution
@@ -40,6 +40,17 @@ export {
  stripExistingFooter,
 } from "../utils/buildPullfrogFooter.ts";
 export type { ResourceUsage, UsageSummary } from "../utils/github.ts";
+export type {
+  CreateProgressCommentTarget,
+  ProgressComment,
+  ProgressCommentType,
+} from "../utils/progressComment.ts";
+export {
+  createLeapingProgressComment,
+  deleteProgressCommentApi,
+  getProgressComment,
+  updateProgressComment,
+} from "../utils/progressComment.ts";
 export {
  isValidTimeString,
  parseTimeString,
@@ -36,6 +36,7 @@ import { aggregateUsage, patchWorkflowRunFields } from "./utils/patchWorkflowRun
 import { resolvePayload, resolvePromptInput } from "./utils/payload.ts";
 import { postReviewCleanup } from "./utils/reviewCleanup.ts";
 import { handleAgentResult } from "./utils/run.ts";
+import { type AccountPlan, isInfraCovered } from "./utils/runContext.ts";
 import { resolveRunContextData } from "./utils/runContextData.ts";
 import { setEnvAllowlist } from "./utils/secrets.ts";
 import { createTempDirectory, setupGit } from "./utils/setup.ts";
@@ -111,6 +112,106 @@ interface OidcCredentials {
  requestToken: string;
 }

+/**
+ * Billing-layer error surfaced from `/api/proxy-token` as a 402. User-actionable
+ * — distinct from TransientError (503 / transient sync issue) so the job
+ * summary + PR comment can use affirmative "you need to do X" copy rather than
+ * the ambiguous "billing error" label that makes transient outages look like
+ * the user's fault.
+ *
+ * `code` is a server-side discriminator: `router_requires_card` (no card + no
+ * wallet balance on Router), or null for unclassified. `declineCode` is
+ * Stripe's more specific sub-reason on `card_declined` (e.g.
+ * `insufficient_funds`, `lost_card`). `needsReauthentication` is the 3DS case
+ * broken out for convenience.
+ */
+class BillingError extends Error {
+  code: string | null;
+  declineCode: string | null;
+  needsReauthentication: boolean;
+
+  constructor(
+    message: string,
+    opts: {
+      code?: string | null;
+      declineCode?: string | null;
+      needsReauthentication?: boolean;
+    } = {}
+  ) {
+    super(message);
+    this.name = "BillingError";
+    this.code = opts.code ?? null;
+    this.declineCode = opts.declineCode ?? null;
+    this.needsReauthentication = opts.needsReauthentication ?? false;
+  }
+}
+
+/**
+ * Transient service failures from `/api/proxy-token` (503: partial OpenRouter
+ * usage sync, DB flake, in-flight payment intent). Not the user's fault — the
+ * summary uses "temporarily unavailable" framing, and the non-zero exit lets
+ * GH Actions apply whatever retry policy the workflow has configured.
+ */
+class TransientError extends Error {
+  constructor(message: string) {
+    super(message);
+    this.name = "TransientError";
+  }
+}
+
+/**
+ * Render a BillingError as user-facing markdown (shared between GH job summary
+ * and the PR progress comment). Branches:
+ *
+ *   - `router_requires_card`: the user is on Router mode with no card AND
+ *     no wallet balance. Points at the add-card flow in the console.
+ *   - `needsReauthentication` (Stripe `authentication_required` decline): the
+ *     issuer requires a 3D Secure challenge on each off-session charge —
+ *     re-adding the card won't help because the issuer's policy persists
+ *     across credentials. The escape valve is a manual top-up from the
+ *     dashboard, where 3DS runs interactively inside Stripe Checkout.
+ *   - default: generic "manage billing" with the declineCode appended if
+ *     classified (insufficient_funds, lost_card, generic_decline, etc).
+ */
+function formatBillingErrorSummary(error: BillingError): string {
+  if (error.code === "router_requires_card") {
+    return [
+      "### ⛔ Pullfrog Router requires a card",
+      "",
+      "This run was going to use Pullfrog Router, which bills at raw OpenRouter cost and needs a card on file. Runs won't proceed until a card is added.",
+      "",
+      "[Add a card →](https://pullfrog.com/console#model-access) — your first $20 of Router usage is free.",
+    ].join("\n");
+  }
+
+  if (error.needsReauthentication) {
+    return [
+      "### ❌ Pullfrog billing error — card requires 3DS on every charge",
+      "",
+      `Your card issuer requires a 3D Secure challenge on each off-session charge (\`${error.declineCode ?? "authentication_required"}\`), which we can't run from the agent. Top up your Router credit balance manually — 3DS runs interactively in Stripe Checkout, and subsequent runs draw from the prepaid balance without triggering another off-session charge.`,
+      "",
+      "[Top up your Router credit balance →](https://pullfrog.com/console)",
+    ].join("\n");
+  }
+
+  const codeSuffix = error.declineCode ? ` (\`${error.declineCode}\`)` : "";
+  return `### ❌ Pullfrog billing error\n\n${error.message}${codeSuffix}\n\n[Manage billing →](https://pullfrog.com/console)`;
+}
+
+/**
+ * Render a TransientError as user-facing markdown. Distinct framing from
+ * BillingError so the user doesn't read "❌" and assume their card failed.
+ */
+function formatTransientErrorSummary(error: TransientError): string {
+  return [
+    "### ⚠️ Pullfrog temporarily unavailable",
+    "",
+    error.message,
+    "",
+    "This is typically transient — the next dispatch should succeed. If it persists, check [status.pullfrog.com](https://status.pullfrog.com).",
+  ].join("\n");
+}
+
 async function mintProxyKey(ctx: { oidcCredentials: OidcCredentials }): Promise<string | null> {
  try {
    process.env.ACTIONS_ID_TOKEN_REQUEST_URL = ctx.oidcCredentials.requestUrl;
@@ -125,6 +226,31 @@ async function mintProxyKey(ctx: { oidcCredentials: OidcCredentials }): Promise<
      headers: { Authorization: `Bearer ${oidcToken}` },
    });

+    if (response.status === 402) {
+      const body = (await response.json().catch(() => null)) as {
+        error?: string;
+        code?: string;
+        declineCode?: string;
+        needsReauthentication?: boolean;
+      } | null;
+      throw new BillingError(body?.error ?? "insufficient balance", {
+        code: body?.code ?? null,
+        declineCode: body?.declineCode ?? null,
+        needsReauthentication: body?.needsReauthentication ?? false,
+      });
+    }
+
+    // 503 = transient sync issue (partial OpenRouter failure, DB flake,
+    // in-flight top-up). Not the user's fault — TransientError renders a
+    // "temporarily unavailable" summary instead of the "billing error"
+    // label that BillingError uses.
+    if (response.status === 503) {
+      const body = (await response.json().catch(() => null)) as { error?: string } | null;
+      throw new TransientError(
+        body?.error ?? "billing service temporarily unavailable — retry shortly"
+      );
+    }
+
    if (!response.ok) {
      log.warning(`proxy key mint failed (${response.status})`);
      return null;
@@ -133,6 +259,8 @@ async function mintProxyKey(ctx: { oidcCredentials: OidcCredentials }): Promise<
    const data = (await response.json()) as { key: string };
    return data.key;
  } catch (error) {
+    if (error instanceof BillingError) throw error;
+    if (error instanceof TransientError) throw error;
    log.warning(`proxy key mint error: ${error instanceof Error ? error.message : String(error)}`);
    return null;
  } finally {
@@ -144,29 +272,29 @@ async function mintProxyKey(ctx: { oidcCredentials: OidcCredentials }): Promise<
 async function resolveProxyModel(ctx: {
  payload: ResolvedPayload;
  oss: boolean;
+  plan: AccountPlan;
  proxyModel?: string | undefined;
  oidcCredentials: OidcCredentials | null;
 }): Promise<void> {
  // env override = BYOK escape hatch, don't proxy
  if (process.env.PULLFROG_MODEL?.trim()) return;

-  // OSS: server decided the model
-  if (ctx.oss && ctx.proxyModel) {
-    if (!ctx.oidcCredentials) {
-      log.warning("» oss repo but no OIDC credentials available — skipping proxy");
-      return;
-    }
-    const key = await mintProxyKey({ oidcCredentials: ctx.oidcCredentials });
-    if (!key) return;
+  const needsProxy = isInfraCovered({ isOss: ctx.oss, plan: ctx.plan }) && ctx.proxyModel;
+  if (!needsProxy) return;

-    process.env.OPENROUTER_API_KEY = key;
-    core.setSecret(key);
-    ctx.payload.proxyModel = ctx.proxyModel;
-    log.info(`» proxy: oss → ${ctx.proxyModel}`);
+  if (!ctx.oidcCredentials) {
+    log.warning("» proxy requested but no OIDC credentials available — skipping");
    return;
  }

-  // managed billing will add its path here later
+  const key = await mintProxyKey({ oidcCredentials: ctx.oidcCredentials });
+  if (!key) return;
+
+  process.env.OPENROUTER_API_KEY = key;
+  core.setSecret(key);
+  ctx.payload.proxyModel = ctx.proxyModel;
+  const label = ctx.oss ? "oss" : "router";
+  log.info(`» proxy: ${label} → ${ctx.proxyModel}`);
 }

 async function writeJobSummary(toolState: ToolState): Promise<void> {
@@ -191,12 +319,12 @@ export async function main(): Promise<MainResult> {
  let activityTimeout: ActivityTimeout | null = null;
  let safetyNetTimer: NodeJS.Timeout | undefined;

-  // parse prompt early to extract progressCommentId for toolState
+  // parse prompt early to extract progressComment for toolState
  const resolvedPromptInput = resolvePromptInput();

  const toolState = initToolState({
-    progressCommentId:
-      typeof resolvedPromptInput !== "string" ? resolvedPromptInput.progressCommentId : undefined,
+    progressComment:
+      typeof resolvedPromptInput !== "string" ? resolvedPromptInput.progressComment : undefined,
  });

  // resolve and fingerprint git binary before any agent code runs
@@ -251,13 +379,39 @@ export async function main(): Promise<MainResult> {
    delete process.env.ACTIONS_ID_TOKEN_REQUEST_TOKEN;
  }

-  // proxy decision: mint an OpenRouter key for OSS repos (or later, managed billing)
-  await resolveProxyModel({
-    payload,
-    oss: runContext.oss,
-    proxyModel: runContext.proxyModel,
-    oidcCredentials,
-  });
+  // Proxy decision: mint an OpenRouter key for OSS repos or managed billing
+  // accounts. BillingError (402) and TransientError (503) both surface here.
+  // Handle explicitly so the user sees an actionable message (job summary +
+  // PR progress comment when one exists) — otherwise the error unwinds past
+  // the main try/catch (which needs toolState) and lands in runMain with only
+  // a generic core.setFailed.
+  try {
+    await resolveProxyModel({
+      payload,
+      oss: runContext.oss,
+      plan: runContext.plan,
+      proxyModel: runContext.proxyModel,
+      oidcCredentials,
+    });
+  } catch (error) {
+    if (error instanceof BillingError) {
+      const summary = formatBillingErrorSummary(error);
+      await writeSummary(summary).catch(() => {});
+      // Mirror to the PR progress comment if the trigger created one
+      // (mention / PR event). Without this, auto-reload declines are only
+      // visible in the job summary — users rarely open that, so the agent
+      // just appears to silently stop mid-run.
+      await reportErrorToComment({ toolState, error: summary }).catch(() => {});
+      throw error;
+    }
+    if (error instanceof TransientError) {
+      const summary = formatTransientErrorSummary(error);
+      await writeSummary(summary).catch(() => {});
+      await reportErrorToComment({ toolState, error: summary }).catch(() => {});
+      throw error;
+    }
+    throw error;
+  }

  // create octokit with MCP token for GitHub API calls
  const octokit = createOctokit(tokenRef.mcpToken);
@@ -350,6 +504,8 @@ export async function main(): Promise<MainResult> {
      jobId: runInfo.jobId,
      mcpServerUrl: "",
      tmpdir,
+      oss: runContext.oss,
+      plan: runContext.plan,
      resolvedModel,
    };
    await using mcpHttpServer = await startMcpHttpServer(toolContext, { outputSchema });
@@ -421,6 +577,15 @@ export async function main(): Promise<MainResult> {
    });
    toolState.todoTracker = todoTracker;

+    // on cancellation, stop scheduling new tracker writes immediately. without this, a
+    // debounced write queued just before SIGTERM could land at GitHub *after* the post-step
+    // writes its "This run was cancelled" message, clobbering it back to the task list.
+    // we can't await in-flight writes (the process is exiting), but cancelling the timer
+    // shrinks the race window. post-cleanup has its own verify-retry loop for the rest.
+    onExitSignal(() => {
+      todoTracker?.cancel();
+    });
+
    // when the agent subprocess is killed for inner activity timeout, stop
    // the MCP HTTP server so mcp-proxy's SSE reconnect attempts don't keep
    // the outer activity timer alive. start a short safety-net timer — if
@@ -551,7 +716,7 @@ export async function main(): Promise<MainResult> {
    const trackerWasLastWriter = todoTracker?.hasPublished && !toolState.finalSummaryWritten;
    if (
      toolContext &&
-      toolState.progressCommentId &&
+      toolState.progressComment &&
      (!toolState.wasUpdated || trackerWasLastWriter)
    ) {
      await deleteProgressComment(toolContext).catch((error) => {
@@ -0,0 +1,110 @@
+{
+  "owner": "pullfrog",
+  "name": "scratch",
+  "pullNumber": 49,
+  "reviewId": 3485940013,
+  "review": {
+    "body": "### This is the final PR Bugbot will review for you during this billing cycle\n\nYour free Bugbot reviews will reset on November 30\n\n<details>\n<summary>Details</summary>\n\nYour team is on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle for each member of your team.\n\nTo receive Bugbot reviews on all of your PRs, visit the [Cursor dashboard](https://www.cursor.com/dashboard?tab=bugbot) to activate Pro and start your 14-day free trial.\n</details>\n\n",
+    "user": {
+      "login": "cursor[bot]"
+    }
+  },
+  "threads": [
+    {
+      "id": "PRRT_kwDOPaxxp85iysVl",
+      "path": ".github/workflows/test.yml",
+      "line": null,
+      "startLine": null,
+      "diffSide": "RIGHT",
+      "isResolved": true,
+      "isOutdated": true,
+      "comments": {
+        "nodes": [
+          {
+            "fullDatabaseId": "2544544046",
+            "body": "### Bug: GitHub Actions workflow triggered for wrong branch\n\n<!-- **High Severity** -->\n\n<!-- DESCRIPTION START -->\nThe `pull_request` trigger specifies `branches: [mainc]`, but the `push` trigger specifies `branches: [main]`. This mismatch means pull requests will only trigger tests if targeting a non-existent `mainc` branch rather than the actual `main` development branch, preventing CI from running on most pull requests.\n<!-- DESCRIPTION END -->\n\n<!-- LOCATIONS START\n.github/workflows/test.yml#L6-L7\nLOCATIONS END -->\n<a href=\"https://cursor.com/open?data=eyJhbGciOiJSUzI1NiIsInR5cCI6IkpXVCIsImtpZCI6ImJ1Z2JvdC12MSJ9.eyJ2ZXJzaW9uIjoxLCJ0eXBlIjoiQlVHQk9UX0ZJWF9JTl9DVVJTT1IiLCJkYXRhIjp7InJlZGlzS2V5IjoiYnVnYm90OjllMTgyY2U2LWY0YWMtNDAwNS1hMzQ4LWIyYzJkZTk4OGM1ZSIsImVuY3J5cHRpb25LZXkiOiJmSW93NEdsUGUwYlYtd3M2UC1UNHdHT1JmMGZjakxfWVZEdC00SWNveXo0IiwiYnJhbmNoIjoiZGl2aWRlIn0sImlhdCI6MTc2MzYyMDgxOSwiZXhwIjoxNzY0MjI1NjE5fQ.BjkWsTqiNriojI5v10JcveUY2M50f9eflTNDgWAdjdW9w7E0EEY4GJfyzBrA72neco3qAlc34WipASNuEQbTD1fZvwtJY-TeNTDzoKmwA6gtwICB8t7qT87GPvcbDrdGGWdC8kW1jf-LntTmD0k7gt0AeENRAdRSiD3dbqYFN0huXHaB8f2Y48mpmLcnnUpoaaZe7By-Y0DnILyHppwx3AH75nKE_ZeAee3rQNGX4cwcHgB5emTSM93pMDQhT1vbIRYHMaFkOaW2-kDOA8H2QqxD4mT8VzY3skvxIo5HNZCvqE84NtEygHqkBv88g2EEijOPAAeskfsdp087yIzV9g\"><picture><source media=\"(prefers-color-scheme: dark)\" srcset=\"https://cursor.com/fix-in-cursor-dark.svg\"><source media=\"(prefers-color-scheme: light)\" srcset=\"https://cursor.com/fix-in-cursor-light.svg\"><img alt=\"Fix in Cursor\" src=\"https://cursor.com/fix-in-cursor.svg\"></picture></a>&nbsp;<a href=\"https://cursor.com/agents?data=eyJhbGciOiJSUzI1NiIsInR5cCI6IkpXVCIsImtpZCI6ImJ1Z2JvdC12MSJ9.eyJ2ZXJzaW9uIjoxLCJ0eXBlIjoiQlVHQk9UX0ZJWF9JTl9XRUIiLCJkYXRhIjp7InJlZGlzS2V5IjoiYnVnYm90OjllMTgyY2U2LWY0YWMtNDAwNS1hMzQ4LWIyYzJkZTk4OGM1ZSIsImVuY3J5cHRpb25LZXkiOiJmSW93NEdsUGUwYlYtd3M2UC1UNHdHT1JmMGZjakxfWVZEdC00SWNveXo0IiwiYnJhbmNoIjoiZGl2aWRlIiwicmVwb093bmVyIjoicHVsbGZyb2dhaSIsInJlcG9OYW1lIjoic2NyYXRjaCIsInByTnVtYmVyIjo0OSwiY29tbWl0U2hhIjoiNThiOGJmNmQ1MWE1Mjg4OGFjNGFkNzA5YWVmYTk2MWFkZDMyNDBiMSJ9LCJpYXQiOjE3NjM2MjA4MTksImV4cCI6MTc2NDIyNTYxOX0.SFDZe8R9uwhPjS55J4i_mV2ybsSZoQYM6YzdUOava4IKy1IK2OrVkVsG3-8p4rRaMBXdDZZ4ObPbtk70KqdAiLEDKBqaFcWqELc49lr0XRKUmu4F6EhESFQOvt7MLSVDIOgee8YRlhS6xtoPDqsRiV2KGOwyLEdCeYdrYz9i1DanIswWSoMRVvkjxZ6GUBYVAUg_JsgAXoKVJ-L9Q5Ygho6acVAr5NlGeBp2f6g49GX4GfDOPeV3SORQS1CjxQVRbjI-g0rW55NIisBEl8279VwG6-dTISNbyasZOB6R3eEmC4vmyAAGJjUsMwqhMPw1oaMMmYNSbtZLDESxME9IUg\"><picture><source media=\"(prefers-color-scheme: dark)\" srcset=\"https://cursor.com/fix-in-web-dark.svg\"><source media=\"(prefers-color-scheme: light)\" srcset=\"https://cursor.com/fix-in-web-light.svg\"><img alt=\"Fix in Web\" src=\"https://cursor.com/fix-in-web.svg\"></picture></a>\n\n",
+            "createdAt": "2025-11-20T06:40:19Z",
+            "diffHunk": "@@ -0,0 +1,36 @@\n+name: Test\n+\n+on:\n+  push:\n+    branches: [main]\n+  pull_request:\n+    branches: [mainc]",
+            "line": null,
+            "startLine": null,
+            "originalLine": 7,
+            "originalStartLine": null,
+            "author": {
+              "login": "cursor"
+            },
+            "pullRequestReview": {
+              "databaseId": 3485940013,
+              "author": {
+                "login": "cursor"
+              }
+            },
+            "reactionGroups": [
+              {
+                "content": "THUMBS_UP",
+                "reactors": {
+                  "nodes": []
+                }
+              },
+              {
+                "content": "THUMBS_DOWN",
+                "reactors": {
+                  "nodes": []
+                }
+              },
+              {
+                "content": "LAUGH",
+                "reactors": {
+                  "nodes": []
+                }
+              },
+              {
+                "content": "HOORAY",
+                "reactors": {
+                  "nodes": []
+                }
+              },
+              {
+                "content": "CONFUSED",
+                "reactors": {
+                  "nodes": []
+                }
+              },
+              {
+                "content": "HEART",
+                "reactors": {
+                  "nodes": []
+                }
+              },
+              {
+                "content": "ROCKET",
+                "reactors": {
+                  "nodes": []
+                }
+              },
+              {
+                "content": "EYES",
+                "reactors": {
+                  "nodes": []
+                }
+              }
+            ]
+          }
+        ]
+      }
+    }
+  ],
+  "prFiles": [
+    {
+      "filename": ".github/workflows/test.yml",
+      "patch": "@@ -0,0 +1,36 @@\n+name: Test\n+\n+on:\n+  push:\n+    branches: [main]\n+  pull_request:\n+    branches: [main]\n+\n+jobs:\n+  test:\n+    runs-on: ubuntu-latest\n+\n+    strategy:\n+      matrix:\n+        node-version: [22.x]\n+\n+    steps:\n+      - name: Checkout code\n+        uses: actions/checkout@v4\n+\n+      - name: Setup pnpm\n+        uses: pnpm/action-setup@v2\n+        with:\n+          version: 8\n+\n+      - name: Setup Node.js ${{ matrix.node-version }}\n+        uses: actions/setup-node@v4\n+        with:\n+          node-version: ${{ matrix.node-version }}\n+          cache: 'pnpm'\n+\n+      - name: Install dependencies\n+        run: pnpm install\n+\n+      - name: Run tests\n+        run: pnpm test"
+    },
+    {
+      "filename": "index.test.ts",
+      "patch": "@@ -1,5 +1,5 @@\n import { describe, it, expect } from 'vitest'\n-import { add } from './index.js'\n+import { add, multiply, subtract, divide } from './index.js'\n \n describe('add function', () => {\n   it('should add two positive numbers correctly', () => {\n@@ -25,3 +25,51 @@ describe('add function', () => {\n     expect(add(0.1, 0.2)).toBeCloseTo(0.3)\n   })\n })\n+\n+describe('multiply function', () => {\n+  it('should multiply two positive numbers correctly', () => {\n+    expect(multiply(3, 4)).toBe(12)\n+  })\n+\n+  it('should multiply negative numbers correctly', () => {\n+    expect(multiply(-2, 3)).toBe(-6)\n+    expect(multiply(-2, -3)).toBe(6)\n+  })\n+\n+  it('should handle zero correctly', () => {\n+    expect(multiply(5, 0)).toBe(0)\n+    expect(multiply(0, 5)).toBe(0)\n+  })\n+})\n+\n+describe('subtract function', () => {\n+  it('should subtract two positive numbers correctly', () => {\n+    expect(subtract(10, 3)).toBe(7)\n+  })\n+\n+  it('should handle negative numbers correctly', () => {\n+    expect(subtract(5, -3)).toBe(8)\n+    expect(subtract(-5, 3)).toBe(-8)\n+  })\n+\n+  it('should handle zero correctly', () => {\n+    expect(subtract(5, 0)).toBe(5)\n+    expect(subtract(0, 5)).toBe(-5)\n+  })\n+})\n+\n+describe('divide function', () => {\n+  it('should divide two positive numbers correctly', () => {\n+    expect(divide(10, 2)).toBe(5)\n+  })\n+\n+  it('should handle negative numbers correctly', () => {\n+    expect(divide(-10, 2)).toBe(-5)\n+    expect(divide(10, -2)).toBe(-5)\n+  })\n+\n+  it('should handle decimal results correctly', () => {\n+    expect(divide(10, 3)).toBeCloseTo(3.333, 2)\n+    expect(divide(7, 2)).toBe(3.5)\n+  })\n+})"
+    },
+    {
+      "filename": "index.ts",
+      "patch": "@@ -3,11 +3,13 @@ export function add(a: number, b: number) {\n }\n \n export function multiply(a: number, b: number) {\n-  // Bug: accidentally adding 1 to the result\n-  return a * b + 1;\n+  return a * b;\n }\n \n export function subtract(a: number, b: number) {\n-  // Bug: accidentally adding instead of subtracting\n-  return a + b;\n+  return a - b;\n+}\n+\n+export function divide(a: number, b: number) {\n+  return a / b;\n }"
+    }
+  ]
+}
@@ -0,0 +1,14 @@
+{
+  "owner": "pullfrog",
+  "name": "scratch",
+  "pullNumber": 64,
+  "reviewId": 3531000326,
+  "review": {
+    "body": "This PR looks great. The retry logic is well-implemented and the tests are comprehensive.",
+    "user": {
+      "login": "pullfrog[bot]"
+    }
+  },
+  "threads": [],
+  "prFiles": []
+}
@@ -0,0 +1,67 @@
+{
+  "owner": "pullfrog",
+  "name": "test-repo",
+  "pullNumber": 1,
+  "files": [
+    {
+      "sha": "a2d9c355792f1883c26d43d219db006b05781e4c",
+      "filename": "src/format.ts",
+      "status": "modified",
+      "additions": 12,
+      "deletions": 2,
+      "changes": 14,
+      "blob_url": "https://github.com/pullfrog/test-repo/blob/0311c0fb58fc7faa46e51c174394a4468f379681/src%2Fformat.ts",
+      "raw_url": "https://github.com/pullfrog/test-repo/raw/0311c0fb58fc7faa46e51c174394a4468f379681/src%2Fformat.ts",
+      "contents_url": "https://api.github.com/repos/pullfrog/test-repo/contents/src%2Fformat.ts?ref=0311c0fb58fc7faa46e51c174394a4468f379681",
+      "patch": "@@ -1,7 +1,17 @@\n-export function formatCurrency(amount: number) {\n-  return `$${amount.toFixed(2)}`;\n+export function formatCurrency(amount: number, currency = \"USD\") {\n+  return new Intl.NumberFormat(\"en-US\", {\n+    style: \"currency\",\n+    currency,\n+  }).format(amount);\n }\n \n export function formatPercent(value: number) {\n   return `${(value * 100).toFixed(1)}%`;\n }\n+\n+export function formatNumber(value: number, decimals = 2) {\n+  return new Intl.NumberFormat(\"en-US\", {\n+    minimumFractionDigits: decimals,\n+    maximumFractionDigits: decimals,\n+  }).format(value);\n+}"
+    },
+    {
+      "sha": "0786b9ce6870e65c644673745266e87eef057ce4",
+      "filename": "src/math.ts",
+      "status": "modified",
+      "additions": 5,
+      "deletions": 2,
+      "changes": 7,
+      "blob_url": "https://github.com/pullfrog/test-repo/blob/0311c0fb58fc7faa46e51c174394a4468f379681/src%2Fmath.ts",
+      "raw_url": "https://github.com/pullfrog/test-repo/raw/0311c0fb58fc7faa46e51c174394a4468f379681/src%2Fmath.ts",
+      "contents_url": "https://api.github.com/repos/pullfrog/test-repo/contents/src%2Fmath.ts?ref=0311c0fb58fc7faa46e51c174394a4468f379681",
+      "patch": "@@ -3,13 +3,16 @@ export function add(a: number, b: number) {\n }\n \n export function subtract(a: number, b: number) {\n-  return a + b; // bug: should be a - b\n+  return a - b;\n }\n \n export function multiply(a: number, b: number) {\n-  return a * b + 1; // bug: off by one\n+  return a * b;\n }\n \n export function divide(a: number, b: number) {\n+  if (b === 0) {\n+    throw new Error(\"division by zero\");\n+  }\n   return a / b;\n }"
+    },
+    {
+      "sha": "cf92d8f6562c1be779506fec1049f38c9206c869",
+      "filename": "src/old-module.ts",
+      "status": "removed",
+      "additions": 0,
+      "deletions": 4,
+      "changes": 4,
+      "blob_url": "https://github.com/pullfrog/test-repo/blob/91ef1048326ef786fbcf95f29b3e2555506d2d54/src%2Fold-module.ts",
+      "raw_url": "https://github.com/pullfrog/test-repo/raw/91ef1048326ef786fbcf95f29b3e2555506d2d54/src%2Fold-module.ts",
+      "contents_url": "https://api.github.com/repos/pullfrog/test-repo/contents/src%2Fold-module.ts?ref=91ef1048326ef786fbcf95f29b3e2555506d2d54",
+      "patch": "@@ -1,4 +0,0 @@\n-// this module is deprecated and will be removed\n-export function legacyHelper() {\n-  return \"old\";\n-}"
+    },
+    {
+      "sha": "a5bfb8a1be72e4f0816a5c4c83ee784a06559629",
+      "filename": "src/validate.ts",
+      "status": "added",
+      "additions": 11,
+      "deletions": 0,
+      "changes": 11,
+      "blob_url": "https://github.com/pullfrog/test-repo/blob/0311c0fb58fc7faa46e51c174394a4468f379681/src%2Fvalidate.ts",
+      "raw_url": "https://github.com/pullfrog/test-repo/raw/0311c0fb58fc7faa46e51c174394a4468f379681/src%2Fvalidate.ts",
+      "contents_url": "https://api.github.com/repos/pullfrog/test-repo/contents/src%2Fvalidate.ts?ref=0311c0fb58fc7faa46e51c174394a4468f379681",
+      "patch": "@@ -0,0 +1,11 @@\n+export function isPositive(n: number) {\n+  return n > 0;\n+}\n+\n+export function isInRange(value: number, min: number, max: number) {\n+  return value >= min && value <= max;\n+}\n+\n+export function isInteger(n: number) {\n+  return Number.isInteger(n);\n+}"
+    },
+    {
+      "sha": "5815895211d8e3355fdb77b9e216e73a248644d9",
+      "filename": "test/math.test.ts",
+      "status": "modified",
+      "additions": 4,
+      "deletions": 0,
+      "changes": 4,
+      "blob_url": "https://github.com/pullfrog/test-repo/blob/0311c0fb58fc7faa46e51c174394a4468f379681/test%2Fmath.test.ts",
+      "raw_url": "https://github.com/pullfrog/test-repo/raw/0311c0fb58fc7faa46e51c174394a4468f379681/test%2Fmath.test.ts",
+      "contents_url": "https://api.github.com/repos/pullfrog/test-repo/contents/test%2Fmath.test.ts?ref=0311c0fb58fc7faa46e51c174394a4468f379681",
+      "patch": "@@ -17,4 +17,8 @@ describe(\"math\", () => {\n   it(\"divides\", () => {\n     expect(divide(10, 2)).toBe(5);\n   });\n+\n+  it(\"throws on division by zero\", () => {\n+    expect(() => divide(1, 0)).toThrow(\"division by zero\");\n+  });\n });"
+    }
+  ]
+}
@@ -1,6 +1,6 @@
 // Vitest Snapshot v1, https://vitest.dev/guide/snapshot.html

-exports[`fetchAndFormatPrDiff > generates accurate TOC line numbers for pullfrog/test-repo#1 > content 1`] = `
+exports[`formatFilesWithLineNumbers > generates accurate TOC line numbers for pullfrog/test-repo#1 > content 1`] = `
 "## Files (5)
 - src/format.ts → lines 9-32 · diff-41c7b3ac268a3a1ae5c7be92f1230f600013b7170e44a693570ccbdb183ea36b
 - src/math.ts → lines 33-55 · diff-9c6e445a719b33e276684bdf95c69e617f0303638d44cf90d61295f2720ecc63
@@ -96,7 +96,7 @@ diff --git a/test/math.test.ts b/test/math.test.ts
 "
 `;

-exports[`fetchAndFormatPrDiff > generates accurate TOC line numbers for pullfrog/test-repo#1 > toc 1`] = `
+exports[`formatFilesWithLineNumbers > generates accurate TOC line numbers for pullfrog/test-repo#1 > toc 1`] = `
 "## Files (5)
 - src/format.ts → lines 9-32 · diff-41c7b3ac268a3a1ae5c7be92f1230f600013b7170e44a693570ccbdb183ea36b
 - src/math.ts → lines 33-55 · diff-9c6e445a719b33e276684bdf95c69e617f0303638d44cf90d61295f2720ecc63
@@ -1,6 +1,6 @@
 // Vitest Snapshot v1, https://vitest.dev/guide/snapshot.html

-exports[`getFormattedReviewThreads > formats body-only review > content 1`] = `
+exports[`formatReviewData > formats body-only review > content 1`] = `
 "# Review Threads (0) for PR #64 - Review 3531000326 by pullfrog[bot]

 ## Review Body
@@ -11,9 +11,9 @@ This PR looks great. The retry logic is well-implemented and the tests are compr
 "
 `;

-exports[`getFormattedReviewThreads > formats body-only review > toc 1`] = `""`;
+exports[`formatReviewData > formats body-only review > toc 1`] = `""`;

-exports[`getFormattedReviewThreads > formats thread blocks with TOC and correct line numbers > content 1`] = `
+exports[`formatReviewData > formats thread blocks with TOC and correct line numbers > content 1`] = `
 "# Review Threads (1) for PR #49 - Review 3485940013 by cursor[bot]

 ## TOC
@@ -68,4 +68,4 @@ LOCATIONS END -->
 "
 `;

-exports[`getFormattedReviewThreads > formats thread blocks with TOC and correct line numbers > toc 1`] = `"- .github/workflows/test.yml:7 → lines 25-52"`;
+exports[`formatReviewData > formats thread blocks with TOC and correct line numbers > toc 1`] = `"- .github/workflows/test.yml:7 → lines 25-52"`;
@@ -1,8 +1,7 @@
-import type { RestEndpointMethodTypes } from "@octokit/rest";
+import { readFileSync } from "node:fs";
+import { resolve } from "node:path";
 import { describe, expect, it } from "vitest";
-import { acquireNewToken, createOctokit } from "../utils/github.ts";
-import { fetchAndFormatPrDiff } from "./checkout.ts";
-import type { ToolContext } from "./server.ts";
+import { type FormatFilesResult, formatFilesWithLineNumbers } from "./checkout.ts";

 /**
 * parses TOC entries like "- src/math.ts → lines 7-42 · diff-<hex>" into structured data.
@@ -22,59 +21,50 @@ function parseTocEntries(toc: string) {
  return entries;
 }

-async function getToken(): Promise<string> {
-  // prefer explicit GH_TOKEN, fall back to acquiring one via GitHub App credentials
-  if (process.env.GH_TOKEN) return process.env.GH_TOKEN;
-  return await acquireNewToken();
+// fixture captured by action/scripts/refresh-test-fixtures.ts. running
+// the formatter against checked-in JSON keeps this test offline and
+// deterministic — re-fetch the fixture (with creds) when GitHub's
+// pulls.listFiles response shape changes, then review the snapshot diff.
+type DiffFixture = {
+  owner: string;
+  name: string;
+  pullNumber: number;
+  files: Parameters<typeof formatFilesWithLineNumbers>[0];
+};
+
+function loadFixture<T>(file: string): T {
+  return JSON.parse(readFileSync(resolve(import.meta.dirname, "__fixtures__", file), "utf-8")) as T;
 }

-describe("fetchAndFormatPrDiff", () => {
-  it(
-    "generates accurate TOC line numbers for pullfrog/test-repo#1",
-    { timeout: 30000 },
-    async () => {
-      const token = await getToken();
-      const octokit = createOctokit(token);
-      const ctx = {
-        octokit,
-        repo: {
-          owner: "pullfrog",
-          name: "test-repo",
-          data: {} as RestEndpointMethodTypes["repos"]["get"]["response"]["data"],
-        },
-      } as ToolContext;
-      const result = await fetchAndFormatPrDiff(ctx, 1);
+describe("formatFilesWithLineNumbers", () => {
+  it("generates accurate TOC line numbers for pullfrog/test-repo#1", () => {
+    const fx = loadFixture<DiffFixture>("pullfrog-test-repo-pr-1.diff.json");
+    const result: FormatFilesResult = formatFilesWithLineNumbers(fx.files);

-      // verify content includes TOC at the start
-      expect(result.content.startsWith(result.toc)).toBe(true);
+    expect(result.content.startsWith(result.toc)).toBe(true);

-      // parse TOC and validate every entry's line numbers against actual content
-      const contentLines = result.content.split("\n");
-      const tocEntries = parseTocEntries(result.toc);
-      expect(tocEntries.length).toBeGreaterThan(0);
+    const contentLines = result.content.split("\n");
+    const tocEntries = parseTocEntries(result.toc);
+    expect(tocEntries.length).toBeGreaterThan(0);

-      for (const entry of tocEntries) {
-        // line numbers are 1-indexed, arrays are 0-indexed
-        const firstLine = contentLines[entry.startLine - 1];
-        expect(firstLine).toBeDefined();
-        // first line of each file section should be the diff header
-        expect(firstLine).toBe(`diff --git a/${entry.filename} b/${entry.filename}`);
+    for (const entry of tocEntries) {
+      // line numbers are 1-indexed, arrays are 0-indexed
+      const firstLine = contentLines[entry.startLine - 1];
+      expect(firstLine).toBeDefined();
+      // first line of each file section should be the diff header
+      expect(firstLine).toBe(`diff --git a/${entry.filename} b/${entry.filename}`);

-        // endLine should be within bounds
-        expect(entry.endLine).toBeLessThanOrEqual(contentLines.length);
-      }
-
-      // verify adjacent files don't overlap and are contiguous
-      for (let i = 1; i < tocEntries.length; i++) {
-        const prev = tocEntries[i - 1];
-        const curr = tocEntries[i];
-        // current file starts right after previous file ends
-        expect(curr.startLine).toBe(prev.endLine + 1);
-      }
-
-      // snapshot the full output for regression detection
-      expect(result.toc).toMatchSnapshot("toc");
-      expect(result.content).toMatchSnapshot("content");
+      expect(entry.endLine).toBeLessThanOrEqual(contentLines.length);
    }
-  );
+
+    // verify adjacent files don't overlap and are contiguous
+    for (let i = 1; i < tocEntries.length; i++) {
+      const prev = tocEntries[i - 1];
+      const curr = tocEntries[i];
+      expect(curr.startLine).toBe(prev.endLine + 1);
+    }
+
+    expect(result.toc).toMatchSnapshot("toc");
+    expect(result.content).toMatchSnapshot("content");
+  });
 });
@@ -4,6 +4,11 @@ import { buildPullfrogFooter, stripExistingFooter } from "../utils/buildPullfrog
 import { log } from "../utils/cli.ts";
 import { fixDoubleEscapedString } from "../utils/fixDoubleEscapedString.ts";
 import { patchWorkflowRunFields } from "../utils/patchWorkflowRunFields.ts";
+import {
+  createLeapingProgressComment,
+  deleteProgressCommentApi,
+  updateProgressComment,
+} from "../utils/progressComment.ts";
 import type { ToolContext } from "./server.ts";
 import { execute, tool } from "./shared.ts";

@@ -184,12 +189,15 @@ export const ReportProgress = type({
 /**
 * Report progress to a GitHub comment.
 *
- * progressCommentId has three states:
+ * progressComment has three states:
 *   - undefined: no comment yet — will create one if an issue/PR target exists
- *   - number:    active comment — will update it in place
+ *   - object:    active comment — will update it in place via the right REST endpoint for its type
 *   - null:      deliberately deleted (e.g. after submitting a PR review) — skips silently
 *
 * The body is always tracked in lastProgressBody for the job summary regardless of comment state.
+ *
+ * The "existing plan comment" path always targets a top-level issue comment (plan comments are
+ * created by create_issue_comment with type:"Plan", never as review-thread replies).
 */
 export async function reportProgress(
  ctx: ToolContext,
@@ -212,6 +220,7 @@ export async function reportProgress(

  const issueNumber = ctx.payload.event.issue_number ?? ctx.toolState.issueNumber;
  const isPlanMode = ctx.toolState.selectedMode === "Plan";
+  const apiCtx = { octokit: ctx.octokit, owner: ctx.repo.owner, repo: ctx.repo.name };

  // when editing existing plan: update the plan comment from tool state (set by select_mode)
  if (target_plan_comment === true && ctx.toolState.existingPlanCommentId === undefined) {
@@ -225,63 +234,57 @@ export async function reportProgress(
    const footer = buildCommentFooter(ctx, customParts);
    const bodyWithFooter = `${bodyWithoutFooter}${footer}`;

-    const result = await ctx.octokit.rest.issues.updateComment({
-      owner: ctx.repo.owner,
-      repo: ctx.repo.name,
-      comment_id: commentId,
-      body: bodyWithFooter,
-    });
+    const result = await updateProgressComment(
+      apiCtx,
+      { id: commentId, type: "issue" },
+      bodyWithFooter
+    );

    ctx.toolState.wasUpdated = true;

-    if (isPlanMode && result.data.node_id) {
-      await patchWorkflowRunFields(ctx, { planCommentNodeId: result.data.node_id });
+    if (isPlanMode && result.node_id) {
+      await patchWorkflowRunFields(ctx, { planCommentNodeId: result.node_id });
    }

    return {
-      commentId: result.data.id,
-      url: result.data.html_url,
-      body: result.data.body || "",
+      commentId: result.id,
+      url: result.html_url,
+      body: result.body || "",
      action: "updated",
    };
  }

-  const existingCommentId = ctx.toolState.progressCommentId;
+  const existingComment = ctx.toolState.progressComment;

  // if we already have a progress comment, update it
-  if (existingCommentId) {
+  if (existingComment) {
    const customParts =
      isPlanMode && issueNumber !== undefined
-        ? [buildImplementPlanLink(ctx, issueNumber, existingCommentId)]
+        ? [buildImplementPlanLink(ctx, issueNumber, existingComment.id)]
        : undefined;

    const bodyWithoutFooter = stripExistingFooter(body);
    const footer = buildCommentFooter(ctx, customParts);
    const bodyWithFooter = `${bodyWithoutFooter}${footer}`;

-    const result = await ctx.octokit.rest.issues.updateComment({
-      owner: ctx.repo.owner,
-      repo: ctx.repo.name,
-      comment_id: existingCommentId,
-      body: bodyWithFooter,
-    });
+    const result = await updateProgressComment(apiCtx, existingComment, bodyWithFooter);

    ctx.toolState.wasUpdated = true;

-    if (isPlanMode && result.data.node_id) {
-      await patchWorkflowRunFields(ctx, { planCommentNodeId: result.data.node_id });
+    if (isPlanMode && result.node_id) {
+      await patchWorkflowRunFields(ctx, { planCommentNodeId: result.node_id });
    }

    return {
-      commentId: result.data.id,
-      url: result.data.html_url,
-      body: result.data.body || "",
+      commentId: result.id,
+      url: result.html_url,
+      body: result.body || "",
      action: "updated",
    };
  }

  // null = progress comment was deleted by stranded-comment cleanup in main.ts
-  if (existingCommentId === null) {
+  if (existingComment === null) {
    return { body, action: "skipped" };
  }

@@ -294,49 +297,43 @@ export async function reportProgress(
  }

  // for new comments, we need to create first, then update with Plan link if in Plan mode
+  // self-created progress comments are always top-level issue comments — review-reply
+  // progress comments only originate from the dispatch path and arrive pre-created.
  const initialBody = addFooter(ctx, body);
+  const created = await createLeapingProgressComment(
+    apiCtx,
+    { kind: "issue", issueNumber },
+    initialBody
+  );

-  const result = await ctx.octokit.rest.issues.createComment({
-    owner: ctx.repo.owner,
-    repo: ctx.repo.name,
-    issue_number: issueNumber,
-    body: initialBody,
-  });
-
-  // store the comment ID for future updates
-  ctx.toolState.progressCommentId = result.data.id;
+  ctx.toolState.progressComment = created.comment;
  ctx.toolState.wasUpdated = true;

  // if Plan mode, update the comment to add the "Implement plan" link
  if (isPlanMode) {
-    const customParts = [buildImplementPlanLink(ctx, issueNumber, result.data.id)];
+    const customParts = [buildImplementPlanLink(ctx, issueNumber, created.comment.id)];
    const bodyWithoutFooter = stripExistingFooter(body);
    const footer = buildCommentFooter(ctx, customParts);
    const bodyWithPlanLink = `${bodyWithoutFooter}${footer}`;

-    const updateResult = await ctx.octokit.rest.issues.updateComment({
-      owner: ctx.repo.owner,
-      repo: ctx.repo.name,
-      comment_id: result.data.id,
-      body: bodyWithPlanLink,
-    });
+    const updateResult = await updateProgressComment(apiCtx, created.comment, bodyWithPlanLink);

-    if (updateResult.data.node_id) {
-      await patchWorkflowRunFields(ctx, { planCommentNodeId: updateResult.data.node_id });
+    if (updateResult.node_id) {
+      await patchWorkflowRunFields(ctx, { planCommentNodeId: updateResult.node_id });
    }

    return {
-      commentId: updateResult.data.id,
-      url: updateResult.data.html_url,
-      body: updateResult.data.body || "",
+      commentId: updateResult.id,
+      url: updateResult.html_url,
+      body: updateResult.body || "",
      action: "created",
    };
  }

  return {
-    commentId: result.data.id,
-    url: result.data.html_url,
-    body: result.data.body || "",
+    commentId: created.comment.id,
+    url: created.html_url,
+    body: created.body || "",
    action: "created",
  };
 }
@@ -393,20 +390,19 @@ export function ReportProgressTool(ctx: ToolContext) {
 * Delete the progress comment if it exists.
 * Used by main.ts for stranded-comment cleanup (orphaned "Leaping into action" or
 * checklist left by the todo tracker when the agent didn't call report_progress).
- * Sets progressCommentId to null so subsequent report_progress calls are no-ops.
+ * Sets progressComment to null so subsequent report_progress calls are no-ops.
 */
 export async function deleteProgressComment(ctx: ToolContext): Promise<boolean> {
-  const existingCommentId = ctx.toolState.progressCommentId;
-  if (!existingCommentId) {
+  const existing = ctx.toolState.progressComment;
+  if (!existing) {
    return false;
  }

  try {
-    await ctx.octokit.rest.issues.deleteComment({
-      owner: ctx.repo.owner,
-      repo: ctx.repo.name,
-      comment_id: existingCommentId,
-    });
+    await deleteProgressCommentApi(
+      { octokit: ctx.octokit, owner: ctx.repo.owner, repo: ctx.repo.name },
+      existing
+    );
  } catch (error) {
    // ignore 404 - comment already deleted
    if (error instanceof Error && error.message.includes("Not Found")) {
@@ -417,7 +413,7 @@ export async function deleteProgressComment(ctx: ToolContext): Promise<boolean>
  }

  // set to null (not undefined) so report_progress skips instead of creating a new comment
-  ctx.toolState.progressCommentId = null;
+  ctx.toolState.progressComment = null;

  return true;
 }
@@ -1,4 +1,5 @@
 import { describe, expect, it } from "vitest";
+import { classifyPushError } from "./git.ts";

 // re-export the normalizeUrl function for testing
 // note: in a real scenario, we'd export this from git.ts or move to a shared utils file
@@ -61,3 +62,123 @@ describe("push URL validation", () => {
    expect(pushUrlNormalized).toBe(actualUrlNormalized);
  });
 });
+
+describe("classifyPushError", () => {
+  describe("concurrent-push", () => {
+    it("matches client-side non-fast-forward (`fetch first`)", () => {
+      const msg =
+        "git push failed (exit 1): To https://github.com/o/r.git\n" +
+        " ! [rejected]        feature -> feature (fetch first)\n" +
+        "error: failed to push some refs to 'https://github.com/o/r.git'\n" +
+        "hint: Updates were rejected because the remote contains work";
+      expect(classifyPushError(msg)).toBe("concurrent-push");
+    });
+
+    it("matches client-side `non-fast-forward` wording", () => {
+      const msg = "! [rejected] main -> main (non-fast-forward)";
+      expect(classifyPushError(msg)).toBe("concurrent-push");
+    });
+
+    it("matches server-side `cannot lock ref` (the case from #571)", () => {
+      const msg =
+        "remote: error: cannot lock ref 'refs/heads/feature': is at " +
+        "abc123 but expected def456\n" +
+        " ! [remote rejected] feature -> feature (cannot lock ref ...)";
+      expect(classifyPushError(msg)).toBe("concurrent-push");
+    });
+  });
+
+  describe("transient", () => {
+    it("matches RPC failed with HTTP 502", () => {
+      expect(
+        classifyPushError(
+          "fatal: unable to access 'https://github.com/o/r.git/': The requested URL returned error: 502"
+        )
+      ).toBe("transient");
+    });
+
+    it("matches early EOF mid-pack", () => {
+      expect(
+        classifyPushError("fatal: the remote end hung up unexpectedly\nfatal: early EOF")
+      ).toBe("transient");
+    });
+
+    it("matches RPC failed", () => {
+      expect(
+        classifyPushError("fatal: RPC failed; curl 56 OpenSSL SSL_read: Connection reset by peer")
+      ).toBe("transient");
+    });
+
+    it("matches HTTP/2 stream not closed cleanly", () => {
+      expect(
+        classifyPushError("fatal: HTTP/2 stream 7 was not closed cleanly: PROTOCOL_ERROR (err 1)")
+      ).toBe("transient");
+    });
+
+    it("matches DNS resolution failure", () => {
+      expect(classifyPushError("fatal: Could not resolve host: github.com")).toBe("transient");
+    });
+
+    it("matches unexpected disconnect during sideband read", () => {
+      expect(classifyPushError("fatal: unexpected disconnect while reading sideband packet")).toBe(
+        "transient"
+      );
+    });
+
+    it("classifies HTTP 429 (rate-limit / abuse detection) as transient", () => {
+      // 429 is the documented exception to the otherwise-permanent 4xx class —
+      // GitHub's abuse detection occasionally surfaces it on git push.
+      expect(
+        classifyPushError(
+          "fatal: unable to access 'https://github.com/o/r.git/': The requested URL returned error: 429"
+        )
+      ).toBe("transient");
+      expect(classifyPushError("remote: HTTP 429: too many requests")).toBe("transient");
+    });
+  });
+
+  describe("unknown", () => {
+    it("does NOT classify auth/403 as transient", () => {
+      // permission denied is permanent within a run — retrying just wastes
+      // time. must NOT match the HTTP-5xx regex.
+      expect(
+        classifyPushError(
+          "remote: Permission to o/r.git denied to bot.\n" +
+            "fatal: unable to access 'https://github.com/o/r.git/': The requested URL returned error: 403"
+        )
+      ).toBe("unknown");
+    });
+
+    it("does NOT classify protected-branch rejection as concurrent-push", () => {
+      expect(
+        classifyPushError(
+          " ! [remote rejected] main -> main (push declined due to repository rule violations)"
+        )
+      ).toBe("unknown");
+    });
+
+    it("does NOT classify 404 as transient", () => {
+      expect(
+        classifyPushError(
+          "fatal: unable to access 'https://github.com/o/r.git/': The requested URL returned error: 404"
+        )
+      ).toBe("unknown");
+    });
+
+    it("returns unknown for an empty message", () => {
+      expect(classifyPushError("")).toBe("unknown");
+    });
+  });
+
+  describe("ordering", () => {
+    it("prefers concurrent-push over transient when both signals appear", () => {
+      // a server-side cannot-lock-ref response that also includes an HTTP
+      // 5xx in the libcurl envelope should still route to the recovery
+      // path, not a blind retry.
+      const msg =
+        "remote: error: cannot lock ref 'refs/heads/feature': is at A but expected B\n" +
+        "fatal: unable to access ...: The requested URL returned error: 500";
+      expect(classifyPushError(msg)).toBe("concurrent-push");
+    });
+  });
+});
@@ -153,6 +153,62 @@ export const PushBranch = type({
  force: type.boolean.describe("Force push (use with caution)").default(false),
 });

+// classify an error from `$git("push", ...)` to decide retry vs. recovery
+// vs. rethrow. exported for tests.
+//
+// - `concurrent-push`: server-side compare-and-swap failed because the ref
+//   advanced between fetch and push. recovery is fetch + integrate + retry.
+//   matches both the client-side detection (`fetch first` /
+//   `non-fast-forward`) and the server-side detection (`cannot lock ref`
+//   with `is at <SHA1> but expected <SHA2>`).
+// - `transient`: network or upstream server hiccup (RPC failed mid-stream,
+//   HTTP 5xx, early EOF, reset, timeout, dns flake). push is idempotent so
+//   verbatim retry with backoff is safe.
+// - `unknown`: anything else (including auth/permission/protected-branch
+//   rejections). retrying these wastes time; surface to the caller.
+//
+// kept conservative: a misclassification of `unknown` -> `transient` would
+// cause two extra round-trips on a permanently-failing push, while the
+// reverse (true transient labeled `unknown`) just falls back to current
+// behavior. so we only mark as transient when the error string is
+// unambiguously a network/server-side fault, not a refusal.
+export type PushErrorKind = "concurrent-push" | "transient" | "unknown";
+
+const CONCURRENT_PUSH_PATTERNS = ["fetch first", "non-fast-forward", "cannot lock ref"] as const;
+
+const TRANSIENT_PATTERNS: RegExp[] = [
+  /RPC failed/i,
+  /early EOF/,
+  /the remote end hung up unexpectedly/,
+  /Connection reset/i,
+  /Could not resolve host/i,
+  /Operation timed out/i,
+  /HTTP\/2 stream \d+ was not closed cleanly/i,
+  /unexpected disconnect while reading sideband packet/i,
+  // libcurl HTTP 5xx surfaced by git over https. matches both the
+  // libcurl-style "The requested URL returned error: 502" and the more
+  // recent "HTTP 502" wording. most 4xx is intentionally excluded —
+  // 401/403/404 indicate auth/permission problems that are not
+  // retry-safe — but 429 (rate-limited / abuse detection) IS retry-safe
+  // and GitHub occasionally surfaces it on git push, so it's included
+  // explicitly below.
+  /HTTP 5\d\d/,
+  /returned error: 5\d\d/i,
+  /HTTP 429/,
+  /returned error: 429/i,
+];
+
+export function classifyPushError(msg: string): PushErrorKind {
+  if (CONCURRENT_PUSH_PATTERNS.some((p) => msg.includes(p))) return "concurrent-push";
+  if (TRANSIENT_PATTERNS.some((p) => p.test(msg))) return "transient";
+  return "unknown";
+}
+
+// backoff delays before retry attempts 2 and 3. attempt 1 is the original
+// push. total worst-case added latency: ~7s. small enough that the agent
+// rarely notices, large enough to ride out most upstream hiccups.
+const TRANSIENT_RETRY_DELAYS_MS = [2000, 5000];
+
 export function PushBranchTool(ctx: ToolContext) {
  const defaultBranch = ctx.repo.data.default_branch || "main";
  const pushPermission = ctx.payload.push;
@@ -234,30 +290,65 @@ export function PushBranchTool(ctx: ToolContext) {
        log.warning(`force pushing - this will overwrite remote history`);
      }

-      try {
-        await $git("push", pushArgs, {
-          token: ctx.gitToken,
-        });
-      } catch (err) {
-        const msg = err instanceof Error ? err.message : String(err);
-        if (msg.includes("fetch first") || msg.includes("non-fast-forward")) {
-          // git rebase is blocked through the MCP tool when shell is disabled
-          // (rebase --exec can execute arbitrary code). merge always works and
-          // integrates remote changes cleanly, so suggest it as the default.
-          const integrateStep =
-            ctx.payload.shell === "disabled"
-              ? `2. use the git tool to merge the remote branch into yours: git({ command: "merge", args: ["origin/${pushDest.remoteBranch}"] })`
-              : `2. use the git tool to rebase or merge your changes on top: git({ command: "merge", args: ["origin/${pushDest.remoteBranch}"] }) (or 'rebase')`;
-          throw new Error(
-            `push rejected: the remote branch '${pushDest.remoteBranch}' has new commits you don't have locally.\n\n` +
-              `to resolve this:\n` +
-              `1. use git_fetch to fetch the remote branch: git_fetch({ ref: "${pushDest.remoteBranch}" })\n` +
-              `${integrateStep}\n` +
-              `3. resolve any merge conflicts if needed\n` +
-              `4. retry push_branch`
-          );
+      // retry transient network/server errors (RPC failed, early EOF, 5xx,
+      // connection reset, etc) with backoff. push is idempotent: if the remote
+      // never received the pack, retry creates the ref; if it did, the retry
+      // is a no-op fast-forward to the same SHA. concurrent-push rejections
+      // and permission errors are NOT retried — they need user intervention.
+      let lastErr: unknown;
+      let pushed = false;
+      for (let attempt = 0; attempt <= TRANSIENT_RETRY_DELAYS_MS.length; attempt++) {
+        try {
+          await $git("push", pushArgs, {
+            token: ctx.gitToken,
+          });
+          if (attempt > 0) {
+            log.info(`push succeeded on attempt ${attempt + 1}`);
+          }
+          pushed = true;
+          break;
+        } catch (err) {
+          lastErr = err;
+          const msg = err instanceof Error ? err.message : String(err);
+          const kind = classifyPushError(msg);
+
+          if (kind === "concurrent-push") {
+            // git rebase is blocked through the MCP tool when shell is disabled
+            // (rebase --exec can execute arbitrary code). merge always works and
+            // integrates remote changes cleanly, so suggest it as the default.
+            const integrateStep =
+              ctx.payload.shell === "disabled"
+                ? `2. use the git tool to merge the remote branch into yours: git({ command: "merge", args: ["origin/${pushDest.remoteBranch}"] })`
+                : `2. use the git tool to rebase or merge your changes on top: git({ command: "merge", args: ["origin/${pushDest.remoteBranch}"] }) (or 'rebase')`;
+            throw new Error(
+              `push rejected: the remote branch '${pushDest.remoteBranch}' has new commits you don't have locally (often a concurrent push to the same branch).\n\n` +
+                `to resolve this:\n` +
+                `1. use git_fetch to fetch the remote branch: git_fetch({ ref: "${pushDest.remoteBranch}" })\n` +
+                `${integrateStep}\n` +
+                `3. resolve any merge conflicts if needed\n` +
+                `4. retry push_branch`
+            );
+          }
+
+          if (kind === "transient" && attempt < TRANSIENT_RETRY_DELAYS_MS.length) {
+            // jitter avoids lockstep retries when several agents are hit by the
+            // same upstream blip simultaneously — without it, all retries land
+            // on the same recovering server at the same instant.
+            const baseDelay = TRANSIENT_RETRY_DELAYS_MS[attempt] ?? 5000;
+            const delay = Math.round(baseDelay * (0.75 + Math.random() * 0.5));
+            log.info(
+              `push attempt ${attempt + 1} failed (transient), retrying in ${delay}ms: ${msg.slice(0, 300)}`
+            );
+            await new Promise((r) => setTimeout(r, delay));
+            continue;
+          }
+
+          throw err;
        }
-        throw err;
+      }
+      if (!pushed) {
+        // safety net — loop should always either break with success or throw.
+        throw lastErr instanceof Error ? lastErr : new Error(String(lastErr));
      }

      return {
@@ -540,7 +540,7 @@ export function CreatePullRequestReviewTool(ctx: ToolContext) {
      // a submitted review obsoletes the progress comment — the review IS the
      // durable artifact. owned here (not in main.ts) so cleanup is atomic with
      // submission and survives any path out of the run (success, timeout,
-      // crash). deleteProgressComment sets progressCommentId = null, so a later
+      // crash). deleteProgressComment sets progressComment = null, so a later
      // report_progress call short-circuits to a no-op.
      // best-effort: a cleanup failure must not turn a successful review into
      // a tool-call failure visible to the agent.
@@ -1,43 +1,40 @@
-import { Octokit } from "@octokit/rest";
+import { readFileSync } from "node:fs";
+import { resolve } from "node:path";
 import { describe, expect, it } from "vitest";
-import { acquireNewToken } from "../utils/github.ts";
-import { getReviewData } from "./reviewComments.ts";
+import { type FormatReviewDataInput, formatReviewData } from "./reviewComments.ts";

-async function getToken(): Promise<string> {
-  if (process.env.GH_TOKEN) return process.env.GH_TOKEN;
-  return await acquireNewToken();
+// fixtures captured by action/scripts/refresh-test-fixtures.ts; re-run
+// (with creds) when GitHub's review/threads/listFiles response shape
+// changes, then review the snapshot diff.
+type ReviewFixture = FormatReviewDataInput & {
+  owner: string;
+  name: string;
+};
+
+function loadFixture(file: string): ReviewFixture {
+  return JSON.parse(
+    readFileSync(resolve(import.meta.dirname, "__fixtures__", file), "utf-8")
+  ) as ReviewFixture;
 }

-describe("getFormattedReviewThreads", () => {
-  it("formats thread blocks with TOC and correct line numbers", { timeout: 30000 }, async () => {
-    const token = await getToken();
-    const octokit = new Octokit({ auth: token });
+describe("formatReviewData", () => {
+  it("formats thread blocks with TOC and correct line numbers", () => {
+    const fx = loadFixture("pullfrog-scratch-pr-49-review-3485940013.json");
+    const result = formatReviewData(fx);
+    expect(result).toBeDefined();
+    if (!result) return;

-    const { formatted } = (await getReviewData({
-      octokit,
-      owner: "pullfrog",
-      name: "scratch",
-      pullNumber: 49,
-      reviewId: 3485940013,
-    }))!;
-
-    expect(formatted.toc).toMatchSnapshot("toc");
-    expect(formatted.content).toMatchSnapshot("content");
+    expect(result.formatted.toc).toMatchSnapshot("toc");
+    expect(result.formatted.content).toMatchSnapshot("content");
  });

-  it("formats body-only review", { timeout: 30000 }, async () => {
-    const token = await getToken();
-    const octokit = new Octokit({ auth: token });
+  it("formats body-only review", () => {
+    const fx = loadFixture("pullfrog-scratch-pr-64-review-3531000326.json");
+    const result = formatReviewData(fx);
+    expect(result).toBeDefined();
+    if (!result) return;

-    const { formatted } = (await getReviewData({
-      octokit,
-      owner: "pullfrog",
-      name: "scratch",
-      pullNumber: 64,
-      reviewId: 3531000326,
-    }))!;
-
-    expect(formatted.toc).toMatchSnapshot("toc");
-    expect(formatted.content).toMatchSnapshot("content");
+    expect(result.formatted.toc).toMatchSnapshot("toc");
+    expect(result.formatted.content).toMatchSnapshot("content");
  });
 });
@@ -497,6 +497,67 @@ interface GetReviewDataInput {
  approvedBy?: string | undefined;
 }

+// pure formatter: takes already-fetched GitHub responses and produces the
+// review data the MCP tool returns. extracted from getReviewData so tests
+// can drive it from checked-in fixtures without live API access.
+//
+// `prFiles` may be empty when `threads` is empty — callers that hit the
+// network should skip the listFiles call in that case as a perf
+// optimization. when both are empty and `review.body` is also empty, the
+// formatter returns undefined just like getReviewData.
+export interface FormatReviewDataInput {
+  review: ReviewResponse;
+  threads: ReviewThread[];
+  prFiles: ReviewPrFile[];
+  pullNumber: number;
+  reviewId: number;
+}
+
+export type ReviewResponse = {
+  body: string | null | undefined;
+  user: { login: string } | null | undefined;
+};
+
+export type ReviewPrFile = {
+  filename: string;
+  patch?: string | undefined;
+};
+
+export function formatReviewData(input: FormatReviewDataInput):
+  | {
+      threadBlocks: Array<{ path: string; lineRange: string; content: string[] }>;
+      reviewer: string;
+      formatted: { toc: string; content: string };
+    }
+  | undefined {
+  const rawReviewBody = input.review.body;
+  const reviewBody = rawReviewBody ? stripExistingFooter(rawReviewBody) : "";
+  const reviewer = input.review.user?.login ?? "unknown";
+
+  if (input.threads.length === 0 && !reviewBody) return undefined;
+
+  let threadBlocks: Array<{ path: string; lineRange: string; content: string[] }> = [];
+
+  if (input.threads.length > 0) {
+    const filePatchMap = new Map<string, ParsedHunk[]>();
+    for (const file of input.prFiles) {
+      if (file.patch) {
+        filePatchMap.set(file.filename, parseFilePatches(file.patch));
+      }
+    }
+    threadBlocks = buildThreadBlocks(input.threads, filePatchMap, input.reviewId);
+  }
+
+  const formatted = formatReviewThreads(threadBlocks, {
+    pullNumber: input.pullNumber,
+    reviewId: input.reviewId,
+    reviewer,
+    reviewBody,
+  });
+
+  return { threadBlocks, reviewer, formatted };
+}
+
 export async function getReviewData(input: GetReviewDataInput): Promise<
  | {
      threadBlocks: Array<{ path: string; lineRange: string; content: string[] }>;
@@ -515,38 +576,25 @@ export async function getReviewData(input: GetReviewDataInput): Promise<
    getReviewThreads(input),
  ]);

-  const rawReviewBody = review.data.body;
-  const reviewBody = rawReviewBody ? stripExistingFooter(rawReviewBody) : "";
-  const reviewer = review.data.user?.login ?? "unknown";
+  // skip listFiles when there are no threads — prFiles is only used for
+  // building thread blocks, and an empty array short-circuits below.
+  const prFiles =
+    threads.length > 0
+      ? await input.octokit.paginate(input.octokit.rest.pulls.listFiles, {
+          owner: input.owner,
+          repo: input.name,
+          pull_number: input.pullNumber,
+          per_page: 100,
+        })
+      : [];

-  if (threads.length === 0 && !reviewBody) return undefined;
-
-  let threadBlocks: Array<{ path: string; lineRange: string; content: string[] }> = [];
-
-  if (threads.length > 0) {
-    const prFiles = await input.octokit.paginate(input.octokit.rest.pulls.listFiles, {
-      owner: input.owner,
-      repo: input.name,
-      pull_number: input.pullNumber,
-      per_page: 100,
-    });
-    const filePatchMap = new Map<string, ParsedHunk[]>();
-    for (const file of prFiles) {
-      if (file.patch) {
-        filePatchMap.set(file.filename, parseFilePatches(file.patch));
-      }
-    }
-    threadBlocks = buildThreadBlocks(threads, filePatchMap, input.reviewId);
-  }
-
-  const formatted = formatReviewThreads(threadBlocks, {
+  return formatReviewData({
+    review: review.data,
+    threads,
+    prFiles,
    pullNumber: input.pullNumber,
    reviewId: input.reviewId,
-    reviewer,
-    reviewBody,
  });
-
-  return { threadBlocks, reviewer, formatted };
 }

 export function GetReviewCommentsTool(ctx: ToolContext) {
@@ -12,6 +12,12 @@ import { log } from "../utils/cli.ts";
 import type { DiffCoverageState } from "../utils/diffCoverage.ts";
 import type { OctokitWithPlugins } from "../utils/github.ts";
 import type { ResolvedPayload } from "../utils/payload.ts";
+import {
+  type ProgressComment,
+  type ProgressCommentType,
+  parseProgressComment,
+} from "../utils/progressComment.ts";
+import type { AccountPlan } from "../utils/runContext.ts";
 import type { RunContextData } from "../utils/runContextData.ts";
 import type { TodoTracker } from "../utils/todoTracking.ts";
 import { CheckoutPrTool } from "./checkout.ts";
@@ -109,8 +115,8 @@ export interface ToolState {
    promise: Promise<PrepResult[]> | undefined;
    results: PrepResult[] | undefined;
  };
-  // undefined = no comment yet, number = active comment, null = deliberately deleted
-  progressCommentId: number | null | undefined;
+  // undefined = no comment yet, object = active comment, null = deliberately deleted
+  progressComment: ProgressComment | null | undefined;
  // immutable snapshot: true if a progress comment was pre-created at init time.
  // survives deleteProgressComment so handleAgentResult can still detect "expected but never reported".
  hadProgressComment: boolean;
@@ -132,20 +138,19 @@ export interface ToolState {
 }

 interface InitToolStateParams {
-  progressCommentId: string | undefined;
+  progressComment: { id: string; type: ProgressCommentType } | undefined;
 }

 export function initToolState(params: InitToolStateParams): ToolState {
-  const parsed = params.progressCommentId ? parseInt(params.progressCommentId, 10) : NaN;
-  const resolvedId = Number.isNaN(parsed) || parsed <= 0 ? undefined : parsed;
+  const resolved = parseProgressComment(params.progressComment);

-  if (resolvedId) {
-    log.info(`» using pre-created progress comment: ${resolvedId}`);
+  if (resolved) {
+    log.info(`» using pre-created progress comment: ${resolved.id} (${resolved.type})`);
  }

  return {
-    progressCommentId: resolvedId,
-    hadProgressComment: !!resolvedId,
+    progressComment: resolved,
+    hadProgressComment: !!resolved,
    backgroundProcesses: new Map(),
    usageEntries: [],
  };
@@ -169,6 +174,13 @@ export interface ToolContext {
  jobId: string | undefined;
  mcpServerUrl: string;
  tmpdir: string;
+  // repo-level OSS flag + account-level billing plan. together they decide
+  // whether pullfrog is paying for marginal infra — see isInfraCovered in
+  // utils/runContext.ts. plan gating for things like update_learnings is
+  // enforced server-side via 402, so we pass plan along mostly for future
+  // use / observability. see wiki/pricing.md.
+  oss: boolean;
+  plan: AccountPlan;
  // resolved upstream model specifier (e.g. "google/gemini-3.1-pro-preview").
  // undefined when payload.proxyModel is set or when the alias is unresolvable.
  // used by the schema sanitizer to detect Gemini-routed traffic.
@@ -140,14 +140,14 @@ export const providers = {
    models: {
      grok: {
        displayName: "Grok",
-        resolve: "xai/grok-4",
-        openRouterResolve: "openrouter/x-ai/grok-4",
+        resolve: "xai/grok-4.3",
+        openRouterResolve: "openrouter/x-ai/grok-4.3",
        preferred: true,
      },
      "grok-fast": {
        displayName: "Grok Fast",
-        resolve: "xai/grok-4-fast",
-        openRouterResolve: "openrouter/x-ai/grok-4-fast",
+        resolve: "xai/grok-4-1-fast",
+        openRouterResolve: "openrouter/x-ai/grok-4.1-fast",
      },
      "grok-code-fast": {
        displayName: "Grok Code Fast",
@@ -354,8 +354,8 @@ export const providers = {
      },
      grok: {
        displayName: "Grok",
-        resolve: "openrouter/x-ai/grok-4",
-        openRouterResolve: "openrouter/x-ai/grok-4",
+        resolve: "openrouter/x-ai/grok-4.3",
+        openRouterResolve: "openrouter/x-ai/grok-4.3",
      },
      "deepseek-pro": {
        displayName: "DeepSeek Pro",
@@ -1,6 +1,6 @@
 {
  "name": "pullfrog",
-  "version": "0.0.203",
+  "version": "0.0.204",
  "type": "module",
  "bin": {
    "pullfrog": "dist/cli.mjs",
@@ -0,0 +1,161 @@
+#!/usr/bin/env node
+/**
+ * refresh checked-in test fixtures for mcp/checkout.test.ts and
+ * mcp/reviewComments.test.ts.
+ *
+ * those tests used to hit live GitHub on every run, which made them
+ * cred-gated (GH_TOKEN or GITHUB_APP_ID + GITHUB_PRIVATE_KEY) and
+ * non-deterministic. they now read from action/mcp/__fixtures__/*.json,
+ * which this script regenerates on demand.
+ *
+ * run with creds set (locally via .env, or in a CI cron with secrets):
+ *
+ *   GH_TOKEN=… node action/scripts/refresh-test-fixtures.ts
+ *   # or
+ *   GITHUB_APP_ID=… GITHUB_PRIVATE_KEY=… node action/scripts/refresh-test-fixtures.ts
+ *
+ * commit the resulting fixture changes; review the diff before merging
+ * (anything unexpected indicates real GitHub API drift).
+ */
+
+import { mkdirSync, writeFileSync } from "node:fs";
+import { dirname, resolve } from "node:path";
+import { fileURLToPath } from "node:url";
+import { Octokit } from "@octokit/rest";
+import { config as loadDotenv } from "dotenv";
+import {
+  REVIEW_THREADS_QUERY,
+  type ReviewThread,
+  type ReviewThreadsQueryResponse,
+} from "../mcp/reviewComments.ts";
+import { acquireNewToken } from "../utils/github.ts";
+
+const scriptDir = dirname(fileURLToPath(import.meta.url));
+const repoRoot = resolve(scriptDir, "../..");
+const fixturesDir = resolve(scriptDir, "../mcp/__fixtures__");
+
+loadDotenv({ path: resolve(repoRoot, ".env") });
+
+type DiffFixture = {
+  owner: string;
+  name: string;
+  pullNumber: number;
+  files: unknown;
+};
+
+type ReviewFixture = {
+  owner: string;
+  name: string;
+  pullNumber: number;
+  reviewId: number;
+  review: { body: string | null | undefined; user: { login: string } | null | undefined };
+  threads: ReviewThread[];
+  prFiles: Array<{ filename: string; patch?: string | undefined }>;
+};
+
+const DIFF_TARGETS: Array<Pick<DiffFixture, "owner" | "name" | "pullNumber">> = [
+  { owner: "pullfrog", name: "test-repo", pullNumber: 1 },
+];
+
+const REVIEW_TARGETS: Array<Pick<ReviewFixture, "owner" | "name" | "pullNumber" | "reviewId">> = [
+  { owner: "pullfrog", name: "scratch", pullNumber: 49, reviewId: 3485940013 },
+  { owner: "pullfrog", name: "scratch", pullNumber: 64, reviewId: 3531000326 },
+];
+
+async function getToken(): Promise<string> {
+  if (process.env.GH_TOKEN) return process.env.GH_TOKEN;
+  return await acquireNewToken();
+}
+
+async function refreshDiffFixture(
+  octokit: Octokit,
+  target: (typeof DIFF_TARGETS)[number]
+): Promise<void> {
+  const files = await octokit.paginate(octokit.rest.pulls.listFiles, {
+    owner: target.owner,
+    repo: target.name,
+    pull_number: target.pullNumber,
+    per_page: 100,
+  });
+  const fixture: DiffFixture = { ...target, files };
+  const path = resolve(
+    fixturesDir,
+    `${target.owner}-${target.name}-pr-${target.pullNumber}.diff.json`
+  );
+  writeFileSync(path, `${JSON.stringify(fixture, null, 2)}\n`);
+  console.log(`wrote ${path}`);
+}
+
+async function refreshReviewFixture(
+  octokit: Octokit,
+  target: (typeof REVIEW_TARGETS)[number]
+): Promise<void> {
+  const [review, threadsResp] = await Promise.all([
+    octokit.rest.pulls.getReview({
+      owner: target.owner,
+      repo: target.name,
+      pull_number: target.pullNumber,
+      review_id: target.reviewId,
+    }),
+    octokit.graphql<ReviewThreadsQueryResponse>(REVIEW_THREADS_QUERY, {
+      owner: target.owner,
+      name: target.name,
+      prNumber: target.pullNumber,
+    }),
+  ]);
+
+  const allThreads = threadsResp.repository?.pullRequest?.reviewThreads?.nodes ?? [];
+  const threads = allThreads.filter((thread): thread is ReviewThread => {
+    if (!thread?.comments?.nodes) return false;
+    return thread.comments.nodes.some((c) => c?.pullRequestReview?.databaseId === target.reviewId);
+  });
+
+  // skip listFiles entirely when there are no threads — prFiles is only
+  // used for thread blocks, so an empty array short-circuits in the
+  // formatter. mirrors getReviewData's runtime perf optimization and
+  // keeps body-only-review fixtures small.
+  const prFiles =
+    threads.length > 0
+      ? await octokit.paginate(octokit.rest.pulls.listFiles, {
+          owner: target.owner,
+          repo: target.name,
+          pull_number: target.pullNumber,
+          per_page: 100,
+        })
+      : [];
+
+  // strip prFiles down to the fields the formatter actually reads. keeps
+  // fixtures small and avoids capturing volatile fields (sha, blob_url,
+  // contents_url, etc.) that would churn unrelated to formatter behavior.
+  const trimmedFiles = prFiles.map((f) => ({
+    filename: f.filename,
+    ...(f.patch ? { patch: f.patch } : {}),
+  }));
+
+  const fixture: ReviewFixture = {
+    ...target,
+    review: {
+      body: review.data.body,
+      user: review.data.user ? { login: review.data.user.login } : null,
+    },
+    threads,
+    prFiles: trimmedFiles,
+  };
+  const path = resolve(
+    fixturesDir,
+    `${target.owner}-${target.name}-pr-${target.pullNumber}-review-${target.reviewId}.json`
+  );
+  writeFileSync(path, `${JSON.stringify(fixture, null, 2)}\n`);
+  console.log(`wrote ${path}`);
+}
+
+async function main(): Promise<void> {
+  const token = await getToken();
+  const octokit = new Octokit({ auth: token });
+  mkdirSync(fixturesDir, { recursive: true });
+
+  for (const t of DIFF_TARGETS) await refreshDiffFixture(octokit, t);
+  for (const t of REVIEW_TARGETS) await refreshReviewFixture(octokit, t);
+}
+
+await main();
@@ -27,8 +27,8 @@ exports[`latest model per provider snapshot > matches snapshot 1`] = `
    "releaseDate": "2026-04-24",
  },
  "openrouter": {
-    "modelId": "poolside/laguna-xs.2:free",
-    "releaseDate": "2026-04-28",
+    "modelId": "x-ai/grok-4.3",
+    "releaseDate": "2026-05-01",
  },
  "xai": {
    "modelId": "grok-4.3",
@@ -2,6 +2,7 @@ import type { ToolState } from "../mcp/server.ts";
 import { getApiUrl } from "./apiUrl.ts";
 import { buildPullfrogFooter } from "./buildPullfrogFooter.ts";
 import { createOctokit, parseRepoContext } from "./github.ts";
+import { updateProgressComment } from "./progressComment.ts";
 import { getGitHubInstallationToken } from "./token.ts";

 interface ReportErrorParams {
@@ -13,8 +14,8 @@ interface ReportErrorParams {
 export async function reportErrorToComment(ctx: ReportErrorParams): Promise<void> {
  const formattedError = ctx.title ? `${ctx.title}\n\n${ctx.error}` : ctx.error;

-  const commentId = ctx.toolState.progressCommentId;
-  if (!commentId) {
+  const comment = ctx.toolState.progressComment;
+  if (!comment) {
    return;
  }

@@ -39,12 +40,11 @@ export async function reportErrorToComment(ctx: ReportErrorParams): Promise<void
    model: ctx.toolState.model,
  });

-  await octokit.rest.issues.updateComment({
-    owner: repoContext.owner,
-    repo: repoContext.name,
-    comment_id: commentId,
-    body: `${formattedError}${footer}`,
-  });
+  await updateProgressComment(
+    { octokit, owner: repoContext.owner, repo: repoContext.name },
+    comment,
+    `${formattedError}${footer}`
+  );

  // mark as updated so exit handler doesn't try to update again
  ctx.toolState.wasUpdated = true;
@@ -154,8 +154,19 @@ export async function $git(

    if (result.exitCode !== 0) {
      const stderr = result.stderr.trim();
-      log.info(`git ${subcommand} failed: ${stderr}`);
-      throw new Error(`git ${subcommand} failed: ${stderr}`);
+      const stdout = result.stdout.trim();
+      // stderr is the primary channel for git diagnostics, but in rare cases
+      // (e.g. some HTTPS smart-protocol failures) the only useful detail is
+      // on stdout — without it the agent / operator sees an empty error.
+      // include exit code so we can distinguish e.g. signal-killed (1 with
+      // empty output) from a genuine git-level rejection.
+      const detail =
+        stderr && stdout
+          ? `${stderr}\n--- stdout ---\n${stdout}`
+          : stderr || stdout || "(no output)";
+      const message = `git ${subcommand} failed (exit ${result.exitCode}): ${detail}`;
+      log.info(message);
+      throw new Error(message);
    }

    return {
@@ -23,7 +23,10 @@ export const JsonPayload = type({
  "eventInstructions?": "string",
  "event?": "object",
  "timeout?": "string | undefined",
-  "progressCommentId?": "string | undefined",
+  "progressComment?": type({
+    id: "string",
+    type: "'issue' | 'review'",
+  }).or("undefined"),
 });

 // permission levels that indicate collaborator status (have push access)
@@ -156,7 +159,7 @@ export function resolvePayload(
    event,
    timeout: inputs.timeout ?? jsonPayload?.timeout,
    cwd: resolveCwd(inputs.cwd),
-    progressCommentId: jsonPayload?.progressCommentId,
+    progressComment: jsonPayload?.progressComment,

    // permissions: inputs > repoSettings > fallbacks
    push: inputs.push ?? repoSettings.push ?? "restricted",
@@ -4,6 +4,12 @@ import { buildPullfrogFooter } from "./buildPullfrogFooter.ts";
 import { log } from "./cli.ts";
 import { createOctokit, parseRepoContext } from "./github.ts";
 import { type ResolvedPromptInput, resolvePromptInput } from "./payload.ts";
+import {
+  getProgressComment,
+  type ProgressComment,
+  parseProgressComment,
+  updateProgressComment,
+} from "./progressComment.ts";
 import { getJobToken } from "./token.ts";

 type JsonPromptInput = Extract<ResolvedPromptInput, object>; // not string
@@ -50,41 +56,47 @@ function buildErrorCommentBody(ctx: PostCleanupContext, isCancellation: boolean)
  return `${errorMessage}${footer}`;
 }

-async function validateStuckProgressComment(ctx: PostCleanupContext): Promise<number | null> {
-  if (!ctx.promptInput?.progressCommentId) {
-    log.info("[post] no progressCommentId in prompt input, skipping cleanup");
+async function validateStuckProgressComment(
+  ctx: PostCleanupContext
+): Promise<ProgressComment | null> {
+  const promptComment = ctx.promptInput?.progressComment;
+  if (!promptComment) {
+    log.info("[post] no progressComment in prompt input, skipping cleanup");
    return null;
  }

-  const commentId = parseInt(ctx.promptInput.progressCommentId, 10);
-  log.info(`[post] validating progressCommentId from prompt input: ${commentId}`);
+  const comment = parseProgressComment(promptComment);
+  if (!comment) {
+    log.info(`[post] progressComment.id is not a positive integer: ${promptComment.id}`);
+    return null;
+  }
+  log.info(`[post] validating progressComment from prompt input: ${comment.id} (${comment.type})`);

  try {
-    const commentResult = await ctx.octokit.rest.issues.getComment({
-      owner: ctx.repoContext.owner,
-      repo: ctx.repoContext.name,
-      comment_id: commentId,
-    });
+    const fetched = await getProgressComment(
+      { octokit: ctx.octokit, owner: ctx.repoContext.owner, repo: ctx.repoContext.name },
+      comment
+    );

-    const body = commentResult.data.body ?? "";
+    const body = fetched.body ?? "";

    if (isLeapingIntoActionCommentBody(body)) {
-      log.info(`[post] comment ${commentId} is stuck on "Leaping into action"`);
-      return commentId;
+      log.info(`[post] comment ${comment.id} is stuck on "Leaping into action"`);
+      return comment;
    }

    // detect stranded todo checklists left by the tracker when the process was killed
    // before the agent could call report_progress with a final summary
    if (/^- \[[ x]\] |^- \*\*→\*\* |^- ~~/.test(body)) {
-      log.info(`[post] comment ${commentId} is stuck on a todo checklist`);
-      return commentId;
+      log.info(`[post] comment ${comment.id} is stuck on a todo checklist`);
+      return comment;
    }

-    log.info(`[post] comment ${commentId} is not stuck (already updated or different content)`);
+    log.info(`[post] comment ${comment.id} is not stuck (already updated or different content)`);
    return null;
  } catch (error) {
    const errorMessage = error instanceof Error ? error.message : String(error);
-    log.info(`[post] failed to get comment ${commentId}: ${errorMessage}`);
+    log.info(`[post] failed to get comment ${comment.id}: ${errorMessage}`);
    return null;
  }
 }
@@ -158,11 +170,13 @@ export async function runPostCleanup(): Promise<void> {

  const ctx: PostCleanupContext = { repoContext, octokit, runId, promptInput };

-  const commentId = await validateStuckProgressComment(ctx);
+  const stuck = await validateStuckProgressComment(ctx);

-  if (!commentId) return log.info("» [post] no stuck progress comment to update, skipping cleanup");
+  if (!stuck) return log.info("» [post] no stuck progress comment to update, skipping cleanup");

-  log.info(`» [post] validated stuck comment: ${commentId}, updating with error message`);
+  log.info(
+    `» [post] validated stuck comment: ${stuck.id} (${stuck.type}), updating with error message`
+  );

  try {
    const body = buildErrorCommentBody(
@@ -170,16 +184,63 @@ export async function runPostCleanup(): Promise<void> {
      SHOULD_CHECK_REASON ? await getIsCancelled(ctx) : false
    );

-    await ctx.octokit.rest.issues.updateComment({
-      owner: ctx.repoContext.owner,
-      repo: ctx.repoContext.name,
-      comment_id: commentId,
-      body,
-    });
-
-    log.info("» [post] successfully updated progress comment");
+    await writeAndVerify(ctx, stuck, body);
  } catch (error) {
    const errorMessage = error instanceof Error ? error.message : String(error);
    log.info(`[post] failed to update comment: ${errorMessage}`);
  }
 }
+
+// post-cleanup runs in a separate process from the cancelled action, so any in-flight
+// HTTP write the action's todoTracker had on the wire when SIGTERM landed can still get
+// processed by GitHub *after* our update — clobbering the cancellation message back to
+// the stale task list. (action-side mitigation: SIGTERM handler cancels the tracker; here
+// we close the remaining race by reading back our write and re-issuing if it lost.)
+const VERIFY_DELAY_MS = 3000;
+const MAX_WRITE_ATTEMPTS = 3;
+
+async function writeAndVerify(
+  ctx: PostCleanupContext,
+  comment: ProgressComment,
+  body: string
+): Promise<void> {
+  const apiCtx = {
+    octokit: ctx.octokit,
+    owner: ctx.repoContext.owner,
+    repo: ctx.repoContext.name,
+  };
+  for (let attempt = 1; attempt <= MAX_WRITE_ATTEMPTS; attempt++) {
+    await updateProgressComment(apiCtx, comment, body);
+    await new Promise((resolve) => setTimeout(resolve, VERIFY_DELAY_MS));
+
+    let fetched: Awaited<ReturnType<typeof getProgressComment>>;
+    try {
+      fetched = await getProgressComment(apiCtx, comment);
+    } catch (error) {
+      // verify GET failed (5xx, secondary rate limit, network blip). the PUT itself
+      // returned 200, so we trust it landed; another write-and-verify pass would just
+      // amplify writes against a flaky GitHub. log and exit — if a stale tracker write
+      // does clobber us, the comment will be wrong but the agent's commit + replies
+      // already conveyed the substance of the run.
+      log.warning(
+        `[post] verify GET failed after attempt ${attempt} — trusting our PUT landed: ${
+          error instanceof Error ? error.message : String(error)
+        }`
+      );
+      return;
+    }
+
+    if (fetched.body === body) {
+      log.info(
+        `» [post] successfully updated progress comment (attempt ${attempt}/${MAX_WRITE_ATTEMPTS})`
+      );
+      return;
+    }
+    log.info(
+      `[post] body was overwritten after our write (attempt ${attempt}/${MAX_WRITE_ATTEMPTS}), retrying`
+    );
+  }
+  log.warning(
+    `[post] gave up after ${MAX_WRITE_ATTEMPTS} attempts — comment may be stale (in-flight writes from the cancelled run kept clobbering us)`
+  );
+}
@@ -0,0 +1,261 @@
+/**
+ * Single source of truth for reading, updating, deleting, and creating "progress comments" —
+ * the GitHub comments Pullfrog uses to surface a run's status.
+ *
+ * A progress comment can be one of two distinct GitHub entities with non-overlapping IDs and
+ * distinct REST endpoints:
+ *   - "issue":  a top-level issue/PR timeline comment (octokit.rest.issues.*Comment)
+ *   - "review": an inline PR review-thread comment   (octokit.rest.pulls.*ReviewComment)
+ *
+ * Callers carry a `ProgressComment` (id + type) value end-to-end so the right endpoint is always
+ * picked. Adding a third comment type later means one new branch in this file, not six.
+ */
+
+export type ProgressCommentType = "issue" | "review";
+
+export type ProgressComment = {
+  id: number;
+  type: ProgressCommentType;
+};
+
+/**
+ * Parse the on-the-wire `{ id: string; type }` shape (the form carried in `JsonPayload`)
+ * into the in-memory `ProgressComment` shape. Returns undefined when the id isn't a
+ * positive integer so callers can short-circuit cleanly. Callers handle logging.
+ */
+export function parseProgressComment(
+  raw: { id: string; type: ProgressCommentType } | null | undefined
+): ProgressComment | undefined {
+  if (!raw?.id) return undefined;
+  const id = parseInt(raw.id, 10);
+  if (Number.isNaN(id) || id <= 0) return undefined;
+  return { id, type: raw.type };
+}
+
+// minimal Octokit shape needed by the progress-comment helpers. structural so the helper
+// can be called from both the action package (@octokit/rest v22) and the root project
+// (@octokit/rest v21) without a nominal type clash. only the methods used here are listed.
+interface CommentResponse {
+  data: { id: number; body?: string | null | undefined; html_url: string; node_id?: string };
+}
+export interface ProgressCommentOctokit {
+  rest: {
+    issues: {
+      createComment: (params: {
+        owner: string;
+        repo: string;
+        issue_number: number;
+        body: string;
+      }) => Promise<CommentResponse>;
+      getComment: (params: {
+        owner: string;
+        repo: string;
+        comment_id: number;
+      }) => Promise<CommentResponse>;
+      updateComment: (params: {
+        owner: string;
+        repo: string;
+        comment_id: number;
+        body: string;
+      }) => Promise<CommentResponse>;
+      deleteComment: (params: {
+        owner: string;
+        repo: string;
+        comment_id: number;
+      }) => Promise<unknown>;
+    };
+    pulls: {
+      createReplyForReviewComment: (params: {
+        owner: string;
+        repo: string;
+        pull_number: number;
+        comment_id: number;
+        body: string;
+      }) => Promise<CommentResponse>;
+      getReviewComment: (params: {
+        owner: string;
+        repo: string;
+        comment_id: number;
+      }) => Promise<CommentResponse>;
+      updateReviewComment: (params: {
+        owner: string;
+        repo: string;
+        comment_id: number;
+        body: string;
+      }) => Promise<CommentResponse>;
+      deleteReviewComment: (params: {
+        owner: string;
+        repo: string;
+        comment_id: number;
+      }) => Promise<unknown>;
+    };
+  };
+}
+
+interface ApiCtx {
+  octokit: ProgressCommentOctokit;
+  owner: string;
+  repo: string;
+}
+
+/**
+ * Fetch a progress comment via the appropriate REST endpoint for its type.
+ * Returns the common subset of fields callers actually use.
+ */
+export async function getProgressComment(
+  ctx: ApiCtx,
+  comment: ProgressComment
+): Promise<{ id: number; body: string | undefined; html_url: string }> {
+  const result = await (comment.type === "review"
+    ? ctx.octokit.rest.pulls.getReviewComment({
+        owner: ctx.owner,
+        repo: ctx.repo,
+        comment_id: comment.id,
+      })
+    : ctx.octokit.rest.issues.getComment({
+        owner: ctx.owner,
+        repo: ctx.repo,
+        comment_id: comment.id,
+      }));
+  return {
+    id: result.data.id,
+    body: result.data.body ?? undefined,
+    html_url: result.data.html_url,
+  };
+}
+
+/**
+ * Update a progress comment in place via the appropriate REST endpoint.
+ * Returns the common subset of fields callers actually use.
+ */
+export async function updateProgressComment(
+  ctx: ApiCtx,
+  comment: ProgressComment,
+  body: string
+): Promise<{
+  id: number;
+  body: string | undefined;
+  html_url: string;
+  node_id: string | undefined;
+}> {
+  const result = await (comment.type === "review"
+    ? ctx.octokit.rest.pulls.updateReviewComment({
+        owner: ctx.owner,
+        repo: ctx.repo,
+        comment_id: comment.id,
+        body,
+      })
+    : ctx.octokit.rest.issues.updateComment({
+        owner: ctx.owner,
+        repo: ctx.repo,
+        comment_id: comment.id,
+        body,
+      }));
+  return {
+    id: result.data.id,
+    body: result.data.body ?? undefined,
+    html_url: result.data.html_url,
+    node_id: result.data.node_id,
+  };
+}
+
+/**
+ * Delete a progress comment via the appropriate REST endpoint.
+ * Lower-level than `deleteProgressComment` in mcp/comment.ts — that one also clears
+ * tool state. Callers that don't have a ToolContext (post cleanup, error handlers)
+ * should use this directly; the higher-level wrapper delegates here.
+ */
+export async function deleteProgressCommentApi(
+  ctx: ApiCtx,
+  comment: ProgressComment
+): Promise<void> {
+  if (comment.type === "review") {
+    await ctx.octokit.rest.pulls.deleteReviewComment({
+      owner: ctx.owner,
+      repo: ctx.repo,
+      comment_id: comment.id,
+    });
+    return;
+  }
+  await ctx.octokit.rest.issues.deleteComment({
+    owner: ctx.owner,
+    repo: ctx.repo,
+    comment_id: comment.id,
+  });
+}
+
+/**
+ * Discriminated target for `createLeapingProgressComment`. The two variants map to the two
+ * distinct GitHub create endpoints; review-reply additionally needs the parent comment ID.
+ */
+export type CreateProgressCommentTarget =
+  | { kind: "issue"; issueNumber: number }
+  | { kind: "reviewReply"; pullNumber: number; replyToCommentId: number };
+
+export interface CreatedProgressComment {
+  comment: ProgressComment;
+  body: string | undefined;
+  html_url: string;
+}
+
+/**
+ * Create the initial "Leaping into action..." progress comment.
+ *
+ * Reliability: when `kind: "reviewReply"` fails (e.g. the parent comment was deleted or the
+ * thread is otherwise unreachable), falls back to a top-level issue comment on the same PR
+ * rather than leaving the run with no progress surface. The fallback is logged.
+ *
+ * (PR # === issue # in GitHub's number space, so `pullNumber` doubles as the fallback target.)
+ */
+export async function createLeapingProgressComment(
+  ctx: ApiCtx,
+  target: CreateProgressCommentTarget,
+  body: string
+): Promise<CreatedProgressComment> {
+  if (target.kind === "reviewReply") {
+    try {
+      const result = await ctx.octokit.rest.pulls.createReplyForReviewComment({
+        owner: ctx.owner,
+        repo: ctx.repo,
+        pull_number: target.pullNumber,
+        comment_id: target.replyToCommentId,
+        body,
+      });
+      return {
+        comment: { id: result.data.id, type: "review" },
+        body: result.data.body ?? undefined,
+        html_url: result.data.html_url,
+      };
+    } catch (error) {
+      // console.warn (not the action-flavored log.warning) because this helper runs in
+      // both the action runtime and the Next.js webhook context, and we don't want a
+      // ::warning:: GitHub Actions annotation leaking into Vercel logs.
+      console.warn(
+        `[progressComment] review reply failed (parent ${target.replyToCommentId} on PR #${target.pullNumber}), falling back to issue comment:`,
+        error
+      );
+      const fallback = await ctx.octokit.rest.issues.createComment({
+        owner: ctx.owner,
+        repo: ctx.repo,
+        issue_number: target.pullNumber,
+        body,
+      });
+      return {
+        comment: { id: fallback.data.id, type: "issue" },
+        body: fallback.data.body ?? undefined,
+        html_url: fallback.data.html_url,
+      };
+    }
+  }
+  const result = await ctx.octokit.rest.issues.createComment({
+    owner: ctx.owner,
+    repo: ctx.repo,
+    issue_number: target.issueNumber,
+    body,
+  });
+  return {
+    comment: { id: result.data.id, type: "issue" },
+    body: result.data.body ?? undefined,
+    html_url: result.data.html_url,
+  };
+}
@@ -24,10 +24,27 @@ export interface RepoSettings {
  envAllowlist: string | null;
 }

+/**
+ * Account-level billing plan. Orthogonal to repo-level OSS status. Mirrors
+ * the server's `AccountPlan` in `utils/billing.ts`. `"none"` = free tier,
+ * `"payg"` = card on file / pay-as-you-go.
+ */
+export type AccountPlan = "none" | "payg";
+
+/**
+ * "Is Pullfrog absorbing marginal infra cost for this repo?" — composite
+ * predicate over the two orthogonal dimensions (repo-level OSS, account-level
+ * plan). Mirrors `isInfraCovered` in the server's `utils/billing.ts`.
+ */
+export function isInfraCovered(params: { isOss: boolean; plan: AccountPlan }): boolean {
+  return params.isOss || params.plan === "payg";
+}
+
 export interface RunContext {
  settings: RepoSettings;
  apiToken: string;
  oss: boolean;
+  plan: AccountPlan;
  proxyModel?: string | undefined;
  dbSecrets?: Record<string, string> | undefined;
 }
@@ -51,6 +68,7 @@ const defaultRunContext: RunContext = {
  settings: defaultSettings,
  apiToken: "",
  oss: false,
+  plan: "none",
 };

 /**
@@ -92,6 +110,7 @@ export async function fetchRunContext(params: {
      settings: RepoSettings | null;
      apiToken: string;
      oss?: boolean;
+      plan?: AccountPlan;
      proxyModel?: string;
      dbSecrets?: Record<string, string>;
    } | null;
@@ -112,6 +131,7 @@ export async function fetchRunContext(params: {
      },
      apiToken: data.apiToken,
      oss: data.oss ?? false,
+      plan: data.plan ?? "none",
      proxyModel: data.proxyModel,
      dbSecrets: data.dbSecrets,
    };
@@ -3,7 +3,7 @@ import type { Octokit } from "@octokit/rest";
 import packageJson from "../package.json" with { type: "json" };
 import { log } from "./cli.ts";
 import { type OctokitWithPlugins, parseRepoContext } from "./github.ts";
-import { fetchRunContext, type RepoSettings } from "./runContext.ts";
+import { type AccountPlan, fetchRunContext, type RepoSettings } from "./runContext.ts";

 export interface RunContextData {
  repo: {
@@ -14,6 +14,7 @@ export interface RunContextData {
  repoSettings: RepoSettings;
  apiToken: string;
  oss: boolean;
+  plan: AccountPlan;
  proxyModel?: string | undefined;
  dbSecrets?: Record<string, string> | undefined;
 }
@@ -54,6 +55,7 @@ export async function resolveRunContextData(
    repoSettings: runContext.settings,
    apiToken: runContext.apiToken,
    oss: runContext.oss,
+    plan: runContext.plan,
    proxyModel: runContext.proxyModel,
    dbSecrets: runContext.dbSecrets,
  };
Author	SHA1	Message	Date
Colin McDonnell	560e27bda5	refactor progress comments into a bundled type + helper module (#567 ) * refactor progress comments into a single bundled type + helper module introduce ProgressComment ({ id, type: "issue" \| "review" }) as the canonical handle for the GitHub comment a run uses to report progress, and route every read/update/delete/create through a single helper module (action/utils/progressComment.ts). previously every site that touched the progress comment hardcoded octokit.rest.issues.Comment, which made adding a second comment type (review-thread replies) require duplicating the same branch in 6+ places — the same shape that bit pullfrog/app#445. new capability: when the address-reviews trigger fires for a one-off review comment, the "Leaping into action" comment is now posted as a reply in that review thread instead of as a top-level PR timeline comment. the helper handles failure (e.g. parent comment deleted) by silently falling back to a top-level issue comment, so the run never loses its progress surface. changes: - action/utils/progressComment.ts (new) — ProgressComment type + getProgressComment, updateProgressComment, deleteProgressCommentApi, createLeapingProgressComment. uses a structural Octokit interface to bridge the @octokit/rest version mismatch between the action package (v22) and the root project (v21). - action/internal/index.ts — re-export the new types and helpers for cross-boundary use. - action/external.ts, action/utils/payload.ts — replace progressCommentId: string with progressComment: { id: string, type: "issue" \| "review" } in WriteablePayload + JsonPayload. wire-format breaking, no legacy fallback (in-flight runs across the deploy lose their progress comment, fine). - action/mcp/server.ts — ToolState.progressCommentId becomes progressComment: ProgressComment \| null \| undefined (same tristate semantics). - action/main.ts, action/mcp/comment.ts, action/utils/errorReport.ts, action/utils/postCleanup.ts — every issues.Comment call against the progress comment routes through the helper module. zero hardcoded API branching outside the helper. - utils/github/triggerWorkflow.ts — drop createLeapingComment + updateCommentToLeaping; dispatchAndTrackWorkflow gains a resolution chain (existingComment → replyToReviewComment → triggeringIssue → none) and an existingComment: ProgressComment param plus replyToReviewComment: { pullNumber, commentId }. - utils/webhooks/handleWebhook.ts — dispatch closure threads replyToReviewComment through; the one-off review comment branch passes it and skips the now-redundant eyes reaction on the comment we're about to reply to. - app/trigger/[owner]/[repo]/[number]/page.tsx, utils/github/runActionLocal.ts, app/api/cli/dispatch/route.ts, app/api/dispatch-workflow/route.ts — call sites updated to new shape. no schema or DB column changes. the existing WorkflowRun.progressCommentId column is still written by id only; type lives only on the in-flight payload, which is sufficient for runtime since it's the only thing that needs to know which API to call. * anneal pass 1: fallback visibility + stale doc/comment updates - progressComment.ts: when reviewReply→issue fallback fires, prepend a [!NOTE] callout with a permalink back to the original review comment. without this, the parent comment showed no eyes reaction (deliberately skipped) and no reply, leaving the user with no signal that anything happened. - wiki/post-cleanup.md: update progressCommentId references to progressComment, document the new helper-based dispatch by type. - wiki/main.md: update initToolState({ progressCommentId }) → ({ progressComment }) in the resolver-chain diagram. - action/main.ts, action/mcp/review.ts: update two stale comments that referenced the old field name. * anneal pass 2: post-cleanup detection through fallback notice + log cleanup - isLeapingIntoActionCommentBody: strip a leading GFM blockquote/alert before testing the leaping prefix. without this, the [!NOTE] callout that the reviewReply→issue fallback prepends would prevent post-cleanup from recognizing the stuck "Leaping into action..." comment, leaving it permanently on the PR timeline if the workflow died before any progress update. - progressComment helper: switch from log.warning (action-flavored, emits a ::warning:: GitHub Actions annotation) to console.warn so the helper doesn't pollute Vercel logs when invoked from the webhook context. - triggerWorkflow.ts: drop the duplicate caller-side log on review-reply failure — the helper already speaks loudly. Reword the catch-branch log to reflect that it now only fires when both the reply AND the helper's internal fallback failed. - progressComment.ts: document that the [!NOTE] fallback notice is overwritten on the first report_progress call, and explain the trade-off vs persisting it through the action payload + ToolState. * debloat: drop the [!NOTE] fallback callout Reverting two pieces from the prior anneal pass: - progressComment.ts: drop the [!NOTE] callout that the reviewReply→issue fallback prepended to the leaping body. It disappeared on the agent's first report_progress call, which made it half-committed to visibility — worse than either properly persisting it (real engineering) or leaving the fallback silent (current choice). The console.warn diagnostic and the workflow-run footer link in the leaping comment itself give us enough signal for the rare case where both API endpoints fail at once. - isLeapingIntoActionCommentBody: revert the leading-blockquote stripping; only needed to compensate for the [!NOTE] callout. Keeping: the console.warn-vs-log.warning fix (real cross-runtime concern), the duplicate-log drop in triggerWorkflow.ts, the wiki updates, and the two stale source-comment fixes. * fix: prevent stranded task list overwriting post-cleanup message When a run is cancelled, the action's todoTracker may have an HTTP write in flight to GitHub when SIGTERM lands. The action process dies, but the request data has already left the socket — GitHub processes it and updates the comment body to the (stale) task list. Meanwhile post-cleanup, running in a separate process, writes the "This run was cancelled 🛑" message. If the tracker's in-flight write happens to land after post-cleanup's write, the user never sees the cancellation message. Two-layer fix: - Action side: cancel the tracker in the SIGTERM signal handler so no new debounced writes get scheduled. This shrinks the race window but can't un-send a request already on the wire. - Post-cleanup side: after writing, verify the body landed and re-issue if another write clobbered ours. Loops up to 3× with a 3s settle delay so delayed in-flight writes from the dying action have time to arrive before our read-back check decides whether to retry. * lint: import createLeapingProgressComment from pullfrog/internal in test script * address bot review findings: reply-target root, version bump, GET error handling Three real findings from the bot reviews on #567 plus a small DRY pass: 1. handleWebhook reply-target: `newComments[0]` may be a reply, not a top-level review comment. `getReviewCommentsWithReplies` returns root + replies for any thread the review touched, and `pull_request_review_id` filtering only narrows by which review submitted, not root vs reply. When a user submits a single reply as their entire review (e.g. replying to someone else's comment to ping @pullfrog), the reply ID flowed through to `createReplyForReviewComment`, which 422s on replies-to-replies and degraded to a top-level issue comment — exactly the polluted-PR-timeline behavior this PR was built to remove. Walk up `in_reply_to` from the already-fetched thread data to find the root and reply there instead. 2. action/package.json: bumped 0.0.202 → 0.0.204. main is at 0.0.203 and our wire format changed; without a bump validateCompatibility can't surface the mismatch on the deploy boundary, and the merge would have gone backwards. 3. postCleanup writeAndVerify: distinguish a thrown verify-GET from a "body got overwritten" mismatch. Treating a transient 5xx/rate-limit GET the same as a clobber wasted PUT attempts and printed a misleading "in-flight writes kept clobbering us" warning. We trust our PUT (which returned 200) and exit instead of amplifying writes against a flaky API. 4. Small DRY: extracted parseProgressComment for the `{ id: string; type } -> ProgressComment` parse that had drifted across server.ts and postCleanup.ts.	2026-05-06 01:50:58 +00:00
Colin McDonnell	1e17a76863	bump xai/grok to 4.3 and grok-fast to 4-1-fast #1 generational bump on both. xAI shipped grok-4.3 on 2026-05-01 and grok-4-1-fast on 2025-11-19; both are same brand tier as the existing slugs (`grok` and `grok-fast`), so resolve + openRouterResolve update in place with no DB migration needed. Mirrored on the openrouter provider side (openrouter/grok now also points at x-ai/grok-4.3). OpenRouter spells the fast variant `x-ai/grok-4.1-fast` (dot) where models.dev uses `grok-4-1-fast` (dash) — verified both forms against their respective live APIs before committing. See the "naming traps" section in wiki/models-catalog.md. Snapshot regenerated: openrouter latest-GA shifted from poolside/laguna-xs.2:free (2026-04-28) to x-ai/grok-4.3 (2026-05-01) as a mechanical consequence of the bump. Verified via `pnpm -C action test:catalog` (139/139 pass against live models.dev + OpenRouter API) and `pnpm -C action test` (458/458). Considered and explicitly rejected during this audit (recording for future archaeology): - Re-adding opencode/nemotron-3-super-free: removed twice in 71dff24c and 0f8117af with no commit-message rationale, but the removals are intentional per maintainer. - Adding gpt-nano (openai + opencode + openrouter) at gpt-5.4-nano: the snapshot has been silently tracking opencode/gpt-5.4-nano since 7dd80143 (2026-03-18) without a corresponding catalog addition — a deliberate non-add. Also would have collided with the existing opencode/gpt-5-nano displayName "GPT Nano". - Adding opencode/hy3-preview-free: never been in the catalog on main and no positive signal beyond models.dev availability. - Bumping opencode/gpt-5-nano (free) to opencode/gpt-5.4-nano: would silently turn a free alias paid ($0.20/$1.25 per M tokens) — not a generational bump, would require retire-and-replace if pursued.	2026-05-05 23:40:00 +00:00
David Blass	ada5584737	test(mcp): make checkout/reviewComments tests offline (fixture-driven) (#575 ) `mcp/checkout.test.ts` and `mcp/reviewComments.test.ts` previously hit live GitHub on every run via `acquireNewToken()`, requiring `GH_TOKEN` or `GITHUB_APP_ID` + `GITHUB_PRIVATE_KEY` in the env. that made them: - cred-gated — the action runtime filters `_KEY$` / `_TOKEN$` from subprocess env, so the husky pre-push hook (which runs `pnpm -r test`) blocked Pullfrog agents from pushing branches. issues #562, #563, #564, #566 all hit this exact blocker and never got their fixes pushed. - non-deterministic and slow (network round-trips for a snapshot test). both tests are really snapshot tests of pure formatters (`formatFilesWithLineNumbers`, plus `parseFilePatches` / `buildThreadBlocks` / `formatReviewThreads` for review data). the live fetches were just an inefficient way to obtain fixtures. changes: 1. extract a pure `formatReviewData({ review, threads, prFiles, ... })` from `getReviewData` in `mcp/reviewComments.ts`. `getReviewData` becomes thin orchestration: fetch + call formatter. preserves the "skip listFiles when no threads" perf optimization. 2. add `action/mcp/__fixtures__/` with checked-in JSON captures for the three fixture test cases (pullfrog/test-repo#1 listFiles, pullfrog/scratch#49 review 3485940013, pullfrog/scratch#64 review 3531000326). ~14KB total. fixtures store only the fields the formatter reads — volatile fields (sha, blob_url, etc.) are dropped. 3. rewrite both test files to load the fixtures and call the pure formatters directly. snapshot keys updated; snapshot content unchanged (verified by running existing snapshots against the refactored tests). 4. add `action/scripts/refresh-test-fixtures.ts` to re-fetch the fixtures from live GitHub on demand: `node action/scripts/refresh-test-fixtures.ts` (with creds in `.env` or env). re-run when the GitHub API response shape changes and review the snapshot diff. trade-off: a silent change to GitHub's `pulls.listFiles` / `pulls.getReview` / GraphQL `reviewThreads` response shape would no longer break this test on every push. that tradeoff is worth it: shape drift on those endpoints is rare (years between changes), and a dedicated cron that runs the refresh script and opens a PR on diff is a far better signal than a flaky cred-gated pre-push hook. Co-authored-by: Cursor <cursoragent@cursor.com>	2026-05-05 23:25:46 +00:00
David Blass	b6e2c61d30	fix(push_branch): retry transient push errors and surface full stderr/stdout (#573 ) * fix(push_branch): retry transient push errors and surface full stderr/stdout issue #571 motivated three small improvements to `mcp__pullfrog__push_branch`: 1. classify push errors into `concurrent-push` / `transient` / `unknown`. - `concurrent-push` extends the existing `fetch first` / `non-fast-forward` matcher to also catch the server-side `cannot lock ref` form (the case #571 reports). all three route to the same fetch + integrate + retry recovery message; copy now mentions concurrent push as a likely cause. - `transient` covers RPC failed, early EOF, connection reset, dns flake, HTTP 5xx, HTTP/2 stream not closed, and unexpected sideband disconnect. these are retried in-tool with 2s + 5s backoff before surfacing the error. push is idempotent so verbatim retry is safe. - `unknown` (auth/permission/protected-branch/4xx) is rethrown unchanged — retrying these wastes time and noise. 2. surface stdout alongside stderr in `$git` failure messages and include the exit code. previously only `stderr.trim()` was forwarded, which could be empty in rare HTTPS failure modes (the agent on issue #571's run saw a one-line `failed to push some refs` and had nothing to diagnose with). 3. unit tests for the classifier covering all three branches plus the concurrent-push-wins-over-transient ordering. does not introduce auto fetch+rebase+retry inside the tool — that path is blocked under shell=disabled, can leave the working tree mid-conflict, and would create unwanted merge commits. the recovery message keeps the agent in the loop. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(push_branch): retry 429, jitter backoff, downgrade retry log to info - treat HTTP 429 (rate-limit / abuse detection) as transient — GitHub occasionally surfaces it on git push, where it is retry-safe unlike 401/403/404 - add ±25% jitter to backoff so concurrent agents hit by the same upstream blip don't retry in lockstep - log retries with log.info instead of log.warning to match retry.ts convention; a successful retry shouldn't leave a yellow GHA annotation behind in the job summary --------- Co-authored-by: Cursor <cursoragent@cursor.com> Co-authored-by: pullfrog[bot] <226033991+pullfrog[bot]@users.noreply.github.com>	2026-05-05 21:59:40 +00:00
Colin McDonnell	e58299740d	Merge pull request #545 from pullfrog/billing managed billing + stripe v1	2026-05-05 19:33:46 +00:00