From ee100354daff2902030f05d47c4bce2055df384b Mon Sep 17 00:00:00 2001 From: "pullfrog[bot]" <226033991+pullfrog[bot]@users.noreply.github.com> Date: Fri, 20 Feb 2026 10:23:56 +0000 Subject: [PATCH] fix: replace domain-specific exit handler with generic signal handler registry (#299) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix: replace domain-specific exit handler with generic signal handler registry Rewrite exitHandler.ts as a generic, domain-agnostic exit signal module that exports onExitSignal(handler) returning a dispose function. - subprocess.ts now registers via onExitSignal instead of direct process.on(SIGINT/SIGTERM) calls - resolveTokens registers a signal handler that captures tokens by closure, fixing the race condition where the exit handler would read the wrong token after disposal - Remove setupExitHandler and runCleanup — domain cleanup is handled by post.ts + await using Co-authored-by: Cursor * tweaks * simplify handler installation * extract to util * fix race in dispose * wrap dispose body in try/finally to ensure disposingRef always settles --------- Co-authored-by: Mateusz Burzyński Co-authored-by: Cursor Co-authored-by: pullfrog[bot] <226033991+pullfrog[bot]@users.noreply.github.com> --- entry | 206 ++++++++++++++++--------------------------- entry.ts | 3 - main.ts | 7 +- post | 60 ++++++------- test/run.ts | 7 +- test/utils.ts | 4 +- utils/exitHandler.ts | 128 +++++---------------------- utils/postCleanup.ts | 26 +++++- utils/subprocess.ts | 44 ++++----- utils/token.ts | 30 +++++-- 10 files changed, 198 insertions(+), 317 deletions(-) diff --git a/entry b/entry index 1492b53..b7243c6 100755 --- a/entry +++ b/entry @@ -148,11 +148,11 @@ var require_command = __commonJS({ }; Object.defineProperty(exports, "__esModule", { value: true }); exports.issue = exports.issueCommand = void 0; - var os = __importStar(__require("os")); + var os2 = __importStar(__require("os")); var utils_1 = require_utils(); function issueCommand(command, properties, message) { const cmd = new Command(command, properties, message); - process.stdout.write(cmd.toString() + os.EOL); + process.stdout.write(cmd.toString() + os2.EOL); } exports.issueCommand = issueCommand; function issue3(name, message = "") { @@ -236,7 +236,7 @@ var require_file_command = __commonJS({ exports.prepareKeyValueMessage = exports.issueFileCommand = void 0; var crypto2 = __importStar(__require("crypto")); var fs3 = __importStar(__require("fs")); - var os = __importStar(__require("os")); + var os2 = __importStar(__require("os")); var utils_1 = require_utils(); function issueFileCommand(command, message) { const filePath = process.env[`GITHUB_${command}`]; @@ -246,7 +246,7 @@ var require_file_command = __commonJS({ if (!fs3.existsSync(filePath)) { throw new Error(`Missing file at path: ${filePath}`); } - fs3.appendFileSync(filePath, `${(0, utils_1.toCommandValue)(message)}${os.EOL}`, { + fs3.appendFileSync(filePath, `${(0, utils_1.toCommandValue)(message)}${os2.EOL}`, { encoding: "utf8" }); } @@ -260,7 +260,7 @@ var require_file_command = __commonJS({ if (convertedValue.includes(delimiter)) { throw new Error(`Unexpected input: value should not contain the delimiter "${delimiter}"`); } - return `${key}<<${delimiter}${os.EOL}${convertedValue}${os.EOL}${delimiter}`; + return `${key}<<${delimiter}${os2.EOL}${convertedValue}${os2.EOL}${delimiter}`; } exports.prepareKeyValueMessage = prepareKeyValueMessage; } @@ -11280,7 +11280,7 @@ var require_RetryHandler = __commonJS({ return diff; } var RetryHandler = class _RetryHandler { - constructor(opts, handlers) { + constructor(opts, handlers2) { const { retryOptions, ...dispatchOpts } = opts; const { // Retry scoped @@ -11295,8 +11295,8 @@ var require_RetryHandler = __commonJS({ retryAfter, statusCodes } = retryOptions ?? {}; - this.dispatch = handlers.dispatch; - this.handler = handlers.handler; + this.dispatch = handlers2.dispatch; + this.handler = handlers2.handler; this.opts = dispatchOpts; this.abort = null; this.aborted = false; @@ -17513,7 +17513,7 @@ var require_lib = __commonJS({ } exports.isHttps = isHttps; var HttpClient = class { - constructor(userAgent2, handlers, requestOptions) { + constructor(userAgent2, handlers2, requestOptions) { this._ignoreSslError = false; this._allowRedirects = true; this._allowRedirectDowngrade = false; @@ -17523,7 +17523,7 @@ var require_lib = __commonJS({ this._keepAlive = false; this._disposed = false; this.userAgent = userAgent2; - this.handlers = handlers || []; + this.handlers = handlers2 || []; this.requestOptions = requestOptions; if (requestOptions) { if (requestOptions.ignoreSslError != null) { @@ -18983,7 +18983,7 @@ var require_toolrunner = __commonJS({ }; Object.defineProperty(exports, "__esModule", { value: true }); exports.argStringToArray = exports.ToolRunner = void 0; - var os = __importStar(__require("os")); + var os2 = __importStar(__require("os")); var events = __importStar(__require("events")); var child = __importStar(__require("child_process")); var path3 = __importStar(__require("path")); @@ -19038,12 +19038,12 @@ var require_toolrunner = __commonJS({ _processLineBuffer(data, strBuffer, onLine) { try { let s = strBuffer + data.toString(); - let n = s.indexOf(os.EOL); + let n = s.indexOf(os2.EOL); while (n > -1) { const line = s.substring(0, n); onLine(line); - s = s.substring(n + os.EOL.length); - n = s.indexOf(os.EOL); + s = s.substring(n + os2.EOL.length); + n = s.indexOf(os2.EOL); } return s; } catch (err) { @@ -19212,7 +19212,7 @@ var require_toolrunner = __commonJS({ } const optionsNonNull = this._cloneExecOptions(this.options); if (!optionsNonNull.silent && optionsNonNull.outStream) { - optionsNonNull.outStream.write(this._getCommandString(optionsNonNull) + os.EOL); + optionsNonNull.outStream.write(this._getCommandString(optionsNonNull) + os2.EOL); } const state = new ExecState(optionsNonNull, this.toolPath); state.on("debug", (message) => { @@ -19700,7 +19700,7 @@ var require_core = __commonJS({ var command_1 = require_command(); var file_command_1 = require_file_command(); var utils_1 = require_utils(); - var os = __importStar(__require("os")); + var os2 = __importStar(__require("os")); var path3 = __importStar(__require("path")); var oidc_utils_1 = require_oidc_utils(); var ExitCode; @@ -19768,7 +19768,7 @@ Support boolean input list: \`true | True | TRUE | false | False | FALSE\``); if (filePath) { return (0, file_command_1.issueFileCommand)("OUTPUT", (0, file_command_1.prepareKeyValueMessage)(name, value2)); } - process.stdout.write(os.EOL); + process.stdout.write(os2.EOL); (0, command_1.issueCommand)("set-output", { name }, (0, utils_1.toCommandValue)(value2)); } exports.setOutput = setOutput2; @@ -19802,7 +19802,7 @@ Support boolean input list: \`true | True | TRUE | false | False | FALSE\``); } exports.notice = notice; function info2(message) { - process.stdout.write(message + os.EOL); + process.stdout.write(message + os2.EOL); } exports.info = info2; function startGroup3(name) { @@ -135779,6 +135779,31 @@ import { join } from "node:path"; var core3 = __toESM(require_core(), 1); import assert2 from "node:assert/strict"; +// utils/exitHandler.ts +import os from "node:os"; +var handlers = /* @__PURE__ */ new Set(); +var installed = false; +function onExitSignal(handler2) { + installSignalHandlers(); + handlers.add(handler2); + return () => { + handlers.delete(handler2); + }; +} +function installSignalHandlers() { + if (installed) return; + installed = true; + async function handleSignal(signal) { + await Promise.allSettled([...handlers].map((h) => Promise.try(h, signal))); + exitWithSignal(signal); + } + process.on("SIGINT", handleSignal); + process.on("SIGTERM", handleSignal); +} +function exitWithSignal(signal) { + process.exit(128 + os.constants.signals[signal]); +} + // utils/github.ts var core2 = __toESM(require_core(), 1); import { createSign } from "node:crypto"; @@ -139728,18 +139753,31 @@ async function resolveTokens(params) { log.info("\xBB acquired full MCP token"); const revertGithubToken = setEnvironmentVariable("GITHUB_TOKEN", mcpToken); mcpTokenValue = mcpToken; - return { - gitToken, - mcpToken, - async [Symbol.asyncDispose]() { + let disposingRef; + const dispose = async () => { + if (disposingRef) { + return disposingRef.promise; + } + disposingRef = Promise.withResolvers(); + try { mcpTokenValue = void 0; revertGithubToken(); await Promise.all([ revokeGitHubInstallationToken(gitToken), revokeGitHubInstallationToken(mcpToken) ]); + } finally { + removeSignalHandler(); + disposingRef.resolve(); + disposingRef = void 0; } }; + const removeSignalHandler = onExitSignal(dispose); + return { + gitToken, + mcpToken, + [Symbol.asyncDispose]: dispose + }; } function getGitHubInstallationToken() { assert2(mcpTokenValue, "tokens not set. call resolveTokens first."); @@ -140647,13 +140685,13 @@ function createProcessOutputActivityTimeout(ctx) { var activeChildren = /* @__PURE__ */ new Map(); var externalSignalHandler = null; function trackChild(options) { + installSignalHandler(); activeChildren.set(options.child, options.killGroup ?? false); } function untrackChild(child) { activeChildren.delete(child); } function killTrackedChildren() { - const count = activeChildren.size; for (const entry of activeChildren) { const child = entry[0]; const killGroup = entry[1]; @@ -140666,34 +140704,27 @@ function killTrackedChildren() { } child.kill("SIGKILL"); } - return count; -} -function cleanupAndExit(signal) { - const count = killTrackedChildren(); - if (count > 0) { - log.info(`\xBB received ${signal}, killing ${count} subprocess(es)...`); - } - setTimeout(() => process.exit(1), 500).unref(); - process.exit(1); -} -function handleSignal(signal) { - if (externalSignalHandler) { - externalSignalHandler(signal); - return; - } - cleanupAndExit(signal); } var handlersInstalled = false; -function installSignalHandlers() { +function installSignalHandler() { if (handlersInstalled) return; handlersInstalled = true; - process.on("SIGINT", () => handleSignal("SIGINT")); - process.on("SIGTERM", () => handleSignal("SIGTERM")); + onExitSignal((signal) => { + if (externalSignalHandler) { + externalSignalHandler(signal); + return; + } + const count = activeChildren.size; + if (count > 0) { + log.info(`\xBB received ${signal}, killing ${count} subprocess(es)...`); + } + killTrackedChildren(); + }); } async function spawn2(options) { const { cmd, args: args2, env: env2, input, timeout, cwd, stdio, onStdout, onStderr } = options; const activityTimeoutMs = options.activityTimeout ?? DEFAULT_ACTIVITY_TIMEOUT_MS; - installSignalHandlers(); + installSignalHandler(); const startTime = performance3.now(); let stdoutBuffer = ""; let stderrBuffer = ""; @@ -141302,7 +141333,6 @@ function stripExistingFooter(body) { } // mcp/comment.ts -var LEAPING_INTO_ACTION_PREFIX = "Leaping into action"; async function buildCommentFooter({ agent: agent2, octokit, @@ -145378,10 +145408,10 @@ var cursorEffortModels = { max: "opus-4.5-thinking" }; async function installCursor() { - const os = process.platform === "darwin" ? "darwin" : "linux"; + const os2 = process.platform === "darwin" ? "darwin" : "linux"; const arch = process.arch === "arm64" ? "arm64" : "x64"; return await installFromDirectTarball({ - url: `https://downloads.cursor.com/lab/${CURSOR_CLI_VERSION}/${os}/${arch}/agent-cli-package.tar.gz`, + url: `https://downloads.cursor.com/lab/${CURSOR_CLI_VERSION}/${os2}/${arch}/agent-cli-package.tar.gz`, executablePath: "cursor-agent", stripComponents: 1 }); @@ -146495,83 +146525,6 @@ ${ctx.error}` : ctx.error; ctx.toolState.wasUpdated = true; } -// utils/exitHandler.ts -function buildErrorCommentBody(params) { - const workflowRunLink = params.runId ? `[workflow run logs](https://github.com/${params.owner}/${params.repo}/actions/runs/${params.runId})` : "workflow run logs"; - const errorMessage = params.isCancellation ? `This run was cancelled \u{1F6D1} - -The workflow was cancelled before completion. Please check the ${workflowRunLink} for details.` : `This run croaked \u{1F635} - -The workflow encountered an error before any progress could be reported. Please check the ${workflowRunLink} for details.`; - const footer = buildPullfrogFooter({ - triggeredBy: true, - workflowRun: params.runId ? { owner: params.owner, repo: params.repo, runId: params.runId } : void 0 - }); - return `${errorMessage}${footer}`; -} -var cleanupFn; -function setupExitHandler(toolState) { - let hasCleanedUp = false; - async function cleanup(isCancellation) { - if (hasCleanedUp) { - return; - } - hasCleanedUp = true; - const token = process.env.GITHUB_TOKEN; - const commentId = toolState.progressCommentId; - const wasUpdated = toolState.wasUpdated === true; - if (token && commentId && !wasUpdated) { - try { - const repoContext = parseRepoContext(); - const octokit = createOctokit(token); - const existingComment = await octokit.rest.issues.getComment({ - owner: repoContext.owner, - repo: repoContext.name, - comment_id: commentId - }); - const commentBody = existingComment.data.body || ""; - if (commentBody.startsWith(LEAPING_INTO_ACTION_PREFIX)) { - const runId = process.env.GITHUB_RUN_ID; - const body = buildErrorCommentBody({ - owner: repoContext.owner, - repo: repoContext.name, - runId, - isCancellation - }); - await octokit.rest.issues.updateComment({ - owner: repoContext.owner, - repo: repoContext.name, - comment_id: commentId, - body - }); - log.info("\xBB updated progress comment with error message"); - } - } catch { - } - } - if (token) { - try { - await revokeGitHubInstallationToken(token); - log.debug("\xBB installation token revoked"); - } catch { - } - } - } - cleanupFn = cleanup; - function handleSignal2() { - log.info("\xBB workflow cancelled, cleaning up..."); - cleanup(true).finally(() => process.exit(1)); - } - process.on("SIGINT", handleSignal2); - process.on("SIGTERM", handleSignal2); -} -async function runCleanup() { - try { - await cleanupFn?.(false); - } catch { - } -} - // utils/normalizeEnv.ts function maskValue(value2) { if (value2 && typeof value2 === "string" && value2.trim().length > 0) { @@ -146981,7 +146934,6 @@ async function main() { const toolState = initToolState({ progressCommentId: typeof resolvedPromptInput !== "string" ? resolvedPromptInput.progressCommentId : void 0 }); - setupExitHandler(toolState); resolveGit(); const jobToken = getJobToken(); const initialOctokit = createOctokit(jobToken); @@ -147141,9 +147093,7 @@ ${instructions.user}` : null, error: errorMessage }; } finally { - if (activityTimeout) { - activityTimeout.stop(); - } + activityTimeout?.stop(); } } catch (_2) { var _error2 = _2, _hasError2 = true; @@ -147166,8 +147116,6 @@ async function run() { } catch (error49) { const errorMessage = error49 instanceof Error ? error49.message : "Unknown error occurred"; core5.setFailed(`Action failed: ${errorMessage}`); - } finally { - await runCleanup(); } } await run(); diff --git a/entry.ts b/entry.ts index 32f13c1..ef89d70 100644 --- a/entry.ts +++ b/entry.ts @@ -6,7 +6,6 @@ import * as core from "@actions/core"; import { main } from "./main.ts"; -import { runCleanup } from "./utils/exitHandler.ts"; async function run(): Promise { try { @@ -22,8 +21,6 @@ async function run(): Promise { } catch (error) { const errorMessage = error instanceof Error ? error.message : "Unknown error occurred"; core.setFailed(`Action failed: ${errorMessage}`); - } finally { - await runCleanup(); } } diff --git a/main.ts b/main.ts index a3fb782..f06cacc 100644 --- a/main.ts +++ b/main.ts @@ -12,7 +12,6 @@ import { validateAgentApiKey } from "./utils/apiKeys.ts"; import { resolveBody } from "./utils/body.ts"; import { log, writeSummary } from "./utils/cli.ts"; import { reportErrorToComment } from "./utils/errorReport.ts"; -import { setupExitHandler } from "./utils/exitHandler.ts"; import { resolveGit } from "./utils/gitAuth.ts"; import { createOctokit } from "./utils/github.ts"; import { resolveInstructions } from "./utils/instructions.ts"; @@ -52,8 +51,6 @@ export async function main(): Promise { typeof resolvedPromptInput !== "string" ? resolvedPromptInput.progressCommentId : undefined, }); - setupExitHandler(toolState); - // resolve and fingerprint git binary before any agent code runs resolveGit(); @@ -247,8 +244,6 @@ export async function main(): Promise { error: errorMessage, }; } finally { - if (activityTimeout) { - activityTimeout.stop(); - } + activityTimeout?.stop(); } } diff --git a/post b/post index 7eb1b82..08774aa 100755 --- a/post +++ b/post @@ -41198,37 +41198,8 @@ var ReplyToReviewComment = type({ ) }); -// utils/token.ts -var core3 = __toESM(require_core(), 1); -function getJobToken() { - const inputToken = core3.getInput("token"); - if (inputToken) { - return inputToken; - } - const fallbackToken = process.env.GH_TOKEN || process.env.GITHUB_TOKEN; - if (fallbackToken) { - return fallbackToken; - } - throw new Error("token input is required"); -} - -// utils/exitHandler.ts -function buildErrorCommentBody(params) { - const workflowRunLink = params.runId ? `[workflow run logs](https://github.com/${params.owner}/${params.repo}/actions/runs/${params.runId})` : "workflow run logs"; - const errorMessage = params.isCancellation ? `This run was cancelled \u{1F6D1} - -The workflow was cancelled before completion. Please check the ${workflowRunLink} for details.` : `This run croaked \u{1F635} - -The workflow encountered an error before any progress could be reported. Please check the ${workflowRunLink} for details.`; - const footer = buildPullfrogFooter({ - triggeredBy: true, - workflowRun: params.runId ? { owner: params.owner, repo: params.repo, runId: params.runId } : void 0 - }); - return `${errorMessage}${footer}`; -} - // utils/payload.ts -var core4 = __toESM(require_core(), 1); +var core3 = __toESM(require_core(), 1); // external.ts var agentsManifest = { @@ -41399,7 +41370,7 @@ var Inputs = type({ "cwd?": type.string.or("undefined") }); function resolvePromptInput() { - const prompt = core4.getInput("prompt", { required: true }); + const prompt = core3.getInput("prompt", { required: true }); let parsed2; try { parsed2 = JSON.parse(prompt); @@ -41414,8 +41385,35 @@ function resolvePromptInput() { return jsonPayload; } +// utils/token.ts +var core4 = __toESM(require_core(), 1); +function getJobToken() { + const inputToken = core4.getInput("token"); + if (inputToken) { + return inputToken; + } + const fallbackToken = process.env.GH_TOKEN || process.env.GITHUB_TOKEN; + if (fallbackToken) { + return fallbackToken; + } + throw new Error("token input is required"); +} + // utils/postCleanup.ts var SHOULD_CHECK_REASON = true; +function buildErrorCommentBody(params) { + const workflowRunLink = params.runId ? `[workflow run logs](https://github.com/${params.owner}/${params.repo}/actions/runs/${params.runId})` : "workflow run logs"; + const errorMessage = params.isCancellation ? `This run was cancelled \u{1F6D1} + +The workflow was cancelled before completion. Please check the ${workflowRunLink} for details.` : `This run croaked \u{1F635} + +The workflow encountered an error before any progress could be reported. Please check the ${workflowRunLink} for details.`; + const footer = buildPullfrogFooter({ + triggeredBy: true, + workflowRun: params.runId ? { owner: params.owner, repo: params.repo, runId: params.runId } : void 0 + }); + return `${errorMessage}${footer}`; +} async function validateStuckProgressComment(promptInput, octokit, owner, repo) { if (!promptInput?.progressCommentId) { log.info("[post] no progressCommentId in prompt input, skipping cleanup"); diff --git a/test/run.ts b/test/run.ts index 5834eac..4a9e307 100644 --- a/test/run.ts +++ b/test/run.ts @@ -5,11 +5,7 @@ import { config } from "dotenv"; import { runInDocker } from "../utils/docker.ts"; import { ensureGitHubToken } from "../utils/github.ts"; import { isInsideDocker } from "../utils/globals.ts"; -import { - installSignalHandlers, - killTrackedChildren, - setSignalHandler, -} from "../utils/subprocess.ts"; +import { killTrackedChildren, setSignalHandler } from "../utils/subprocess.ts"; import { type AgentResult, agents, @@ -482,7 +478,6 @@ async function main(): Promise { } setSignalHandler(handleCancel); - installSignalHandlers(); // run tests with limited concurrency to avoid overwhelming agent APIs const maxConcurrency = 5; diff --git a/test/utils.ts b/test/utils.ts index 862c7b5..8b8e3a5 100644 --- a/test/utils.ts +++ b/test/utils.ts @@ -5,7 +5,7 @@ import { dirname, join } from "node:path"; import { fileURLToPath } from "node:url"; import { agentsManifest } from "../external.ts"; import type { Inputs } from "../main.ts"; -import { installSignalHandlers, trackChild, untrackChild } from "../utils/subprocess.ts"; +import { trackChild, untrackChild } from "../utils/subprocess.ts"; const __dirname = dirname(fileURLToPath(import.meta.url)); @@ -165,8 +165,6 @@ const DEFAULT_TEST_TIMEOUT = "10m"; // run agent and stream output with prefix labels // note: activity timeout is enforced in action main and subprocess utils export async function runAgentStreaming(options: RunStreamingOptions): Promise { - installSignalHandlers(); - return new Promise((resolve) => { const chunks: Buffer[] = []; const prefix = getPrefix({ test: options.test, agent: options.agent }); diff --git a/utils/exitHandler.ts b/utils/exitHandler.ts index 8cdbd41..c32b6d0 100644 --- a/utils/exitHandler.ts +++ b/utils/exitHandler.ts @@ -1,119 +1,35 @@ -import { LEAPING_INTO_ACTION_PREFIX } from "../mcp/comment.ts"; -import type { ToolState } from "../mcp/server.ts"; -import { buildPullfrogFooter } from "./buildPullfrogFooter.ts"; -import { log } from "./cli.ts"; -import { createOctokit, parseRepoContext } from "./github.ts"; -import { revokeGitHubInstallationToken } from "./token.ts"; +import os from "node:os"; + +type ExitSignalHandler = (signal: "SIGINT" | "SIGTERM") => void | Promise; + +const handlers = new Set(); +let installed = false; /** - * Build error comment body with error message and footer + * Register a handler to run when the process receives SIGINT or SIGTERM. + * Returns a dispose function that removes the handler. */ -export function buildErrorCommentBody(params: { - owner: string; - repo: string; - runId: string | undefined; - isCancellation: boolean; -}): string { - const workflowRunLink = params.runId - ? `[workflow run logs](https://github.com/${params.owner}/${params.repo}/actions/runs/${params.runId})` - : "workflow run logs"; - const errorMessage = params.isCancellation - ? `This run was cancelled 🛑\n\nThe workflow was cancelled before completion. Please check the ${workflowRunLink} for details.` - : `This run croaked 😵\n\nThe workflow encountered an error before any progress could be reported. Please check the ${workflowRunLink} for details.`; - const footer = buildPullfrogFooter({ - triggeredBy: true, - workflowRun: params.runId - ? { owner: params.owner, repo: params.repo, runId: params.runId } - : undefined, - }); - return `${errorMessage}${footer}`; +export function onExitSignal(handler: ExitSignalHandler): () => void { + installSignalHandlers(); + handlers.add(handler); + return () => { + handlers.delete(handler); + }; } -let cleanupFn: ((isCancellation: boolean) => Promise) | undefined; +function installSignalHandlers(): void { + if (installed) return; + installed = true; -export function setupExitHandler(toolState: ToolState): void { - let hasCleanedUp = false; - - async function cleanup(isCancellation: boolean): Promise { - if (hasCleanedUp) { - return; - } - hasCleanedUp = true; - - const token = process.env.GITHUB_TOKEN; - const commentId = toolState.progressCommentId; - const wasUpdated = toolState.wasUpdated === true; - - // update progress comment if it was never updated (still shows "leaping into action") - if (token && commentId && !wasUpdated) { - try { - const repoContext = parseRepoContext(); - const octokit = createOctokit(token); - - const existingComment = await octokit.rest.issues.getComment({ - owner: repoContext.owner, - repo: repoContext.name, - comment_id: commentId, - }); - - const commentBody = existingComment.data.body || ""; - - // only update if comment still shows the initial "leaping into action" message - if (commentBody.startsWith(LEAPING_INTO_ACTION_PREFIX)) { - const runId = process.env.GITHUB_RUN_ID; - - const body = buildErrorCommentBody({ - owner: repoContext.owner, - repo: repoContext.name, - runId, - isCancellation, - }); - - await octokit.rest.issues.updateComment({ - owner: repoContext.owner, - repo: repoContext.name, - comment_id: commentId, - body, - }); - - log.info("» updated progress comment with error message"); - } - } catch { - // ignore errors during cleanup - } - } - - // revoke token - if (token) { - try { - await revokeGitHubInstallationToken(token); - log.debug("» installation token revoked"); - } catch { - // ignore errors during cleanup - } - } - } - - // store cleanup function for runCleanup() - cleanupFn = cleanup; - - // handle cancellation signals - function handleSignal(): void { - log.info("» workflow cancelled, cleaning up..."); - cleanup(true).finally(() => process.exit(1)); + async function handleSignal(signal: "SIGINT" | "SIGTERM") { + await Promise.allSettled([...handlers].map((h) => Promise.try(h, signal))); + exitWithSignal(signal); } process.on("SIGINT", handleSignal); process.on("SIGTERM", handleSignal); } -/** - * Run cleanup explicitly. Called from entry.ts in finally block. - */ -export async function runCleanup(): Promise { - try { - await cleanupFn?.(false); - } catch { - // ignore errors during cleanup - } +export function exitWithSignal(signal: "SIGINT" | "SIGTERM") { + process.exit(128 + os.constants.signals[signal]); } diff --git a/utils/postCleanup.ts b/utils/postCleanup.ts index df4756b..f9a9e79 100644 --- a/utils/postCleanup.ts +++ b/utils/postCleanup.ts @@ -1,6 +1,6 @@ import { LEAPING_INTO_ACTION_PREFIX } from "../mcp/comment.ts"; +import { buildPullfrogFooter } from "./buildPullfrogFooter.ts"; import { log } from "./cli.ts"; -import { buildErrorCommentBody } from "./exitHandler.ts"; import { createOctokit, parseRepoContext } from "./github.ts"; import { type ResolvedPromptInput, resolvePromptInput } from "./payload.ts"; import { getJobToken } from "./token.ts"; @@ -14,6 +14,30 @@ type JsonPromptInput = Extract; // not string * */ const SHOULD_CHECK_REASON = true; +/** + * Build error comment body with error message and footer + */ +function buildErrorCommentBody(params: { + owner: string; + repo: string; + runId: string | undefined; + isCancellation: boolean; +}): string { + const workflowRunLink = params.runId + ? `[workflow run logs](https://github.com/${params.owner}/${params.repo}/actions/runs/${params.runId})` + : "workflow run logs"; + const errorMessage = params.isCancellation + ? `This run was cancelled 🛑\n\nThe workflow was cancelled before completion. Please check the ${workflowRunLink} for details.` + : `This run croaked 😵\n\nThe workflow encountered an error before any progress could be reported. Please check the ${workflowRunLink} for details.`; + const footer = buildPullfrogFooter({ + triggeredBy: true, + workflowRun: params.runId + ? { owner: params.owner, repo: params.repo, runId: params.runId } + : undefined, + }); + return `${errorMessage}${footer}`; +} + /** * Validate that the progress comment is stuck on "Leaping into action" * Fetches the comment by ID and checks if it starts with LEAPING_INTO_ACTION_PREFIX diff --git a/utils/subprocess.ts b/utils/subprocess.ts index 02e3bd0..fc0d0a4 100644 --- a/utils/subprocess.ts +++ b/utils/subprocess.ts @@ -2,6 +2,7 @@ import { type ChildProcess, spawn as nodeSpawn } from "node:child_process"; import { performance } from "node:perf_hooks"; import { DEFAULT_ACTIVITY_CHECK_INTERVAL_MS, DEFAULT_ACTIVITY_TIMEOUT_MS } from "./activity.ts"; import { log } from "./cli.ts"; +import { onExitSignal } from "./exitHandler.ts"; export type TrackChildOptions = { child: ChildProcess; @@ -18,6 +19,9 @@ let externalSignalHandler: SignalHandler | null = null; // track a child process for cleanup on Ctrl+C export function trackChild(options: TrackChildOptions): void { + // the signal handler cleans up all tracked children + // so we only have to install it once some child gets tracked + installSignalHandler(); activeChildren.set(options.child, options.killGroup ?? false); } @@ -32,8 +36,7 @@ export function setSignalHandler(handler: SignalHandler | null): void { } // kill all tracked children without exiting -export function killTrackedChildren(): number { - const count = activeChildren.size; +export function killTrackedChildren() { for (const entry of activeChildren) { const child = entry[0]; const killGroup = entry[1]; @@ -47,35 +50,24 @@ export function killTrackedChildren(): number { } child.kill("SIGKILL"); } - return count; -} - -// cleanup handler for SIGINT/SIGTERM - kills all tracked children -function cleanupAndExit(signal: string): void { - const count = killTrackedChildren(); - if (count > 0) { - log.info(`» received ${signal}, killing ${count} subprocess(es)...`); - } - // force exit after a short delay if process is stuck - setTimeout(() => process.exit(1), 500).unref(); - process.exit(1); -} - -function handleSignal(signal: NodeJS.Signals): void { - if (externalSignalHandler) { - externalSignalHandler(signal); - return; - } - cleanupAndExit(signal); } // install signal handlers once (call early in process lifecycle) let handlersInstalled = false; -export function installSignalHandlers(): void { +function installSignalHandler(): void { if (handlersInstalled) return; handlersInstalled = true; - process.on("SIGINT", () => handleSignal("SIGINT")); - process.on("SIGTERM", () => handleSignal("SIGTERM")); + onExitSignal((signal) => { + if (externalSignalHandler) { + externalSignalHandler(signal); + return; + } + const count = activeChildren.size; + if (count > 0) { + log.info(`» received ${signal}, killing ${count} subprocess(es)...`); + } + killTrackedChildren(); + }); } export interface SpawnOptions { @@ -106,7 +98,7 @@ export async function spawn(options: SpawnOptions): Promise { const { cmd, args, env, input, timeout, cwd, stdio, onStdout, onStderr } = options; const activityTimeoutMs = options.activityTimeout ?? DEFAULT_ACTIVITY_TIMEOUT_MS; - installSignalHandlers(); + installSignalHandler(); const startTime = performance.now(); let stdoutBuffer = ""; diff --git a/utils/token.ts b/utils/token.ts index fe17bba..62bd972 100644 --- a/utils/token.ts +++ b/utils/token.ts @@ -2,6 +2,7 @@ import assert from "node:assert/strict"; import * as core from "@actions/core"; import type { PushPermission } from "../external.ts"; import { log } from "./cli.ts"; +import { onExitSignal } from "./exitHandler.ts"; import { acquireNewToken } from "./github.ts"; import { isGitHubActions } from "./globals.ts"; @@ -131,18 +132,35 @@ export async function resolveTokens(params: ResolveTokensParams): Promise | undefined; + + const dispose = async () => { + if (disposingRef) { + // this can happen if the signal arrives when disposing tokens + // we make sure to wait for the current dispose to complete + return disposingRef.promise; + } + disposingRef = Promise.withResolvers(); + try { mcpTokenValue = undefined; revertGithubToken(); - // revoke both tokens await Promise.all([ revokeGitHubInstallationToken(gitToken), revokeGitHubInstallationToken(mcpToken), ]); - }, + } finally { + removeSignalHandler(); + disposingRef.resolve(); + disposingRef = undefined; + } + }; + + const removeSignalHandler = onExitSignal(dispose); + + return { + gitToken, + mcpToken, + [Symbol.asyncDispose]: dispose, }; }