Retries on oidc. 152

mcp/checkout.ts

@@ -1,3 +1,5 @@
+import { writeFileSync } from "node:fs";
+import { join } from "node:path";
 import type { Octokit } from "@octokit/rest";
 import { type } from "arktype";
 import type { ToolContext } from "../main.ts";
@@ -20,6 +22,7 @@ export type CheckoutPrResult = {
   url: string;
   headRepo: string;
   diff: string;
+  diffPath: string;
 };
 
 interface CheckoutPrBranchParams {
@@ -104,13 +107,13 @@ export async function checkoutPrBranch(
     const remoteName = `pr-${pullNumber}`;
     const forkUrl = `https://x-access-token:${token}@github.com/${headRepo.full_name}.git`;
 
-    // add fork as a named remote (ignore error if already exists)
+    // add fork as a named remote (suppress logging to avoid "error: remote already exists" spam)
     try {
-      $("git", ["remote", "add", remoteName, forkUrl]);
+      $("git", ["remote", "add", remoteName, forkUrl], { log: false });
       log.debug(`📌 added remote '${remoteName}' for fork ${headRepo.full_name}`);
     } catch {
       // remote already exists, update its URL
-      $("git", ["remote", "set-url", remoteName, forkUrl]);
+      $("git", ["remote", "set-url", remoteName, forkUrl], { log: false });
       log.debug(`📌 updated remote '${remoteName}' for fork ${headRepo.full_name}`);
     }
 
@@ -140,7 +143,8 @@ export function CheckoutPrTool(ctx: ToolContext) {
   return tool({
     name: "checkout_pr",
     description:
-      "Checkout a pull request branch locally. This fetches the PR branch and sets up push configuration for fork PRs. Use this when you need to work on an existing PR.",
+      "Checkout a pull request branch locally. This fetches the PR branch and sets up push configuration for fork PRs. " +
+      "The PR diff is written to a file (diffPath) for grep access. For small diffs, it's also returned inline.",
     parameters: CheckoutPr,
     execute: execute(async ({ pull_number }) => {
       const result = await checkoutPrBranch({
@@ -174,6 +178,19 @@ export function CheckoutPrTool(ctx: ToolContext) {
         mediaType: { format: "diff" },
       });
 
+      // write diff to file for grep access
+      const diffContent = diffResponse.data as unknown as string;
+      const diffPath = join(process.env.PULLFROG_TEMP_DIR!, `pr-${pull_number}.diff`);
+      writeFileSync(diffPath, diffContent);
+      log.debug(`wrote diff to ${diffPath} (${diffContent.length} bytes)`);
+
+      // return diff inline only if small enough
+      const MAX_INLINE_DIFF_SIZE = 50 * 1024; // 50KB
+      const diff =
+        diffContent.length <= MAX_INLINE_DIFF_SIZE
+          ? diffContent
+          : `Diff is ${(diffContent.length / 1024).toFixed(0)}KB - use grep or read from ${diffPath}`;
+
       return {
         success: true,
         number: pr.data.number,
@@ -184,7 +201,8 @@ export function CheckoutPrTool(ctx: ToolContext) {
         maintainerCanModify: pr.data.maintainer_can_modify,
         url: pr.data.html_url,
         headRepo: headRepo.full_name,
-        diff: diffResponse.data as unknown as string,
+        diff,
+        diffPath,
       } satisfies CheckoutPrResult;
     }),
   });

mcp/comment.ts

@@ -275,11 +275,20 @@ export async function deleteProgressComment(ctx: ToolContext): Promise<boolean>
     return false;
   }
 
+  try {
     await ctx.octokit.rest.issues.deleteComment({
       owner: ctx.owner,
       repo: ctx.name,
       comment_id: existingCommentId,
     });
+  } catch (error) {
+    // ignore 404 - comment already deleted
+    if (error instanceof Error && error.message.includes("Not Found")) {
+      // comment already deleted, continue
+    } else {
+      throw error;
+    }
+  }
 
   // reset state but mark as "updated" so ensureProgressCommentUpdated doesn't try to handle it
   progressCommentId = null;

mcp/review.ts

@@ -92,6 +92,13 @@ export function StartReviewTool(ctx: ToolContext) {
           commit_id: pr.data.head.sha,
           // no 'event' = PENDING review
         });
+        log.debug(`createReview response: ${JSON.stringify(result.data)}`);
+        if (!result.data.id || !result.data.node_id) {
+          log.debug(result);
+          throw new Error(
+            `createReview returned invalid data: id=${result.data.id}, node_id=${result.data.node_id}`
+          );
+        }
         reviewId = result.data.id;
         reviewNodeId = result.data.node_id;
         log.debug(`created new pending review: id=${reviewId}`);
@@ -123,19 +130,10 @@ export function StartReviewTool(ctx: ToolContext) {
         id: reviewId,
       };
 
+      log.debug(`review session started: id=${reviewId}, nodeId=${reviewNodeId}`);
+
       return {
-        message: `Review session started for PR #${pull_number}.`,
+        message: `Review session started for PR #${pull_number}. Add comments with add_review_comment, then submit with submit_review.`,
-        guidance: {
-          analyze: [
-            "What does this PR change? Summarize in 1-2 sentences.",
-            "Is the approach sound? If not, stop and comment on approach first.",
-            "What bugs, edge cases, or security issues exist?",
-          ],
-          beforeCommenting: [
-            "Skip nitpicks unless explicitly requested.",
-            "Would the codebase maintainer care about this feedback, based on what you can infer about the code quality standards in this repo?",
-          ],
-        },
       };
     }),
   });
