diff --git a/external.ts b/external.ts index 869f4a1..01b1d92 100644 --- a/external.ts +++ b/external.ts @@ -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 diff --git a/internal/index.ts b/internal/index.ts index 46ea930..2242e89 100644 --- a/internal/index.ts +++ b/internal/index.ts @@ -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, diff --git a/main.ts b/main.ts index a673a8a..3463ea2 100644 --- a/main.ts +++ b/main.ts @@ -319,12 +319,12 @@ export async function main(): Promise { 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 @@ -577,6 +577,15 @@ export async function main(): Promise { }); 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 @@ -707,7 +716,7 @@ export async function main(): Promise { const trackerWasLastWriter = todoTracker?.hasPublished && !toolState.finalSummaryWritten; if ( toolContext && - toolState.progressCommentId && + toolState.progressComment && (!toolState.wasUpdated || trackerWasLastWriter) ) { await deleteProgressComment(toolContext).catch((error) => { diff --git a/mcp/comment.ts b/mcp/comment.ts index 4ff456e..c70a07f 100644 --- a/mcp/comment.ts +++ b/mcp/comment.ts @@ -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 { - 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 } // 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; } diff --git a/mcp/review.ts b/mcp/review.ts index 63021cf..dedbf71 100644 --- a/mcp/review.ts +++ b/mcp/review.ts @@ -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. diff --git a/mcp/server.ts b/mcp/server.ts index f29979c..df3fd38 100644 --- a/mcp/server.ts +++ b/mcp/server.ts @@ -12,6 +12,11 @@ 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"; @@ -110,8 +115,8 @@ export interface ToolState { promise: Promise | 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; @@ -133,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: [], }; diff --git a/package.json b/package.json index b469b8c..5112f8d 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "pullfrog", - "version": "0.0.203", + "version": "0.0.204", "type": "module", "bin": { "pullfrog": "dist/cli.mjs", diff --git a/utils/errorReport.ts b/utils/errorReport.ts index bb3cee2..d952669 100644 --- a/utils/errorReport.ts +++ b/utils/errorReport.ts @@ -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 { 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 repoSettings > fallbacks push: inputs.push ?? repoSettings.push ?? "restricted", diff --git a/utils/postCleanup.ts b/utils/postCleanup.ts index 3084a18..5fabbe1 100644 --- a/utils/postCleanup.ts +++ b/utils/postCleanup.ts @@ -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; // not string @@ -50,41 +56,47 @@ function buildErrorCommentBody(ctx: PostCleanupContext, isCancellation: boolean) return `${errorMessage}${footer}`; } -async function validateStuckProgressComment(ctx: PostCleanupContext): Promise { - if (!ctx.promptInput?.progressCommentId) { - log.info("[post] no progressCommentId in prompt input, skipping cleanup"); +async function validateStuckProgressComment( + ctx: PostCleanupContext +): Promise { + 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 { 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 { 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 { + 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>; + 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)` + ); +} diff --git a/utils/progressComment.ts b/utils/progressComment.ts new file mode 100644 index 0000000..4a1dd98 --- /dev/null +++ b/utils/progressComment.ts @@ -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; + getComment: (params: { + owner: string; + repo: string; + comment_id: number; + }) => Promise; + updateComment: (params: { + owner: string; + repo: string; + comment_id: number; + body: string; + }) => Promise; + deleteComment: (params: { + owner: string; + repo: string; + comment_id: number; + }) => Promise; + }; + pulls: { + createReplyForReviewComment: (params: { + owner: string; + repo: string; + pull_number: number; + comment_id: number; + body: string; + }) => Promise; + getReviewComment: (params: { + owner: string; + repo: string; + comment_id: number; + }) => Promise; + updateReviewComment: (params: { + owner: string; + repo: string; + comment_id: number; + body: string; + }) => Promise; + deleteReviewComment: (params: { + owner: string; + repo: string; + comment_id: number; + }) => Promise; + }; + }; +} + +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 { + 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 { + 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, + }; +}