diff --git a/.github/workflows/pullfrog.yml b/.github/workflows/pullfrog.yml index 0b4f6c1..079fac6 100644 --- a/.github/workflows/pullfrog.yml +++ b/.github/workflows/pullfrog.yml @@ -12,16 +12,14 @@ on: description: Run name permissions: - id-token: write - contents: write - pull-requests: write - issues: write - actions: read - checks: read + contents: read jobs: pullfrog: runs-on: ubuntu-latest + permissions: + id-token: write + contents: read steps: - name: Checkout code uses: actions/checkout@v6 diff --git a/README.md b/README.md index 0ae1b2c..96f417e 100644 --- a/README.md +++ b/README.md @@ -72,16 +72,14 @@ on: description: 'Agent prompt' permissions: - id-token: write - contents: write - pull-requests: write - issues: write - actions: read - checks: read + contents: read jobs: pullfrog: runs-on: ubuntu-latest + permissions: + id-token: write + contents: read steps: - name: Checkout code uses: actions/checkout@v6 @@ -130,11 +128,7 @@ jobs: permissions: id-token: write - contents: write - issues: write - pull-requests: write - actions: read - checks: read + contents: read uses: ./.github/workflows/pullfrog.yml with: # pass the full event payload as the prompt diff --git a/action.yml b/action.yml index 6508618..e44a807 100644 --- a/action.yml +++ b/action.yml @@ -36,8 +36,6 @@ outputs: runs: using: "node24" main: "entry.ts" - post: "post.ts" - post-if: "failure() || cancelled()" branding: icon: "code" diff --git a/commands/gha.ts b/commands/gha.ts index db05c6a..b6cb5f6 100644 --- a/commands/gha.ts +++ b/commands/gha.ts @@ -2,8 +2,6 @@ import { dirname } from "node:path"; import * as core from "@actions/core"; import arg from "arg"; import { main } from "../main.ts"; -import { log } from "../utils/cli.ts"; -import { runPostCleanup } from "../utils/postCleanup.ts"; import { acquireInstallationToken, revokeInstallationToken } from "../utils/token.ts"; // GitHub Actions runs the action entry point with the node24 binary specified @@ -31,16 +29,6 @@ async function runMain(): Promise { } } -async function runPost(): Promise { - log.debug(`[post] script started at ${new Date().toISOString()}`); - try { - await runPostCleanup(); - } catch (error) { - const message = error instanceof Error ? error.message : String(error); - log.error(`[post] unexpected error: ${message}`); - } -} - async function tokenMain(): Promise { const reposInput = core.getInput("repos"); const additionalRepos = reposInput @@ -73,7 +61,7 @@ async function tokenPost(): Promise { } function printGhaUsage(params: { stream: typeof console.log; prog: string }): void { - params.stream(`usage: ${params.prog} gha [token] [--post]\n`); + params.stream(`usage: ${params.prog} gha [subcommand]\n`); params.stream("run the github action runtime flow."); params.stream(""); params.stream("subcommands:"); @@ -81,10 +69,31 @@ function printGhaUsage(params: { stream: typeof console.log; prog: string }): vo params.stream(""); params.stream("options:"); params.stream(" -h, --help show help"); - params.stream(" --post run post-cleanup flow"); +} + +function printGhaTokenUsage(params: { stream: typeof console.log; prog: string }): void { + params.stream(`usage: ${params.prog} gha token [--post]\n`); + params.stream("acquire a github app installation token, or revoke it in the post step."); + params.stream(""); + params.stream("options:"); + params.stream(" -h, --help show help"); + params.stream(" --post revoke the previously-acquired token (post-step usage only)"); } function parseGhaArgs(args: string[]) { + return arg( + { + "--help": Boolean, + "-h": "--help", + }, + { + argv: args, + stopAtPositional: true, + } + ); +} + +function parseGhaTokenArgs(args: string[]) { return arg( { "--help": Boolean, @@ -118,27 +127,46 @@ export async function runCli(params: GhaCliParams): Promise { return; } - const normalizedArgs = ["gha"]; const positional = parsed._; + const subcommand = positional[0]; - if (positional.length > 1) { - console.error(`unexpected positional arguments for gha: ${positional.slice(1).join(" ")}\n`); + if (!subcommand) { + await run(["gha"]); + return; + } + + if (subcommand !== "token") { + console.error(`unknown gha subcommand: ${subcommand}\n`); printGhaUsage({ stream: console.error, prog: params.prog }); process.exit(1); } - if (positional[0] === "token") { - normalizedArgs.push("token"); - } else if (positional[0]) { - console.error(`unknown gha subcommand: ${positional[0]}\n`); - printGhaUsage({ stream: console.error, prog: params.prog }); + // gha token [--post] + let tokenParsed: ReturnType; + try { + tokenParsed = parseGhaTokenArgs(positional.slice(1)); + } catch (error) { + const message = error instanceof Error ? error.message : String(error); + console.error(`${message}\n`); + printGhaTokenUsage({ stream: console.error, prog: params.prog }); process.exit(1); } - if (parsed["--post"]) { + if (tokenParsed["--help"]) { + printGhaTokenUsage({ stream: console.log, prog: params.prog }); + return; + } + + if (tokenParsed._.length > 0) { + console.error(`unexpected positional arguments for gha token: ${tokenParsed._.join(" ")}\n`); + printGhaTokenUsage({ stream: console.error, prog: params.prog }); + process.exit(1); + } + + const normalizedArgs = ["gha", "token"]; + if (tokenParsed["--post"]) { normalizedArgs.push("--post"); } - await run(normalizedArgs); } @@ -150,8 +178,6 @@ export async function run(args: string[]) { } else { await tokenMain(); } - } else if (args.includes("--post")) { - await runPost(); } else { await runMain(); } diff --git a/internal/index.ts b/internal/index.ts index 2242e89..caff525 100644 --- a/internal/index.ts +++ b/internal/index.ts @@ -40,6 +40,10 @@ export { stripExistingFooter, } from "../utils/buildPullfrogFooter.ts"; export type { ResourceUsage, UsageSummary } from "../utils/github.ts"; +export { + isLeapingIntoActionCommentBody, + LEAPING_INTO_ACTION_PREFIX, +} from "../utils/leapingComment.ts"; export type { CreateProgressCommentTarget, ProgressComment, diff --git a/main.ts b/main.ts index 3960f0d..75492cd 100644 --- a/main.ts +++ b/main.ts @@ -578,10 +578,11 @@ 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. + // debounced write queued just before SIGTERM could land at GitHub *after* the + // workflow_run.completed webhook has already replaced the comment with the + // "This run was cancelled" body, 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. onExitSignal(() => { todoTracker?.cancel(); }); diff --git a/mcp/comment.ts b/mcp/comment.ts index a797375..8731160 100644 --- a/mcp/comment.ts +++ b/mcp/comment.ts @@ -12,18 +12,11 @@ import { import type { ToolContext } from "./server.ts"; import { execute, tool } from "./shared.ts"; -/** - * The prefix text for the initial "leaping into action" comment. - * This is used to identify if a comment is still in its initial state - * and hasn't been updated with progress or error messages. - */ -export const LEAPING_INTO_ACTION_PREFIX = "Leaping into action"; - -export function isLeapingIntoActionCommentBody(body: string): boolean { - const content = stripExistingFooter(body).trimStart(); - const firstLine = content.split(/\r?\n/, 1)[0]?.trimEnd() ?? ""; - return new RegExp(`(^|\\s)${LEAPING_INTO_ACTION_PREFIX}(\\.\\.\\.)?$`).test(firstLine); -} +// re-export for backward compat with anything importing the leaping helpers from mcp/comment +export { + isLeapingIntoActionCommentBody, + LEAPING_INTO_ACTION_PREFIX, +} from "../utils/leapingComment.ts"; function buildCommentFooter(ctx: ToolContext, customParts?: string[]): string { const runId = ctx.runId; @@ -447,7 +440,8 @@ export function ReplyToReviewCommentTool(ctx: ToolContext) { body: bodyWithFooter, }); - // mark progress as updated so post script doesn't think the run failed + // mark progress as updated so error reporting + run-result handling know + // a substantive write happened (used by reportErrorToComment / handleAgentResult) ctx.toolState.wasUpdated = true; return { diff --git a/play.ts b/play.ts index 8eeef8e..9d2387b 100644 --- a/play.ts +++ b/play.ts @@ -12,7 +12,6 @@ import { log } from "./utils/cli.ts"; import { runInDocker } from "./utils/docker.ts"; import { ensureGitHubToken } from "./utils/github.ts"; import { isInsideDocker } from "./utils/globals.ts"; -import { runPostCleanup } from "./utils/postCleanup.ts"; import { setupTestRepo } from "./utils/setup.ts"; /** @@ -78,13 +77,7 @@ export async function run(inputsOrPrompt: Inputs | string): Promise } } - // wrap main() so post cleanup runs even on failure (mirrors action.yml post-if: "failure() || cancelled()") - let result: AgentResult; - try { - result = await main(); - } finally { - await runPostCleanup(); - } + const result: AgentResult = await main(); process.chdir(originalCwd); diff --git a/post.ts b/post.ts deleted file mode 100644 index d13c144..0000000 --- a/post.ts +++ /dev/null @@ -1,8 +0,0 @@ -#!/usr/bin/env node - -import { runPullfrogCli } from "./runCli.ts"; - -runPullfrogCli({ - cliArgs: ["gha", "--post"], - swallowErrors: true, -}); diff --git a/scripts/check-entrypoint-imports.ts b/scripts/check-entrypoint-imports.ts index 60ac9cd..9db6a8f 100644 --- a/scripts/check-entrypoint-imports.ts +++ b/scripts/check-entrypoint-imports.ts @@ -7,7 +7,6 @@ const scriptDir = dirname(fileURLToPath(import.meta.url)); const entryPoints = [ resolve(scriptDir, "../entry.ts"), - resolve(scriptDir, "../post.ts"), resolve(scriptDir, "../get-installation-token/entry.ts"), resolve(scriptDir, "../get-installation-token/post.ts"), ]; diff --git a/utils/leapingComment.ts b/utils/leapingComment.ts new file mode 100644 index 0000000..6149894 --- /dev/null +++ b/utils/leapingComment.ts @@ -0,0 +1,18 @@ +import { stripExistingFooter } from "./buildPullfrogFooter.ts"; + +/** + * The prefix text for the initial "leaping into action" comment. + * Used to detect whether a progress comment is still in its initial state + * and hasn't been updated with real progress or error messages. + * + * Lives in `utils/` (not `mcp/`) so it can be re-exported via `pullfrog/internal` + * without dragging the MCP server's transitive imports into the Next.js app's + * type-check graph. + */ +export const LEAPING_INTO_ACTION_PREFIX = "Leaping into action"; + +export function isLeapingIntoActionCommentBody(body: string): boolean { + const content = stripExistingFooter(body).trimStart(); + const firstLine = content.split(/\r?\n/, 1)[0]?.trimEnd() ?? ""; + return new RegExp(`(^|\\s)${LEAPING_INTO_ACTION_PREFIX}(\\.\\.\\.)?$`).test(firstLine); +} diff --git a/utils/postCleanup.ts b/utils/postCleanup.ts deleted file mode 100644 index 5fabbe1..0000000 --- a/utils/postCleanup.ts +++ /dev/null @@ -1,246 +0,0 @@ -import { isLeapingIntoActionCommentBody } from "../mcp/comment.ts"; -import { getApiUrl } from "./apiUrl.ts"; -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 - -interface PostCleanupContext { - repoContext: ReturnType; - octokit: ReturnType; - runId: number | undefined; - promptInput: JsonPromptInput | null; -} - -// controls whether the script should check the reason for the workflow termination. -// it can be either canceled or failed. -// YAML file cannot supply it (not in ENV), so an extra request is required to check it. -const SHOULD_CHECK_REASON = true; - -function buildErrorCommentBody(ctx: PostCleanupContext, isCancellation: boolean): string { - let errorMessage = isCancellation - ? `This run was cancelled ๐Ÿ›‘\n\nThe workflow was cancelled before completion.` - : `This run croaked ๐Ÿ˜ต\n\nThe workflow encountered an error before any progress could be reported.`; - - if (ctx.runId) { - errorMessage += " Please check the link below for details."; - } - - const customParts: string[] = []; - if (!isCancellation && ctx.runId) { - const apiUrl = getApiUrl(); - customParts.push( - `[Rerun failed job โž”](${apiUrl}/trigger/${ctx.repoContext.owner}/${ctx.repoContext.name}/${ctx.runId}?action=rerun)` - ); - } - const footer = buildPullfrogFooter({ - triggeredBy: true, - workflowRun: ctx.runId - ? { - owner: ctx.repoContext.owner, - repo: ctx.repoContext.name, - runId: ctx.runId, - } - : undefined, - customParts, - }); - return `${errorMessage}${footer}`; -} - -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 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 fetched = await getProgressComment( - { octokit: ctx.octokit, owner: ctx.repoContext.owner, repo: ctx.repoContext.name }, - comment - ); - - const body = fetched.body ?? ""; - - if (isLeapingIntoActionCommentBody(body)) { - 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 ${comment.id} is stuck on a todo checklist`); - return comment; - } - - 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 ${comment.id}: ${errorMessage}`); - return null; - } -} - -async function getIsCancelled(ctx: PostCleanupContext): Promise { - if (!ctx.runId) return false; // can't check without a run ID โ€” assume failure - try { - const jobsResult = await ctx.octokit.rest.actions.listJobsForWorkflowRun({ - owner: ctx.repoContext.owner, - repo: ctx.repoContext.name, - run_id: ctx.runId, - }); - - // find current job by matching GITHUB_JOB env var. - // GITHUB_JOB is the job ID (yaml key), but job.name is the display name. - // for matrix jobs, the name includes matrix values like "build (ubuntu-latest, node-18)" - // so we match jobs that START with the job ID - const currentJobName = process.env.GITHUB_JOB; - const currentJob = currentJobName - ? jobsResult.data.jobs.find( - (j) => j.name === currentJobName || j.name.startsWith(`${currentJobName} (`) - ) - : jobsResult.data.jobs[0]; // fallback to first job - - if (!currentJob) { - log.warning("[post] could not find current job"); - return false; - } - - log.info(`[post] job status: ${currentJob.status}, conclusion: ${currentJob.conclusion}`); - if (currentJob.conclusion === "cancelled") return true; // whole job explicit cancellation - - // but if it's still null, check steps for cancellation: - const cancelledStep = currentJob.steps?.find((step) => step.conclusion === "cancelled"); - if (cancelledStep) { - log.info(`[post] found cancelled step: ${cancelledStep.name}`); - return true; - } - log.info("[post] no cancellation found, assuming failure"); - } catch (error) { - log.info( - `[post] failed to get job status: ${error instanceof Error ? error.message : String(error)}` - ); - } - return false; // assuming failure -} - -export async function runPostCleanup(): Promise { - log.info("ยป [post] starting post cleanup"); - - const runId = process.env.GITHUB_RUN_ID - ? Number.parseInt(process.env.GITHUB_RUN_ID, 10) - : undefined; - - // resolve prompt input once and use it for both issue number and comment ID extraction - // only use the object form (JSON payload), not plain string prompts - let promptInput: JsonPromptInput | null = null; - try { - const resolved = resolvePromptInput(); - if (typeof resolved !== "string") promptInput = resolved; - } catch (error) { - log.info( - `[post] failed to resolve prompt input: ${error instanceof Error ? error.message : String(error)}` - ); - } - - // get job token for API calls - const token = getJobToken(); - const repoContext = parseRepoContext(); - const octokit = createOctokit(token); - - const ctx: PostCleanupContext = { repoContext, octokit, runId, promptInput }; - - const stuck = await validateStuckProgressComment(ctx); - - if (!stuck) return log.info("ยป [post] no stuck progress comment to update, skipping cleanup"); - - log.info( - `ยป [post] validated stuck comment: ${stuck.id} (${stuck.type}), updating with error message` - ); - - try { - const body = buildErrorCommentBody( - ctx, - SHOULD_CHECK_REASON ? await getIsCancelled(ctx) : false - ); - - 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)` - ); -}