@@ -166,21 +164,58 @@ export function AddReviewCommentTool(ctx: ToolContext) {
         throw new Error("No review session started. Call start_review first.");
       }
 
+      const reviewNodeId = ctx.toolState.review.nodeId;
+      log.debug(
+        `adding review comment: reviewNodeId=${reviewNodeId}, path=${path}, line=${line}, side=${side || "RIGHT"}`
+      );
+
       // add comment thread via GraphQL (REST doesn't support adding to existing pending review)
-      await ctx.octokit.graphql<AddPullRequestReviewThreadResponse>(
+      let result: AddPullRequestReviewThreadResponse;
+      try {
+        result = await ctx.octokit.graphql<AddPullRequestReviewThreadResponse>(
           ADD_PULL_REQUEST_REVIEW_THREAD,
           {
-          pullRequestReviewId: ctx.toolState.review.nodeId,
+            pullRequestReviewId: reviewNodeId,
             path,
             line,
             body,
             side: side || "RIGHT",
           }
         );
+        log.debug(`addPullRequestReviewThread response: ${JSON.stringify(result)}`);
+      } catch (error) {
+        const errorMsg = error instanceof Error ? error.message : String(error);
+        log.debug(`addPullRequestReviewThread error: ${errorMsg}`);
+        throw new Error(
+          `Failed to add comment to ${path}:${line}. GraphQL error: ${errorMsg}. ` +
+            `Ensure the line is part of the diff and the path is correct.`
+        );
+      }
+
+      // check if the mutation succeeded - null means the line is not in the diff
+      if (!result) {
+        throw new Error(
+          `Failed to add comment to ${path}:${line}. GraphQL returned null response.`
+        );
+      }
+      if (!result.addPullRequestReviewThread) {
+        throw new Error(
+          `Failed to add comment to ${path}:${line}. addPullRequestReviewThread is null. Response: ${JSON.stringify(result)}`
+        );
+      }
+      if (!result.addPullRequestReviewThread.thread) {
+        throw new Error(
+          `Failed to add comment to ${path}:${line}. thread is null. The line must be part of the diff. Response: ${JSON.stringify(result)}`
+        );
+      }
+
+      const threadId = result.addPullRequestReviewThread.thread.id;
+      log.debug(`review comment added: threadId=${threadId}`);
 
       return {
         success: true,
         message: `Comment added to ${path}:${line}`,
+        threadId,
       };
     }),
   });
@@ -211,6 +246,9 @@ export function SubmitReviewTool(ctx: ToolContext) {
       }
 
       const reviewId = ctx.toolState.review.id;
+      log.debug(
+        `submitting review: id=${reviewId}, nodeId=${ctx.toolState.review.nodeId}, prNumber=${ctx.toolState.prNumber}`
+      );
 
       // build quick links footer
       const apiUrl = process.env.API_URL || "https://pullfrog.com";
@@ -234,6 +272,12 @@ export function SubmitReviewTool(ctx: ToolContext) {
         body: bodyWithFooter,
       });
 
+      log.debug(`submitReview response: ${JSON.stringify(result.data)}`);
+      if (!result.data.id) {
+        throw new Error(`submitReview returned invalid data: ${JSON.stringify(result.data)}`);
+      }
+      log.debug(`review submitted: reviewId=${result.data.id}, state=${result.data.state}`);
+
       // clear review state
       delete ctx.toolState.review;
 
@@ -336,6 +380,10 @@ export function ReviewTool(ctx: ToolContext) {
         });
       }
       const result = await ctx.octokit.rest.pulls.createReview(params);
