fix workflow detection, duplicate summaries, review resilience (#482)
* fix workflow detection when repos have many workflows Switch workflow lookup to GitHub's direct workflow-by-filename API so pullfrog.yml is found even when list endpoints paginate, and paginate installation scans in maintenance scripts to avoid partial coverage. Made-with: Cursor * fix review comment line resolution: pre-validate against diff hunks + auto-bisect fallback when submitting a review with inline comments, the tool now: 1. fetches the PR diff and validates each comment's line range against the actual hunk boundaries 2. moves invalid comments to the review body with a clear explanation 3. on 422 from GitHub (rare API quirks where valid-looking lines are rejected), bisects comments using disposable pending reviews to isolate failures 4. retries with only the comments GitHub accepts also fixes getHttpStatus (previously isStatusError) which wasn't recognizing Octokit errors, and warns in the tool description that each call creates a permanent visible review. Made-with: Cursor * remove bisect fallback, anchor review to checkout sha, make start_line optional - drop the auto-bisect-on-422 logic entirely; pre-validation catches the real issues and the 422 catch now just throws a clear actionable error - anchor review submission to checkoutSha so line numbers match the diff the agent actually analyzed (avoids stale-line 422s from new pushes) - make start_line optional and only set start_line/start_side when it differs from line (single-line comments don't need the range fields) - improve headMovedDuringReview detection to use latestHeadSha directly Made-with: Cursor * drop review comment pre-validation in favor of pinned commit_id The pre-validation (listFiles + hunk parsing) was checking comments against the current PR diff, but the review is now anchored to checkoutSha. When HEAD moves, pre-validation checks the wrong diff and can false-reject valid comments. GitHub's own commit_id-anchored validation is the correct source of truth. Made-with: Cursor * add logging to fetchExistingSummaryComment for duplicate summary debug Made-with: Cursor * fix duplicate summary comments: guard create_issue_comment for existing summaries When select_mode finds an existing summary comment (existingSummaryCommentId), create_issue_comment with type: "Summary" now auto-redirects to update instead of creating a new comment. Belt-and-suspenders for the token fix in selectMode.ts. Made-with: Cursor * document api auth patterns to prevent token misuse add wiki/api-auth.md explaining the two auth patterns (GitHub token vs Pullfrog JWT) and when to use each. add auth comments to all action-facing routes and their callers so the correct token is obvious. Made-with: Cursor * fix models.dev snapshot: filter beta models, add tiebreaker Skip models with any status (beta, deprecated) so nightly/experimental releases don't cause snapshot churn. Add lexicographic tiebreaker for stable ordering when release dates match. Made-with: Cursor * add tests to pre-push hook Made-with: Cursor * fix: decouple summary dispatch from re-review gate on pull_request_synchronize The summary workflow was never dispatched on new commits because the pull_request_synchronize handler broke early when prReReview was disabled, before reaching the prSummaryComment check. Now re-review and summary are dispatched independently. Made-with: Cursor * resolve merge conflicts in rebase.md and checkout.ts Made-with: Cursor * fix: restore checkout.ts and rebase.md from remote Made-with: Cursor
This commit is contained in:
committed by
pullfrog[bot]
parent
2e37fb3dfa
commit
8a734c32f4
@@ -145111,6 +145111,26 @@ function CreateCommentTool(ctx) {
|
||||
parameters: Comment,
|
||||
execute: execute(async ({ issueNumber, body, type: commentType }) => {
|
||||
const bodyWithFooter = await addFooter(ctx, body);
|
||||
if (commentType === "Summary" && ctx.toolState.existingSummaryCommentId) {
|
||||
log.info(
|
||||
`\xBB redirecting create_issue_comment(Summary) to update existing comment ${ctx.toolState.existingSummaryCommentId}`
|
||||
);
|
||||
const result2 = await ctx.octokit.rest.issues.updateComment({
|
||||
owner: ctx.repo.owner,
|
||||
repo: ctx.repo.name,
|
||||
comment_id: ctx.toolState.existingSummaryCommentId,
|
||||
body: bodyWithFooter
|
||||
});
|
||||
if (result2.data.node_id) {
|
||||
await updateCommentNodeId(ctx, "summaryCommentNodeId", result2.data.node_id);
|
||||
}
|
||||
return {
|
||||
success: true,
|
||||
commentId: result2.data.id,
|
||||
url: result2.data.html_url,
|
||||
body: result2.data.body
|
||||
};
|
||||
}
|
||||
const result = await ctx.octokit.rest.issues.createComment({
|
||||
owner: ctx.repo.owner,
|
||||
repo: ctx.repo.name,
|
||||
@@ -146816,8 +146836,10 @@ function PullRequestInfoTool(ctx) {
|
||||
}
|
||||
|
||||
// mcp/review.ts
|
||||
function isStatusError(err) {
|
||||
return typeof err === "object" && err !== null && "status" in err && typeof err.status === "number";
|
||||
function getHttpStatus(err) {
|
||||
if (typeof err !== "object" || err === null) return void 0;
|
||||
const status = err.status;
|
||||
return typeof status === "number" ? status : void 0;
|
||||
}
|
||||
var CreatePullRequestReview = type({
|
||||
pull_number: type.number.describe("The pull request number to review"),
|
||||
@@ -146833,7 +146855,7 @@ var CreatePullRequestReview = type({
|
||||
"The file path to comment on (relative to repo root). Must be a file that appears in the PR diff."
|
||||
),
|
||||
line: type.number.describe(
|
||||
"End line of the comment range. For single-line comments, set equal to 'start_line'. Use NEW column from diff format."
|
||||
"Line number to comment on. For multi-line ranges, this is the end line. Use NEW column from diff format."
|
||||
),
|
||||
side: type.enumerated("LEFT", "RIGHT").describe(
|
||||
"Side of the diff: LEFT (old code, lines starting with -) or RIGHT (new code, lines starting with + or unchanged). Defaults to RIGHT."
|
||||
@@ -146843,8 +146865,8 @@ var CreatePullRequestReview = type({
|
||||
"Full replacement code for the line range [start_line, line]. MUST preserve the exact indentation of the original code."
|
||||
).optional(),
|
||||
start_line: type.number.describe(
|
||||
"Start line of the comment range. For single-line comments, set equal to 'line'. The range [start_line, line] defines which lines a suggestion replaces."
|
||||
)
|
||||
"Start line for multi-line comment ranges. Omit for single-line comments. The range [start_line, line] defines which lines a suggestion replaces."
|
||||
).optional()
|
||||
}).array().describe(
|
||||
"Inline comments on lines within diff hunks. Feedback about code outside the diff goes in 'body' instead."
|
||||
).optional()
|
||||
@@ -146852,7 +146874,7 @@ var CreatePullRequestReview = type({
|
||||
function CreatePullRequestReviewTool(ctx) {
|
||||
return tool({
|
||||
name: "create_pull_request_review",
|
||||
description: `Submit a review for an existing pull request. IMPORTANT: 95%+ of feedback should be in 'comments' array with file paths and line numbers. Only use 'body' for a 1-2 sentence summary with urgency and critical callouts. Use 'suggestion' to propose replacement code - MUST preserve exact indentation of original code. Example replacing lines 42-44 (3 lines) with 5 lines: { path: 'src/api.ts', start_line: 42, line: 44, suggestion: ' const result = await fetch(url);\\n if (!result.ok) {\\n log.error(result.status);\\n throw new Error("request failed");\\n }' } CONSTRAINT: Inline comments can ONLY target files and lines that appear in the PR diff. Commenting on files or lines outside the diff will cause GitHub API errors. Put feedback about code outside the diff in 'body' instead.`,
|
||||
description: `Submit a review for an existing pull request. Each call creates a permanent, visible review on the PR \u2014 NEVER submit test or diagnostic reviews. IMPORTANT: 95%+ of feedback should be in 'comments' array with file paths and line numbers. Only use 'body' for a 1-2 sentence summary with urgency and critical callouts. Use 'suggestion' to propose replacement code - MUST preserve exact indentation of original code. Example replacing lines 42-44 (3 lines) with 5 lines: { path: 'src/api.ts', start_line: 42, line: 44, suggestion: ' const result = await fetch(url);\\n if (!result.ok) {\\n log.error(result.status);\\n throw new Error("request failed");\\n }' } CONSTRAINT: Inline comments can ONLY target files and lines that appear in the PR diff. If GitHub rejects comments due to incorrect line numbers, re-read the diff and retry.`,
|
||||
parameters: CreatePullRequestReview,
|
||||
execute: execute(async ({ pull_number, body, approved, commit_id, comments = [] }) => {
|
||||
if (body) body = fixDoubleEscapedString(body);
|
||||
@@ -146868,6 +146890,7 @@ function CreatePullRequestReviewTool(ctx) {
|
||||
pull_number,
|
||||
event
|
||||
};
|
||||
let latestHeadSha;
|
||||
if (commit_id) {
|
||||
params.commit_id = commit_id;
|
||||
} else {
|
||||
@@ -146876,42 +146899,54 @@ function CreatePullRequestReviewTool(ctx) {
|
||||
repo: ctx.repo.name,
|
||||
pull_number
|
||||
});
|
||||
params.commit_id = pr.data.head.sha;
|
||||
latestHeadSha = pr.data.head.sha;
|
||||
params.commit_id = ctx.toolState.checkoutSha ?? latestHeadSha;
|
||||
if (ctx.toolState.checkoutSha && latestHeadSha !== ctx.toolState.checkoutSha) {
|
||||
log.info(
|
||||
`anchoring review to checkout ${ctx.toolState.checkoutSha.slice(0, 7)} (HEAD is now ${latestHeadSha.slice(0, 7)})`
|
||||
);
|
||||
}
|
||||
}
|
||||
if (comments.length > 0) {
|
||||
params.comments = comments.map((comment) => {
|
||||
let commentBody = fixDoubleEscapedString(comment.body || "");
|
||||
if (comment.suggestion !== void 0) {
|
||||
const suggestionBlock = "```suggestion\n" + comment.suggestion + "\n```";
|
||||
commentBody = commentBody ? commentBody + "\n\n" + suggestionBlock : suggestionBlock;
|
||||
}
|
||||
const side = comment.side || "RIGHT";
|
||||
const reviewComment = {
|
||||
path: comment.path,
|
||||
line: comment.line,
|
||||
body: commentBody,
|
||||
side,
|
||||
start_line: comment.start_line,
|
||||
start_side: side
|
||||
};
|
||||
return reviewComment;
|
||||
});
|
||||
const reviewComments = comments.map((comment) => {
|
||||
let commentBody = fixDoubleEscapedString(comment.body || "");
|
||||
if (comment.suggestion !== void 0) {
|
||||
const suggestionBlock = "```suggestion\n" + comment.suggestion + "\n```";
|
||||
commentBody = commentBody ? commentBody + "\n\n" + suggestionBlock : suggestionBlock;
|
||||
}
|
||||
const side = comment.side || "RIGHT";
|
||||
const reviewComment = {
|
||||
path: comment.path,
|
||||
line: comment.line,
|
||||
body: commentBody,
|
||||
side
|
||||
};
|
||||
if (comment.start_line != null && comment.start_line !== comment.line) {
|
||||
reviewComment.start_line = comment.start_line;
|
||||
reviewComment.start_side = side;
|
||||
}
|
||||
return reviewComment;
|
||||
});
|
||||
if (reviewComments.length > 0) {
|
||||
params.comments = reviewComments;
|
||||
}
|
||||
let result;
|
||||
try {
|
||||
result = body ? await createAndSubmitWithFooter(ctx, params, {
|
||||
body,
|
||||
approved: approved ?? false,
|
||||
hasComments: comments.length > 0
|
||||
hasComments: reviewComments.length > 0
|
||||
}) : await ctx.octokit.rest.pulls.createReview(params);
|
||||
} catch (err) {
|
||||
if (isStatusError(err) && err.status === 422 && params.comments?.length) {
|
||||
const paths = [...new Set(params.comments.map((comment) => comment.path))];
|
||||
throw new Error(
|
||||
`${err.message ?? "422 Unprocessable Entity"}. The review had ${params.comments.length} inline comment(s) targeting these paths: ${paths.join(", ")}. GitHub cannot resolve one or more of these paths in the PR diff (common when the PR has >100 changed files and some are truncated). Fix: remove the failing comment(s) and retry. Put their feedback in the review body instead.`
|
||||
);
|
||||
}
|
||||
throw err;
|
||||
if (getHttpStatus(err) !== 422 || !params.comments?.length) throw err;
|
||||
const details = params.comments.map((c) => {
|
||||
const line = c.line ?? 0;
|
||||
const startLine = c.start_line ?? line;
|
||||
const range2 = startLine !== line ? `${startLine}-${line}` : `${line}`;
|
||||
return `${c.path}:${range2} (${c.side ?? "RIGHT"})`;
|
||||
});
|
||||
throw new Error(
|
||||
`GitHub rejected inline comment(s) with "Line could not be resolved". This usually means the diff changed since you last read it (new commits pushed). Re-read the diff to get current line numbers, or move failing comments to the review body. Affected: ${details.join(", ")}`
|
||||
);
|
||||
}
|
||||
log.debug(`createReview response: ${JSON.stringify(result.data)}`);
|
||||
if (!result.data.id) {
|
||||
@@ -146925,10 +146960,9 @@ function CreatePullRequestReviewTool(ctx) {
|
||||
nodeId: reviewNodeId,
|
||||
reviewedSha: actuallyReviewedSha
|
||||
};
|
||||
const headMovedDuringReview = ctx.toolState.checkoutSha && params.commit_id !== ctx.toolState.checkoutSha;
|
||||
if (headMovedDuringReview) {
|
||||
if (ctx.toolState.checkoutSha && latestHeadSha && latestHeadSha !== ctx.toolState.checkoutSha) {
|
||||
const fromSha = ctx.toolState.checkoutSha;
|
||||
const toSha = params.commit_id;
|
||||
const toSha = latestHeadSha;
|
||||
ctx.toolState.checkoutSha = toSha;
|
||||
log.info(
|
||||
`new commits detected during review: ${fromSha.slice(0, 7)}..${toSha.slice(0, 7)}`
|
||||
@@ -147699,12 +147733,12 @@ function buildOrchestratorGuidance(mode, opts = {}) {
|
||||
};
|
||||
}
|
||||
async function fetchExistingPlanComment(ctx, issueNumber) {
|
||||
if (!ctx.apiToken) return null;
|
||||
if (!ctx.githubInstallationToken) return null;
|
||||
try {
|
||||
const response = await apiFetch({
|
||||
path: `/api/repo/${ctx.repo.owner}/${ctx.repo.name}/issue/${issueNumber}/plan-comment`,
|
||||
method: "GET",
|
||||
headers: { authorization: `Bearer ${ctx.apiToken}` },
|
||||
headers: { authorization: `Bearer ${ctx.githubInstallationToken}` },
|
||||
signal: AbortSignal.timeout(1e4)
|
||||
});
|
||||
const data = await response.json();
|
||||
@@ -147714,17 +147748,27 @@ async function fetchExistingPlanComment(ctx, issueNumber) {
|
||||
}
|
||||
}
|
||||
async function fetchExistingSummaryComment(ctx, prNumber) {
|
||||
if (!ctx.apiToken) return null;
|
||||
if (!ctx.githubInstallationToken) {
|
||||
log.warning("fetchExistingSummaryComment: no token, skipping");
|
||||
return null;
|
||||
}
|
||||
const path3 = `/api/repo/${ctx.repo.owner}/${ctx.repo.name}/pr/${prNumber}/summary-comment`;
|
||||
try {
|
||||
const response = await apiFetch({
|
||||
path: `/api/repo/${ctx.repo.owner}/${ctx.repo.name}/pr/${prNumber}/summary-comment`,
|
||||
path: path3,
|
||||
method: "GET",
|
||||
headers: { authorization: `Bearer ${ctx.apiToken}` },
|
||||
headers: { authorization: `Bearer ${ctx.githubInstallationToken}` },
|
||||
signal: AbortSignal.timeout(1e4)
|
||||
});
|
||||
const data = await response.json();
|
||||
return response.ok && "commentId" in data ? data : null;
|
||||
} catch {
|
||||
if (response.ok && "commentId" in data) {
|
||||
return data;
|
||||
}
|
||||
const errMsg = "error" in data ? data.error : "(no error body)";
|
||||
log.warning(`fetchExistingSummaryComment: ${response.status} ${path3} \u2014 ${errMsg}`);
|
||||
return null;
|
||||
} catch (error49) {
|
||||
log.warning("fetchExistingSummaryComment failed:", error49);
|
||||
return null;
|
||||
}
|
||||
}
|
||||
|
||||
+26
-1
@@ -11,7 +11,8 @@ import { execute, tool } from "./shared.ts";
|
||||
|
||||
type CommentNodeIdField = "planCommentNodeId" | "summaryCommentNodeId";
|
||||
|
||||
/** PATCH workflow-run with a comment node_id so future runs can update that comment in place. */
|
||||
// IMPORTANT: this route authenticates via Pullfrog API JWT (verifyApiToken),
|
||||
// NOT a GitHub token. use ctx.apiToken here. see wiki/api-auth.md.
|
||||
export async function updateCommentNodeId(
|
||||
ctx: ToolContext,
|
||||
field: CommentNodeIdField,
|
||||
@@ -129,6 +130,30 @@ export function CreateCommentTool(ctx: ToolContext) {
|
||||
execute: execute(async ({ issueNumber, body, type: commentType }) => {
|
||||
const bodyWithFooter = await addFooter(ctx, body);
|
||||
|
||||
// if a summary comment already exists (found by select_mode), update instead of creating
|
||||
if (commentType === "Summary" && ctx.toolState.existingSummaryCommentId) {
|
||||
log.info(
|
||||
`» redirecting create_issue_comment(Summary) to update existing comment ${ctx.toolState.existingSummaryCommentId}`
|
||||
);
|
||||
const result = await ctx.octokit.rest.issues.updateComment({
|
||||
owner: ctx.repo.owner,
|
||||
repo: ctx.repo.name,
|
||||
comment_id: ctx.toolState.existingSummaryCommentId,
|
||||
body: bodyWithFooter,
|
||||
});
|
||||
|
||||
if (result.data.node_id) {
|
||||
await updateCommentNodeId(ctx, "summaryCommentNodeId", result.data.node_id);
|
||||
}
|
||||
|
||||
return {
|
||||
success: true,
|
||||
commentId: result.data.id,
|
||||
url: result.data.html_url,
|
||||
body: result.data.body,
|
||||
};
|
||||
}
|
||||
|
||||
const result = await ctx.octokit.rest.issues.createComment({
|
||||
owner: ctx.repo.owner,
|
||||
repo: ctx.repo.name,
|
||||
|
||||
+66
-47
@@ -8,10 +8,10 @@ import { fixDoubleEscapedString } from "../utils/fixDoubleEscapedString.ts";
|
||||
import type { ToolContext } from "./server.ts";
|
||||
import { execute, tool } from "./shared.ts";
|
||||
|
||||
function isStatusError(err: unknown): err is { status: number; message?: string } {
|
||||
return (
|
||||
typeof err === "object" && err !== null && "status" in err && typeof err.status === "number"
|
||||
);
|
||||
function getHttpStatus(err: unknown): number | undefined {
|
||||
if (typeof err !== "object" || err === null) return undefined;
|
||||
const status = (err as Record<string, unknown>).status;
|
||||
return typeof status === "number" ? status : undefined;
|
||||
}
|
||||
|
||||
// one-shot review tool
|
||||
@@ -35,7 +35,7 @@ export const CreatePullRequestReview = type({
|
||||
"The file path to comment on (relative to repo root). Must be a file that appears in the PR diff."
|
||||
),
|
||||
line: type.number.describe(
|
||||
"End line of the comment range. For single-line comments, set equal to 'start_line'. Use NEW column from diff format."
|
||||
"Line number to comment on. For multi-line ranges, this is the end line. Use NEW column from diff format."
|
||||
),
|
||||
side: type
|
||||
.enumerated("LEFT", "RIGHT")
|
||||
@@ -51,9 +51,11 @@ export const CreatePullRequestReview = type({
|
||||
"Full replacement code for the line range [start_line, line]. MUST preserve the exact indentation of the original code."
|
||||
)
|
||||
.optional(),
|
||||
start_line: type.number.describe(
|
||||
"Start line of the comment range. For single-line comments, set equal to 'line'. The range [start_line, line] defines which lines a suggestion replaces."
|
||||
),
|
||||
start_line: type.number
|
||||
.describe(
|
||||
"Start line for multi-line comment ranges. Omit for single-line comments. The range [start_line, line] defines which lines a suggestion replaces."
|
||||
)
|
||||
.optional(),
|
||||
})
|
||||
.array()
|
||||
.describe(
|
||||
@@ -67,14 +69,14 @@ export function CreatePullRequestReviewTool(ctx: ToolContext) {
|
||||
name: "create_pull_request_review",
|
||||
description:
|
||||
"Submit a review for an existing pull request. " +
|
||||
"Each call creates a permanent, visible review on the PR — NEVER submit test or diagnostic reviews. " +
|
||||
"IMPORTANT: 95%+ of feedback should be in 'comments' array with file paths and line numbers. " +
|
||||
"Only use 'body' for a 1-2 sentence summary with urgency and critical callouts. " +
|
||||
"Use 'suggestion' to propose replacement code - MUST preserve exact indentation of original code. " +
|
||||
"Example replacing lines 42-44 (3 lines) with 5 lines: " +
|
||||
`{ path: 'src/api.ts', start_line: 42, line: 44, suggestion: ' const result = await fetch(url);\\n if (!result.ok) {\\n log.error(result.status);\\n throw new Error("request failed");\\n }' }` +
|
||||
" CONSTRAINT: Inline comments can ONLY target files and lines that appear in the PR diff." +
|
||||
" Commenting on files or lines outside the diff will cause GitHub API errors." +
|
||||
" Put feedback about code outside the diff in 'body' instead.",
|
||||
" If GitHub rejects comments due to incorrect line numbers, re-read the diff and retry.",
|
||||
parameters: CreatePullRequestReview,
|
||||
execute: execute(async ({ pull_number, body, approved, commit_id, comments = [] }) => {
|
||||
if (body) body = fixDoubleEscapedString(body);
|
||||
@@ -95,6 +97,7 @@ export function CreatePullRequestReviewTool(ctx: ToolContext) {
|
||||
pull_number,
|
||||
event,
|
||||
};
|
||||
let latestHeadSha: string | undefined;
|
||||
if (commit_id) {
|
||||
params.commit_id = commit_id;
|
||||
} else {
|
||||
@@ -103,28 +106,39 @@ export function CreatePullRequestReviewTool(ctx: ToolContext) {
|
||||
repo: ctx.repo.name,
|
||||
pull_number,
|
||||
});
|
||||
params.commit_id = pr.data.head.sha;
|
||||
latestHeadSha = pr.data.head.sha;
|
||||
// anchor to checkout sha so line numbers match the diff the agent analyzed
|
||||
params.commit_id = ctx.toolState.checkoutSha ?? latestHeadSha;
|
||||
if (ctx.toolState.checkoutSha && latestHeadSha !== ctx.toolState.checkoutSha) {
|
||||
log.info(
|
||||
`anchoring review to checkout ${ctx.toolState.checkoutSha.slice(0, 7)} ` +
|
||||
`(HEAD is now ${latestHeadSha.slice(0, 7)})`
|
||||
);
|
||||
}
|
||||
}
|
||||
if (comments.length > 0) {
|
||||
type ReviewComment = (typeof params.comments & {})[number];
|
||||
params.comments = comments.map((comment) => {
|
||||
let commentBody = fixDoubleEscapedString(comment.body || "");
|
||||
if (comment.suggestion !== undefined) {
|
||||
const suggestionBlock = "```suggestion\n" + comment.suggestion + "\n```";
|
||||
commentBody = commentBody ? commentBody + "\n\n" + suggestionBlock : suggestionBlock;
|
||||
}
|
||||
type ReviewComment = NonNullable<typeof params.comments>[number];
|
||||
const reviewComments = comments.map((comment) => {
|
||||
let commentBody = fixDoubleEscapedString(comment.body || "");
|
||||
if (comment.suggestion !== undefined) {
|
||||
const suggestionBlock = "```suggestion\n" + comment.suggestion + "\n```";
|
||||
commentBody = commentBody ? commentBody + "\n\n" + suggestionBlock : suggestionBlock;
|
||||
}
|
||||
const side = comment.side || "RIGHT";
|
||||
const reviewComment: ReviewComment = {
|
||||
path: comment.path,
|
||||
line: comment.line,
|
||||
body: commentBody,
|
||||
side,
|
||||
};
|
||||
if (comment.start_line != null && comment.start_line !== comment.line) {
|
||||
reviewComment.start_line = comment.start_line;
|
||||
reviewComment.start_side = side;
|
||||
}
|
||||
return reviewComment;
|
||||
});
|
||||
|
||||
const side = comment.side || "RIGHT";
|
||||
const reviewComment: ReviewComment = {
|
||||
path: comment.path,
|
||||
line: comment.line,
|
||||
body: commentBody,
|
||||
side,
|
||||
start_line: comment.start_line,
|
||||
start_side: side,
|
||||
};
|
||||
return reviewComment;
|
||||
});
|
||||
if (reviewComments.length > 0) {
|
||||
params.comments = reviewComments;
|
||||
}
|
||||
|
||||
// no body → single-step createReview (no footer needed)
|
||||
@@ -135,20 +149,24 @@ export function CreatePullRequestReviewTool(ctx: ToolContext) {
|
||||
? await createAndSubmitWithFooter(ctx, params, {
|
||||
body,
|
||||
approved: approved ?? false,
|
||||
hasComments: comments.length > 0,
|
||||
hasComments: reviewComments.length > 0,
|
||||
})
|
||||
: await ctx.octokit.rest.pulls.createReview(params);
|
||||
} catch (err: unknown) {
|
||||
if (isStatusError(err) && err.status === 422 && params.comments?.length) {
|
||||
const paths = [...new Set(params.comments.map((comment) => comment.path))];
|
||||
throw new Error(
|
||||
`${err.message ?? "422 Unprocessable Entity"}. ` +
|
||||
`The review had ${params.comments.length} inline comment(s) targeting these paths: ${paths.join(", ")}. ` +
|
||||
`GitHub cannot resolve one or more of these paths in the PR diff (common when the PR has >100 changed files and some are truncated). ` +
|
||||
`Fix: remove the failing comment(s) and retry. Put their feedback in the review body instead.`
|
||||
);
|
||||
}
|
||||
throw err;
|
||||
if (getHttpStatus(err) !== 422 || !params.comments?.length) throw err;
|
||||
|
||||
const details = params.comments.map((c) => {
|
||||
const line = c.line ?? 0;
|
||||
const startLine = c.start_line ?? line;
|
||||
const range = startLine !== line ? `${startLine}-${line}` : `${line}`;
|
||||
return `${c.path}:${range} (${c.side ?? "RIGHT"})`;
|
||||
});
|
||||
throw new Error(
|
||||
`GitHub rejected inline comment(s) with "Line could not be resolved". ` +
|
||||
`This usually means the diff changed since you last read it (new commits pushed). ` +
|
||||
`Re-read the diff to get current line numbers, or move failing comments to the review body. ` +
|
||||
`Affected: ${details.join(", ")}`
|
||||
);
|
||||
}
|
||||
log.debug(`createReview response: ${JSON.stringify(result.data)}`);
|
||||
if (!result.data.id) {
|
||||
@@ -169,12 +187,13 @@ export function CreatePullRequestReviewTool(ctx: ToolContext) {
|
||||
|
||||
// detect commits pushed since checkout and guide the agent to review them
|
||||
// inline instead of dispatching a separate workflow run
|
||||
const headMovedDuringReview =
|
||||
ctx.toolState.checkoutSha && params.commit_id !== ctx.toolState.checkoutSha;
|
||||
|
||||
if (headMovedDuringReview) {
|
||||
const fromSha = ctx.toolState.checkoutSha!;
|
||||
const toSha = params.commit_id!;
|
||||
if (
|
||||
ctx.toolState.checkoutSha &&
|
||||
latestHeadSha &&
|
||||
latestHeadSha !== ctx.toolState.checkoutSha
|
||||
) {
|
||||
const fromSha = ctx.toolState.checkoutSha;
|
||||
const toSha = latestHeadSha;
|
||||
// advance checkoutSha so the next review submission tracks correctly
|
||||
ctx.toolState.checkoutSha = toSha;
|
||||
|
||||
|
||||
+21
-7
@@ -2,6 +2,7 @@ import { type } from "arktype";
|
||||
import { ghPullfrogMcpName } from "../external.ts";
|
||||
import type { Mode } from "../modes.ts";
|
||||
import { apiFetch } from "../utils/apiFetch.ts";
|
||||
import { log } from "../utils/log.ts";
|
||||
import type { ToolContext } from "./server.ts";
|
||||
import { execute, tool } from "./shared.ts";
|
||||
|
||||
@@ -253,16 +254,19 @@ export type PlanCommentResponsePayload = { error: string } | { commentId: number
|
||||
// matches the API response for /repo/[owner]/[repo]/pr/[prNumber]/summary-comment
|
||||
export type SummaryCommentResponsePayload = { error: string } | { commentId: number; body: string };
|
||||
|
||||
// IMPORTANT: these routes authenticate via GitHub installation token (getEnrichedRepo),
|
||||
// NOT the Pullfrog API JWT (ctx.apiToken). use ctx.githubInstallationToken here.
|
||||
// see wiki/api-auth.md for the two auth patterns.
|
||||
async function fetchExistingPlanComment(
|
||||
ctx: ToolContext,
|
||||
issueNumber: number
|
||||
): Promise<Extract<PlanCommentResponsePayload, { commentId: number }> | null> {
|
||||
if (!ctx.apiToken) return null;
|
||||
if (!ctx.githubInstallationToken) return null;
|
||||
try {
|
||||
const response = await apiFetch({
|
||||
path: `/api/repo/${ctx.repo.owner}/${ctx.repo.name}/issue/${issueNumber}/plan-comment`,
|
||||
method: "GET",
|
||||
headers: { authorization: `Bearer ${ctx.apiToken}` },
|
||||
headers: { authorization: `Bearer ${ctx.githubInstallationToken}` },
|
||||
signal: AbortSignal.timeout(10_000),
|
||||
});
|
||||
const data = (await response.json()) as PlanCommentResponsePayload;
|
||||
@@ -276,17 +280,27 @@ async function fetchExistingSummaryComment(
|
||||
ctx: ToolContext,
|
||||
prNumber: number
|
||||
): Promise<Extract<SummaryCommentResponsePayload, { commentId: number }> | null> {
|
||||
if (!ctx.apiToken) return null;
|
||||
if (!ctx.githubInstallationToken) {
|
||||
log.warning("fetchExistingSummaryComment: no token, skipping");
|
||||
return null;
|
||||
}
|
||||
const path = `/api/repo/${ctx.repo.owner}/${ctx.repo.name}/pr/${prNumber}/summary-comment`;
|
||||
try {
|
||||
const response = await apiFetch({
|
||||
path: `/api/repo/${ctx.repo.owner}/${ctx.repo.name}/pr/${prNumber}/summary-comment`,
|
||||
path,
|
||||
method: "GET",
|
||||
headers: { authorization: `Bearer ${ctx.apiToken}` },
|
||||
headers: { authorization: `Bearer ${ctx.githubInstallationToken}` },
|
||||
signal: AbortSignal.timeout(10_000),
|
||||
});
|
||||
const data = (await response.json()) as SummaryCommentResponsePayload;
|
||||
return response.ok && "commentId" in data ? data : null;
|
||||
} catch {
|
||||
if (response.ok && "commentId" in data) {
|
||||
return data;
|
||||
}
|
||||
const errMsg = "error" in data ? data.error : "(no error body)";
|
||||
log.warning(`fetchExistingSummaryComment: ${response.status} ${path} — ${errMsg}`);
|
||||
return null;
|
||||
} catch (error) {
|
||||
log.warning("fetchExistingSummaryComment failed:", error);
|
||||
return null;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -19,7 +19,7 @@ exports[`latest model per provider snapshot > matches snapshot 1`] = `
|
||||
"releaseDate": "2026-01",
|
||||
},
|
||||
"openai": {
|
||||
"modelId": "gpt-5.4",
|
||||
"modelId": "gpt-5.4-pro",
|
||||
"releaseDate": "2026-03-05",
|
||||
},
|
||||
"opencode": {
|
||||
@@ -31,8 +31,8 @@ exports[`latest model per provider snapshot > matches snapshot 1`] = `
|
||||
"releaseDate": "2026-03-11",
|
||||
},
|
||||
"xai": {
|
||||
"modelId": "grok-4.20-beta-latest-non-reasoning",
|
||||
"releaseDate": "2026-03-09",
|
||||
"modelId": "grok-4-1-fast-non-reasoning",
|
||||
"releaseDate": "2025-11-19",
|
||||
},
|
||||
}
|
||||
`;
|
||||
|
||||
+8
-2
@@ -58,10 +58,16 @@ describe("latest model per provider snapshot", async () => {
|
||||
|
||||
let latest: { modelId: string; releaseDate: string } | undefined;
|
||||
for (const [modelId, model] of Object.entries(providerData.models)) {
|
||||
if (model.status === "deprecated") continue;
|
||||
// skip non-GA models so beta/nightly churn doesn't break the snapshot
|
||||
if (model.status) continue;
|
||||
const rd = model.release_date;
|
||||
if (!rd) continue;
|
||||
if (!latest || rd > latest.releaseDate) {
|
||||
// tiebreak by modelId for stable ordering when release dates match
|
||||
if (
|
||||
!latest ||
|
||||
rd > latest.releaseDate ||
|
||||
(rd === latest.releaseDate && modelId > latest.modelId)
|
||||
) {
|
||||
latest = { modelId, releaseDate: rd };
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user