+      log.debug(`createReview (legacy) response: ${JSON.stringify(result.data)}`);
+      if (!result.data.id) {
+        throw new Error(`createReview returned invalid data: ${JSON.stringify(result.data)}`);
+      }
       const reviewId = result.data.id;
 
       // build quick links footer and update the review body

package.json

@@ -1,6 +1,6 @@
 {
   "name": "@pullfrog/action",
-  "version": "0.0.149",
+  "version": "0.0.152",
   "type": "module",
   "files": [
     "index.js",

utils/api.ts

@@ -36,8 +36,8 @@ export interface WorkflowRunInfo {
 export async function fetchWorkflowRunInfo(runId: string): Promise<WorkflowRunInfo> {
   const apiUrl = process.env.API_URL || "https://pullfrog.com";
 
-  // add timeout to prevent hanging (5 seconds)
+  // add timeout to prevent hanging (30 seconds)
-  const timeoutMs = 5000;
+  const timeoutMs = 30000;
   const controller = new AbortController();
   const timeoutId = setTimeout(() => controller.abort(), timeoutMs);
 
@@ -90,8 +90,8 @@ export async function getRepoSettings(
 ): Promise<RepoSettings> {
   const apiUrl = process.env.API_URL || "https://pullfrog.com";
 
-  // Add timeout to prevent hanging (5 seconds)
+  // Add timeout to prevent hanging (30 seconds)
-  const timeoutMs = 5000;
+  const timeoutMs = 30000;
   const controller = new AbortController();
   const timeoutId = setTimeout(() => controller.abort(), timeoutMs);
 

utils/cli.ts

@@ -278,7 +278,7 @@ export const log = {
   /**
    * Print debug message (only if LOG_LEVEL=debug)
    */
-  debug: (message: string): void => {
+  debug: (message: string | unknown): void => {
     if (isDebugEnabled()) {
       if (isGitHubActions) {
         // using this instead of core.debug
@@ -342,12 +342,11 @@ export const log = {
    */
   toolCall: ({ toolName, input }: { toolName: string; input: unknown }): void => {
     const inputFormatted = formatJsonValue(input);
-    // if (inputFormatted !== "{}")
+    const timestamp = isDebugEnabled() ? ` [${new Date().toISOString()}]` : "";
-    const output = inputFormatted !== "{}" ? `${toolName}(${inputFormatted})` : `${toolName}()`;
+    const output =
-
+      inputFormatted !== "{}"
-    // if (inputFormatted !== "{}") {
+        ? `→ ${toolName}(${inputFormatted})${timestamp}`
-    // output += formatIndentedField("input", inputFormatted);
+        : `→ ${toolName}()${timestamp}`;
-    // }
 
     log.info(output.trimEnd());
   },

utils/github.ts

@@ -1,6 +1,7 @@
 import { createSign } from "node:crypto";
 import * as core from "@actions/core";
 import { log } from "./cli.ts";
+import { retry } from "./retry.ts";
 
 export interface InstallationToken {
   token: string;
@@ -48,17 +49,15 @@ function isGitHubActionsEnvironment(): boolean {
 }
 
 async function acquireTokenViaOIDC(): Promise<string> {
-  log.debug("» generating OIDC token...");
+  log.info("» generating OIDC token...");
 
   const oidcToken = await core.getIDToken("pullfrog-api");
-  log.debug("» OIDC token generated successfully");
 
   const apiUrl = process.env.API_URL || "https://pullfrog.com";
 
-  log.debug("» exchanging OIDC token for installation token...");
+  log.info("» exchanging OIDC token for installation token...");
 
-  // Add timeout to prevent long waits (5 seconds)
+  const timeoutMs = 30000;
-  const timeoutMs = 5000;
   const controller = new AbortController();
   const timeoutId = setTimeout(() => controller.abort(), timeoutMs);
 
@@ -79,7 +78,7 @@ async function acquireTokenViaOIDC(): Promise<string> {
     }
 
     const tokenData = (await tokenResponse.json()) as InstallationToken;
-    log.debug(`» installation token obtained for ${tokenData.repository || "all repositories"}`);
+    log.info(`» installation token obtained for ${tokenData.repository || "all repositories"}`);
 
     return tokenData.token;
   } catch (error) {
@@ -235,7 +234,7 @@ async function acquireTokenViaGitHubApp(): Promise<string> {
 
 async function acquireNewToken(): Promise<string> {
   if (isGitHubActionsEnvironment()) {
-    return await acquireTokenViaOIDC();
+    return await retry(() => acquireTokenViaOIDC(), { label: "token exchange" });
   } else {
     return await acquireTokenViaGitHubApp();
   }