refactor: replace narrow parameter types with context objects (#519)

* refactor: replace narrow parameter types with context objects across action/

pass broader context objects (ToolContext, PromptContext, PostCleanupContext) to
utility functions instead of cherry-picking fields into single-use interfaces.
deletes 8 narrow types, simplifies call sites, and makes buildCommentFooter
synchronous by reading ctx.runId/ctx.jobId directly instead of re-deriving
from env vars and making an extra API call.

Made-with: Cursor

* fix: replace non-null assertion with local guard in validatePushDestination

addresses review feedback — the function now validates pushUrl itself instead
of relying on the caller's check, eliminating the ! assertion.

Made-with: Cursor

* revert: remove GH_TOKEN injection from restricted shell

the original change exposed the git token in restricted-mode shell so
`gh` CLI would work. this is a security regression for public repos: MCP
tools are deliberately constrained (no merge, no release, no arbitrary
API calls), but `gh api` with the token gives full GitHub API access to
any prompt-injected agent.

Made-with: Cursor
This commit is contained in:
Colin McDonnell
2026-04-04 20:51:49 +00:00
committed by pullfrog[bot]
parent ab76a4ad04
commit 2ea447a780
9 changed files with 5652 additions and 5829 deletions
+5445 -5503
View File
File diff suppressed because it is too large Load Diff
+12 -8
View File
@@ -1,7 +1,8 @@
import { Octokit } from "@octokit/rest"; import type { RestEndpointMethodTypes } from "@octokit/rest";
import { describe, expect, it } from "vitest"; import { describe, expect, it } from "vitest";
import { acquireNewToken } from "../utils/github.ts"; import { acquireNewToken, createOctokit } from "../utils/github.ts";
import { fetchAndFormatPrDiff } from "./checkout.ts"; import { fetchAndFormatPrDiff } from "./checkout.ts";
import type { ToolContext } from "./server.ts";
/** /**
* parses TOC entries like "- src/math.ts → lines 7-42" into structured data. * parses TOC entries like "- src/math.ts → lines 7-42" into structured data.
@@ -33,13 +34,16 @@ describe("fetchAndFormatPrDiff", () => {
{ timeout: 30000 }, { timeout: 30000 },
async () => { async () => {
const token = await getToken(); const token = await getToken();
const octokit = new Octokit({ auth: token }); const octokit = createOctokit(token);
const result = await fetchAndFormatPrDiff({ const ctx = {
octokit, octokit,
owner: "pullfrog", repo: {
repo: "test-repo", owner: "pullfrog",
pullNumber: 1, name: "test-repo",
}); data: {} as RestEndpointMethodTypes["repos"]["get"]["response"]["data"],
},
} as ToolContext;
const result = await fetchAndFormatPrDiff(ctx, 1);
// verify content includes TOC at the start // verify content includes TOC at the start
expect(result.content.startsWith(result.toc)).toBe(true); expect(result.content.startsWith(result.toc)).toBe(true);
+9 -18
View File
@@ -145,22 +145,18 @@ export type CheckoutPrResult = {
instructions: string; instructions: string;
}; };
type FetchPrDiffParams = {
octokit: Octokit;
owner: string;
repo: string;
pullNumber: number;
};
/** /**
* fetches PR files from GitHub and formats them with line numbers and TOC. * fetches PR files from GitHub and formats them with line numbers and TOC.
* this is the core diff formatting logic, extracted for testability. * this is the core diff formatting logic, extracted for testability.
*/ */
export async function fetchAndFormatPrDiff(params: FetchPrDiffParams): Promise<FormatFilesResult> { export async function fetchAndFormatPrDiff(
const files = await params.octokit.paginate(params.octokit.rest.pulls.listFiles, { ctx: ToolContext,
owner: params.owner, pullNumber: number
repo: params.repo, ): Promise<FormatFilesResult> {
pull_number: params.pullNumber, const files = await ctx.octokit.paginate(ctx.octokit.rest.pulls.listFiles, {
owner: ctx.repo.owner,
repo: ctx.repo.name,
pull_number: pullNumber,
per_page: 100, per_page: 100,
}); });
return formatFilesWithLineNumbers(files); return formatFilesWithLineNumbers(files);
@@ -502,12 +498,7 @@ export function CheckoutPrTool(ctx: ToolContext) {
} }
// fetch PR files and format with line numbers // fetch PR files and format with line numbers
const formatResult = await fetchAndFormatPrDiff({ const formatResult = await fetchAndFormatPrDiff(ctx, pull_number);
octokit: ctx.octokit,
owner: ctx.repo.owner,
repo: ctx.repo.name,
pullNumber: pull_number,
});
const diffPreview = formatResult.content.split("\n").slice(0, 100).join("\n"); const diffPreview = formatResult.content.split("\n").slice(0, 100).join("\n");
log.debug(`formatted diff preview (first 100 lines):\n${diffPreview}`); log.debug(`formatted diff preview (first 100 lines):\n${diffPreview}`);
const diffPath = join(tempDir, `pr-${pull_number}-${headShort}.diff`); const diffPath = join(tempDir, `pr-${pull_number}-${headShort}.diff`);
+27 -70
View File
@@ -4,7 +4,6 @@ import { getApiUrl } from "../utils/apiUrl.ts";
import { buildPullfrogFooter, stripExistingFooter } from "../utils/buildPullfrogFooter.ts"; import { buildPullfrogFooter, stripExistingFooter } from "../utils/buildPullfrogFooter.ts";
import { log } from "../utils/cli.ts"; import { log } from "../utils/cli.ts";
import { fixDoubleEscapedString } from "../utils/fixDoubleEscapedString.ts"; import { fixDoubleEscapedString } from "../utils/fixDoubleEscapedString.ts";
import { type OctokitWithPlugins, parseRepoContext } from "../utils/github.ts";
import { retry } from "../utils/retry.ts"; import { retry } from "../utils/retry.ts";
import type { ToolContext } from "./server.ts"; import type { ToolContext } from "./server.ts";
import { execute, tool } from "./shared.ts"; import { execute, tool } from "./shared.ts";
@@ -52,65 +51,37 @@ export async function updateCommentNodeId(
*/ */
export const LEAPING_INTO_ACTION_PREFIX = "Leaping into action"; export const LEAPING_INTO_ACTION_PREFIX = "Leaping into action";
interface BuildCommentFooterParams { function buildCommentFooter(ctx: ToolContext, customParts?: string[]): string {
octokit?: OctokitWithPlugins | undefined; const runId = ctx.runId;
customParts?: string[] | undefined;
model?: string | undefined;
}
async function buildCommentFooter(params: BuildCommentFooterParams): Promise<string> {
const repoContext = parseRepoContext();
const runId = process.env.GITHUB_RUN_ID
? Number.parseInt(process.env.GITHUB_RUN_ID, 10)
: undefined;
let jobId: string | undefined;
if (runId && params.octokit) {
try {
const { data: jobs } = await params.octokit.rest.actions.listJobsForWorkflowRun({
owner: repoContext.owner,
repo: repoContext.name,
run_id: runId,
});
jobId = jobs.jobs[0]?.id.toString();
} catch {
// fall back to computed URL from runId alone
}
}
return buildPullfrogFooter({ return buildPullfrogFooter({
triggeredBy: true, triggeredBy: true,
workflowRun: runId workflowRun:
? { owner: repoContext.owner, repo: repoContext.name, runId, jobId } runId !== undefined
: undefined, ? {
customParts: params.customParts, owner: ctx.repo.owner,
model: params.model, repo: ctx.repo.name,
runId,
jobId: ctx.jobId,
}
: undefined,
customParts,
model: ctx.toolState.model,
}); });
} }
function buildImplementPlanLink( function buildImplementPlanLink(ctx: ToolContext, issueNumber: number, commentId: number): string {
owner: string,
repo: string,
issueNumber: number,
commentId: number
): string {
const apiUrl = getApiUrl(); const apiUrl = getApiUrl();
return `[Implement plan ➔](${apiUrl}/trigger/${owner}/${repo}/${issueNumber}?action=implement&comment_id=${commentId})`; return `[Implement plan ➔](${apiUrl}/trigger/${ctx.repo.owner}/${ctx.repo.name}/${issueNumber}?action=implement&comment_id=${commentId})`;
} }
export interface AddFooterCtx { export function addFooter(ctx: ToolContext, body: string): string {
octokit?: OctokitWithPlugins | undefined;
toolState?: { model?: string | undefined } | undefined;
}
export async function addFooter(ctx: AddFooterCtx, body: string): Promise<string> {
if (/<br\s*\/?>[ \t]*\n(?!\s*\n)/i.test(body)) { if (/<br\s*\/?>[ \t]*\n(?!\s*\n)/i.test(body)) {
throw new Error( throw new Error(
"body contains <br/> followed by a non-blank line, which breaks GitHub markdown rendering. always add a blank line after <br/> tags." "body contains <br/> followed by a non-blank line, which breaks GitHub markdown rendering. always add a blank line after <br/> tags."
); );
} }
const bodyWithoutFooter = stripExistingFooter(fixDoubleEscapedString(body)); const bodyWithoutFooter = stripExistingFooter(fixDoubleEscapedString(body));
const footer = await buildCommentFooter({ octokit: ctx.octokit, model: ctx.toolState?.model }); const footer = buildCommentFooter(ctx);
return `${bodyWithoutFooter}${footer}`; return `${bodyWithoutFooter}${footer}`;
} }
@@ -132,7 +103,7 @@ export function CreateCommentTool(ctx: ToolContext) {
"Create a comment on a GitHub issue or PR. For progress/plan updates on the current run use report_progress instead. Use type: 'Plan' for plan comments, type: 'Summary' for PR summary comments.", "Create a comment on a GitHub issue or PR. For progress/plan updates on the current run use report_progress instead. Use type: 'Plan' for plan comments, type: 'Summary' for PR summary comments.",
parameters: Comment, parameters: Comment,
execute: execute(async ({ issueNumber, body, type: commentType }) => { execute: execute(async ({ issueNumber, body, type: commentType }) => {
const bodyWithFooter = await addFooter(ctx, body); const bodyWithFooter = addFooter(ctx, body);
// if a summary comment already exists (found by select_mode), update instead of creating // if a summary comment already exists (found by select_mode), update instead of creating
if (commentType === "Summary" && ctx.toolState.existingSummaryCommentId) { if (commentType === "Summary" && ctx.toolState.existingSummaryCommentId) {
@@ -193,7 +164,7 @@ export function EditCommentTool(ctx: ToolContext) {
description: "Edit a GitHub issue comment by its ID", description: "Edit a GitHub issue comment by its ID",
parameters: EditComment, parameters: EditComment,
execute: execute(async ({ commentId, body }) => { execute: execute(async ({ commentId, body }) => {
const bodyWithFooter = await addFooter(ctx, body); const bodyWithFooter = addFooter(ctx, body);
const result = await ctx.octokit.rest.issues.updateComment({ const result = await ctx.octokit.rest.issues.updateComment({
owner: ctx.repo.owner, owner: ctx.repo.owner,
@@ -260,14 +231,10 @@ export async function reportProgress(
const commentId = ctx.toolState.existingPlanCommentId; const commentId = ctx.toolState.existingPlanCommentId;
const customParts = const customParts =
isPlanMode && issueNumber !== undefined isPlanMode && issueNumber !== undefined
? [buildImplementPlanLink(ctx.repo.owner, ctx.repo.name, issueNumber, commentId)] ? [buildImplementPlanLink(ctx, issueNumber, commentId)]
: undefined; : undefined;
const bodyWithoutFooter = stripExistingFooter(body); const bodyWithoutFooter = stripExistingFooter(body);
const footer = await buildCommentFooter({ const footer = buildCommentFooter(ctx, customParts);
octokit: ctx.octokit,
customParts,
model: ctx.toolState.model,
});
const bodyWithFooter = `${bodyWithoutFooter}${footer}`; const bodyWithFooter = `${bodyWithoutFooter}${footer}`;
const result = await ctx.octokit.rest.issues.updateComment({ const result = await ctx.octokit.rest.issues.updateComment({
@@ -297,15 +264,11 @@ export async function reportProgress(
if (existingCommentId) { if (existingCommentId) {
const customParts = const customParts =
isPlanMode && issueNumber !== undefined isPlanMode && issueNumber !== undefined
? [buildImplementPlanLink(ctx.repo.owner, ctx.repo.name, issueNumber, existingCommentId)] ? [buildImplementPlanLink(ctx, issueNumber, existingCommentId)]
: undefined; : undefined;
const bodyWithoutFooter = stripExistingFooter(body); const bodyWithoutFooter = stripExistingFooter(body);
const footer = await buildCommentFooter({ const footer = buildCommentFooter(ctx, customParts);
octokit: ctx.octokit,
customParts,
model: ctx.toolState.model,
});
const bodyWithFooter = `${bodyWithoutFooter}${footer}`; const bodyWithFooter = `${bodyWithoutFooter}${footer}`;
const result = await ctx.octokit.rest.issues.updateComment({ const result = await ctx.octokit.rest.issues.updateComment({
@@ -343,7 +306,7 @@ export async function reportProgress(
} }
// for new comments, we need to create first, then update with Plan link if in Plan mode // for new comments, we need to create first, then update with Plan link if in Plan mode
const initialBody = await addFooter(ctx, body); const initialBody = addFooter(ctx, body);
const result = await ctx.octokit.rest.issues.createComment({ const result = await ctx.octokit.rest.issues.createComment({
owner: ctx.repo.owner, owner: ctx.repo.owner,
@@ -358,15 +321,9 @@ export async function reportProgress(
// if Plan mode, update the comment to add the "Implement plan" link // if Plan mode, update the comment to add the "Implement plan" link
if (isPlanMode) { if (isPlanMode) {
const customParts = [ const customParts = [buildImplementPlanLink(ctx, issueNumber, result.data.id)];
buildImplementPlanLink(ctx.repo.owner, ctx.repo.name, issueNumber, result.data.id),
];
const bodyWithoutFooter = stripExistingFooter(body); const bodyWithoutFooter = stripExistingFooter(body);
const footer = await buildCommentFooter({ const footer = buildCommentFooter(ctx, customParts);
octokit: ctx.octokit,
customParts,
model: ctx.toolState.model,
});
const bodyWithPlanLink = `${bodyWithoutFooter}${footer}`; const bodyWithPlanLink = `${bodyWithoutFooter}${footer}`;
const updateResult = await ctx.octokit.rest.issues.updateComment({ const updateResult = await ctx.octokit.rest.issues.updateComment({
@@ -490,7 +447,7 @@ export function ReplyToReviewCommentTool(ctx: ToolContext) {
"Reply to a PR review comment thread (NOT issue comments — this only works for inline review comments on PR diffs). Call this for EACH comment you address in AddressReviews mode. Keep replies extremely brief (1 sentence max).", "Reply to a PR review comment thread (NOT issue comments — this only works for inline review comments on PR diffs). Call this for EACH comment you address in AddressReviews mode. Keep replies extremely brief (1 sentence max).",
parameters: ReplyToReviewComment, parameters: ReplyToReviewComment,
execute: execute(async ({ pull_number, comment_id, body }) => { execute: execute(async ({ pull_number, comment_id, body }) => {
const bodyWithFooter = await addFooter(ctx, body); const bodyWithFooter = addFooter(ctx, body);
const result = await ctx.octokit.rest.pulls.createReplyForReviewComment({ const result = await ctx.octokit.rest.pulls.createReplyForReviewComment({
owner: ctx.repo.owner, owner: ctx.repo.owner,
+8 -19
View File
@@ -57,23 +57,20 @@ function normalizeUrl(url: string): string {
return url.replace(/\.git$/, "").toLowerCase(); return url.replace(/\.git$/, "").toLowerCase();
} }
type ValidatePushParams = {
branch: string;
pushUrl: string;
storedDest: StoredPushDest | undefined;
};
/** /**
* validate that the push destination matches expected URL. * validate that the push destination matches expected URL.
* pushUrl is set by setupGit (base repo) and updated by checkout_pr (fork repo). * pushUrl is set by setupGit (base repo) and updated by checkout_pr (fork repo).
*/ */
function validatePushDestination(params: ValidatePushParams): PushDestination { function validatePushDestination(ctx: ToolContext, branch: string): PushDestination {
const dest = getPushDestination(params.branch, params.storedDest); const pushUrl = ctx.toolState.pushUrl;
if (!pushUrl) throw new Error("pushUrl not set - setupGit must run before push_branch");
if (normalizeUrl(dest.url) !== normalizeUrl(params.pushUrl)) { const dest = getPushDestination(branch, ctx.toolState.pushDest);
if (normalizeUrl(dest.url) !== normalizeUrl(pushUrl)) {
throw new Error( throw new Error(
`Push blocked: destination does not match expected repository.\n` + `Push blocked: destination does not match expected repository.\n` +
`Expected: ${params.pushUrl}\n` + `Expected: ${pushUrl}\n` +
`Actual: ${dest.url}\n` + `Actual: ${dest.url}\n` +
`Git configuration may have been tampered with.` `Git configuration may have been tampered with.`
); );
@@ -119,15 +116,7 @@ export function PushBranchTool(ctx: ToolContext) {
} }
// validate push destination matches expected URL // validate push destination matches expected URL
const pushUrl = ctx.toolState.pushUrl; const pushDest = validatePushDestination(ctx, branch);
if (!pushUrl) {
throw new Error("pushUrl not set - setupGit must run before push_branch");
}
const pushDest = validatePushDestination({
branch,
pushUrl,
storedDest: ctx.toolState.pushDest,
});
// block pushes to default branch in restricted mode // block pushes to default branch in restricted mode
if (pushPermission === "restricted" && pushDest.remoteBranch === defaultBranch) { if (pushPermission === "restricted" && pushDest.remoteBranch === defaultBranch) {
+10 -19
View File
@@ -65,15 +65,14 @@ const modeInstructionParent: Record<string, string> = {
Fix: "Build", Fix: "Build",
}; };
type BuildGuidanceOpts = { function buildOrchestratorGuidance(
modeInstructions?: Record<string, string>; ctx: ToolContext,
overrideGuidance?: string; mode: Mode,
}; overrideGuidance?: string
): OrchestratorGuidance {
function buildOrchestratorGuidance(mode: Mode, opts: BuildGuidanceOpts = {}): OrchestratorGuidance { const hardcoded = overrideGuidance ?? mode.prompt ?? "";
const hardcoded = opts.overrideGuidance ?? mode.prompt ?? "";
const lookupKey = modeInstructionParent[mode.name] ?? mode.name; const lookupKey = modeInstructionParent[mode.name] ?? mode.name;
const userInstructions = opts.modeInstructions?.[lookupKey] ?? ""; const userInstructions = ctx.modeInstructions[lookupKey] ?? "";
const guidance = [hardcoded, userInstructions].filter(Boolean).join("\n\n"); const guidance = [hardcoded, userInstructions].filter(Boolean).join("\n\n");
return { return {
modeName: mode.name, modeName: mode.name,
@@ -172,8 +171,6 @@ export function SelectModeTool(ctx: ToolContext) {
ctx.toolState.selectedMode = selectedMode.name; ctx.toolState.selectedMode = selectedMode.name;
const guidanceOpts: BuildGuidanceOpts = { modeInstructions: ctx.modeInstructions };
if (selectedMode.name === "Plan") { if (selectedMode.name === "Plan") {
const issueNumber = params.issue_number ?? ctx.payload.event.issue_number; const issueNumber = params.issue_number ?? ctx.payload.event.issue_number;
if (issueNumber !== undefined) { if (issueNumber !== undefined) {
@@ -182,10 +179,7 @@ export function SelectModeTool(ctx: ToolContext) {
ctx.toolState.existingPlanCommentId = existing.commentId; ctx.toolState.existingPlanCommentId = existing.commentId;
ctx.toolState.previousPlanBody = existing.body; ctx.toolState.previousPlanBody = existing.body;
return { return {
...buildOrchestratorGuidance(selectedMode, { ...buildOrchestratorGuidance(ctx, selectedMode, overrides.PlanEdit),
...guidanceOpts,
overrideGuidance: overrides.PlanEdit,
}),
previousPlanBody: existing.body, previousPlanBody: existing.body,
}; };
} }
@@ -199,10 +193,7 @@ export function SelectModeTool(ctx: ToolContext) {
if (existing !== null) { if (existing !== null) {
ctx.toolState.existingSummaryCommentId = existing.commentId; ctx.toolState.existingSummaryCommentId = existing.commentId;
return { return {
...buildOrchestratorGuidance(selectedMode, { ...buildOrchestratorGuidance(ctx, selectedMode, overrides.SummaryUpdate),
...guidanceOpts,
overrideGuidance: overrides.SummaryUpdate,
}),
existingSummaryCommentId: existing.commentId, existingSummaryCommentId: existing.commentId,
previousSummaryBody: existing.body, previousSummaryBody: existing.body,
}; };
@@ -210,7 +201,7 @@ export function SelectModeTool(ctx: ToolContext) {
} }
} }
return buildOrchestratorGuidance(selectedMode, guidanceOpts); return buildOrchestratorGuidance(ctx, selectedMode);
}), }),
}); });
} }
+58 -60
View File
@@ -37812,6 +37812,33 @@ ${PULLFROG_DIVIDER}
<sup>${FROG_LOGO}&nbsp;&nbsp;\uFF5C ${allParts.join(" \uFF5C ")}</sup>`; <sup>${FROG_LOGO}&nbsp;&nbsp;\uFF5C ${allParts.join(" \uFF5C ")}</sup>`;
} }
// mcp/comment.ts
var LEAPING_INTO_ACTION_PREFIX = "Leaping into action";
var Comment = type({
issueNumber: type.number.describe("the issue number to comment on"),
body: type.string.describe("the comment body content"),
type: type.enumerated("Plan", "Summary", "Comment").describe(
"Plan: record as the plan for this run. Summary: record as the PR summary comment (one per PR, updated in place). Comment: regular comment (default)."
).optional()
});
var EditComment = type({
commentId: type.number.describe("the ID of the comment to edit"),
body: type.string.describe("the new comment body content")
});
var ReportProgress = type({
body: type.string.describe("the progress update content to share"),
"target_plan_comment?": type("boolean").describe(
"when true, update the existing plan comment (from select_mode lookup) instead of the progress comment; use when editing an existing plan"
)
});
var ReplyToReviewComment = type({
pull_number: type.number.describe("the pull request number"),
comment_id: type.number.describe("the ID of the review comment to reply to"),
body: type.string.describe(
"extremely brief reply (1 sentence max) explaining what was fixed, e.g. 'Fixed by renaming to X' or 'Added null check'"
)
});
// utils/github.ts // utils/github.ts
var core2 = __toESM(require_core(), 1); var core2 = __toESM(require_core(), 1);
@@ -41526,33 +41553,6 @@ function createOctokit(token) {
return octokit; return octokit;
} }
// mcp/comment.ts
var LEAPING_INTO_ACTION_PREFIX = "Leaping into action";
var Comment = type({
issueNumber: type.number.describe("the issue number to comment on"),
body: type.string.describe("the comment body content"),
type: type.enumerated("Plan", "Summary", "Comment").describe(
"Plan: record as the plan for this run. Summary: record as the PR summary comment (one per PR, updated in place). Comment: regular comment (default)."
).optional()
});
var EditComment = type({
commentId: type.number.describe("the ID of the comment to edit"),
body: type.string.describe("the new comment body content")
});
var ReportProgress = type({
body: type.string.describe("the progress update content to share"),
"target_plan_comment?": type("boolean").describe(
"when true, update the existing plan comment (from select_mode lookup) instead of the progress comment; use when editing an existing plan"
)
});
var ReplyToReviewComment = type({
pull_number: type.number.describe("the pull request number"),
comment_id: type.number.describe("the ID of the review comment to reply to"),
body: type.string.describe(
"extremely brief reply (1 sentence max) explaining what was fixed, e.g. 'Fixed by renaming to X' or 'Added null check'"
)
});
// utils/payload.ts // utils/payload.ts
var core3 = __toESM(require_core(), 1); var core3 = __toESM(require_core(), 1);
@@ -41725,40 +41725,44 @@ function getJobToken() {
// utils/postCleanup.ts // utils/postCleanup.ts
var SHOULD_CHECK_REASON = true; var SHOULD_CHECK_REASON = true;
function buildErrorCommentBody(params) { function buildErrorCommentBody(ctx, isCancellation) {
let errorMessage = params.isCancellation ? `This run was cancelled \u{1F6D1} let errorMessage = isCancellation ? `This run was cancelled \u{1F6D1}
The workflow was cancelled before completion.` : `This run croaked \u{1F635} The workflow was cancelled before completion.` : `This run croaked \u{1F635}
The workflow encountered an error before any progress could be reported.`; The workflow encountered an error before any progress could be reported.`;
if (params.runId) { if (ctx.runId) {
errorMessage += " Please check the link below for details."; errorMessage += " Please check the link below for details.";
} }
const customParts = []; const customParts = [];
if (!params.isCancellation && params.runId) { if (!isCancellation && ctx.runId) {
const apiUrl = getApiUrl(); const apiUrl = getApiUrl();
customParts.push( customParts.push(
`[Rerun failed job \u2794](${apiUrl}/trigger/${params.owner}/${params.repo}/${params.runId}?action=rerun)` `[Rerun failed job \u2794](${apiUrl}/trigger/${ctx.repoContext.owner}/${ctx.repoContext.name}/${ctx.runId}?action=rerun)`
); );
} }
const footer = buildPullfrogFooter({ const footer = buildPullfrogFooter({
triggeredBy: true, triggeredBy: true,
workflowRun: params.runId ? { owner: params.owner, repo: params.repo, runId: params.runId } : void 0, workflowRun: ctx.runId ? {
owner: ctx.repoContext.owner,
repo: ctx.repoContext.name,
runId: ctx.runId
} : void 0,
customParts customParts
}); });
return `${errorMessage}${footer}`; return `${errorMessage}${footer}`;
} }
async function validateStuckProgressComment(params) { async function validateStuckProgressComment(ctx) {
if (!params.promptInput?.progressCommentId) { if (!ctx.promptInput?.progressCommentId) {
log.info("[post] no progressCommentId in prompt input, skipping cleanup"); log.info("[post] no progressCommentId in prompt input, skipping cleanup");
return null; return null;
} }
const commentId = parseInt(params.promptInput.progressCommentId, 10); const commentId = parseInt(ctx.promptInput.progressCommentId, 10);
log.info(`[post] validating progressCommentId from prompt input: ${commentId}`); log.info(`[post] validating progressCommentId from prompt input: ${commentId}`);
try { try {
const commentResult = await params.octokit.rest.issues.getComment({ const commentResult = await ctx.octokit.rest.issues.getComment({
owner: params.owner, owner: ctx.repoContext.owner,
repo: params.repo, repo: ctx.repoContext.name,
comment_id: commentId comment_id: commentId
}); });
const body = commentResult.data.body ?? ""; const body = commentResult.data.body ?? "";
@@ -41778,13 +41782,13 @@ async function validateStuckProgressComment(params) {
return null; return null;
} }
} }
async function getIsCancelled(params) { async function getIsCancelled(ctx) {
if (!params.runId) return false; if (!ctx.runId) return false;
try { try {
const jobsResult = await params.octokit.rest.actions.listJobsForWorkflowRun({ const jobsResult = await ctx.octokit.rest.actions.listJobsForWorkflowRun({
owner: params.repoContext.owner, owner: ctx.repoContext.owner,
repo: params.repoContext.name, repo: ctx.repoContext.name,
run_id: params.runId run_id: ctx.runId
}); });
const currentJobName = process.env.GITHUB_JOB; const currentJobName = process.env.GITHUB_JOB;
const currentJob = currentJobName ? jobsResult.data.jobs.find( const currentJob = currentJobName ? jobsResult.data.jobs.find(
@@ -41824,24 +41828,18 @@ async function runPostCleanup() {
const token = getJobToken(); const token = getJobToken();
const repoContext = parseRepoContext(); const repoContext = parseRepoContext();
const octokit = createOctokit(token); const octokit = createOctokit(token);
const commentId = await validateStuckProgressComment({ const ctx = { repoContext, octokit, runId, promptInput };
promptInput, const commentId = await validateStuckProgressComment(ctx);
octokit,
owner: repoContext.owner,
repo: repoContext.name
});
if (!commentId) return log.info("\xBB [post] no stuck progress comment to update, skipping cleanup"); if (!commentId) return log.info("\xBB [post] no stuck progress comment to update, skipping cleanup");
log.info(`\xBB [post] validated stuck comment: ${commentId}, updating with error message`); log.info(`\xBB [post] validated stuck comment: ${commentId}, updating with error message`);
try { try {
const body = buildErrorCommentBody({ const body = buildErrorCommentBody(
owner: repoContext.owner, ctx,
repo: repoContext.name, SHOULD_CHECK_REASON ? await getIsCancelled(ctx) : false
runId, );
isCancellation: SHOULD_CHECK_REASON ? await getIsCancelled({ octokit, repoContext, runId }) : false await ctx.octokit.rest.issues.updateComment({
}); owner: ctx.repoContext.owner,
await octokit.rest.issues.updateComment({ repo: ctx.repoContext.name,
owner: repoContext.owner,
repo: repoContext.name,
comment_id: commentId, comment_id: commentId,
body body
}); });
+43 -75
View File
@@ -15,6 +15,14 @@ interface InstructionsContext {
learnings: string | null; learnings: string | null;
} }
interface PromptContext extends InstructionsContext {
t: (name: string) => string;
eventTitle: string;
eventMetadata: string;
runtime: string;
userQuoted: string;
}
function buildRuntimeContext(ctx: InstructionsContext): string { function buildRuntimeContext(ctx: InstructionsContext): string {
// extract payload fields excluding prompt/instructions/event (those are rendered separately) // extract payload fields excluding prompt/instructions/event (those are rendered separately)
const { const {
@@ -136,24 +144,25 @@ In case of conflict between instructions, follow this precedence (highest to low
// --------------------------------------------------------------------------- // ---------------------------------------------------------------------------
// the user's task: blockquoted user prompt, or event-level instructions for auto-triggers // the user's task: blockquoted user prompt, or event-level instructions for auto-triggers
function buildTaskSection(ctx: { userQuoted: string; eventInstructions: string }): string { function buildTaskSection(ctx: PromptContext): string {
if (ctx.userQuoted) { if (ctx.userQuoted) {
return `************* YOUR TASK ************* return `************* YOUR TASK *************
${ctx.userQuoted}`; ${ctx.userQuoted}`;
} }
if (ctx.eventInstructions) { const eventInstructions = ctx.payload.eventInstructions ?? "";
if (eventInstructions) {
return `************* YOUR TASK ************* return `************* YOUR TASK *************
${ctx.eventInstructions}`; ${eventInstructions}`;
} }
return ""; return "";
} }
// mode selection and execution steps // mode selection and execution steps
function buildProcedure(ctx: { modes: Mode[]; t: (name: string) => string }): string { function buildProcedure(ctx: PromptContext): string {
const t = ctx.t; const t = ctx.t;
return `************* PROCEDURE ************* return `************* PROCEDURE *************
@@ -180,11 +189,7 @@ Eagerly inspect the MCP tools available to you via the \`${pullfrogMcpName}\` MC
} }
// event title + metadata (omitted when empty, e.g. workflow_dispatch) // event title + metadata (omitted when empty, e.g. workflow_dispatch)
function buildEventContext(ctx: { function buildEventContext(ctx: PromptContext): string {
payload: ResolvedPayload;
eventTitle: string;
eventMetadata: string;
}): string {
const isPr = ctx.payload.event.is_pr === true; const isPr = ctx.payload.event.is_pr === true;
const relatedLabel = isPr ? "--- related PR ---" : "--- related issue ---"; const relatedLabel = isPr ? "--- related PR ---" : "--- related issue ---";
@@ -200,12 +205,7 @@ ${content}`;
} }
// persona, environment, priority, security, tools, workflow // persona, environment, priority, security, tools, workflow
function buildSystemBody(ctx: { function buildSystemBody(ctx: PromptContext): string {
shell: ResolvedPayload["shell"];
trigger: string;
t: (name: string) => string;
outputSchema?: Record<string, unknown> | undefined;
}): string {
const t = ctx.t; const t = ctx.t;
return `************* SYSTEM ************* return `************* SYSTEM *************
@@ -257,11 +257,11 @@ Rules:
Use MCP tools from ${pullfrogMcpName} for all GitHub operations. Never use the \`gh\` CLI — it is not authenticated and will fail. The MCP tools handle authentication and enforce permissions. Use MCP tools from ${pullfrogMcpName} for all GitHub operations. Never use the \`gh\` CLI — it is not authenticated and will fail. The MCP tools handle authentication and enforce permissions.
${getShellInstructions(ctx.shell, t)} ${getShellInstructions(ctx.payload.shell, t)}
${getFileInstructions()} ${getFileInstructions()}
${getStandaloneModeInstructions(ctx.trigger, t, ctx.outputSchema)} ${getStandaloneModeInstructions(ctx.payload.event.trigger, t, ctx.outputSchema)}
## Workflow ## Workflow
@@ -312,38 +312,20 @@ function buildToc(entries: TocEntry[]): string {
${entries.map((e) => `- ${e.label}${e.description}`).join("\n")}`; ${entries.map((e) => `- ${e.label}${e.description}`).join("\n")}`;
} }
// shared computation for all instruction builders function buildPromptContext(ctx: InstructionsContext): PromptContext {
interface CommonInputs {
eventTitle: string;
eventMetadata: string;
runtime: string;
user: string;
eventInstructions: string;
event: string;
userQuoted: string;
}
function buildCommonInputs(ctx: InstructionsContext): CommonInputs {
const eventTitle = buildEventTitle(ctx.payload.event);
const eventMetadata = buildEventMetadata(ctx.payload.event);
const runtime = buildRuntimeContext(ctx);
const user = ctx.payload.prompt; const user = ctx.payload.prompt;
const eventInstructions = ctx.payload.eventInstructions ?? "";
const event = [eventTitle, eventMetadata].filter(Boolean).join("\n\n---\n\n");
const userQuoted = user
? user
.split("\n")
.map((line) => `> ${line}`)
.join("\n")
: "";
return { return {
eventTitle, ...ctx,
eventMetadata, t: (toolName: string) => formatMcpToolRef(ctx.agentId, toolName),
runtime, eventTitle: buildEventTitle(ctx.payload.event),
user, eventMetadata: buildEventMetadata(ctx.payload.event),
eventInstructions, runtime: buildRuntimeContext(ctx),
event, userQuoted: user
userQuoted, ? user
.split("\n")
.map((line) => `> ${line}`)
.join("\n")
: "",
}; };
} }
@@ -387,28 +369,12 @@ function assembleFullPrompt(ctx: {
} }
export function resolveInstructions(ctx: InstructionsContext): ResolvedInstructions { export function resolveInstructions(ctx: InstructionsContext): ResolvedInstructions {
const inputs = buildCommonInputs(ctx); const pctx = buildPromptContext(ctx);
const t = (toolName: string) => formatMcpToolRef(ctx.agentId, toolName);
const task = buildTaskSection({ const task = buildTaskSection(pctx);
userQuoted: inputs.userQuoted, const procedure = buildProcedure(pctx);
eventInstructions: inputs.eventInstructions, const eventContext = buildEventContext(pctx);
}); const system = buildSystemBody(pctx);
const procedure = buildProcedure({ modes: ctx.modes, t });
const eventContext = buildEventContext({
payload: ctx.payload,
eventTitle: inputs.eventTitle,
eventMetadata: inputs.eventMetadata,
});
const system = buildSystemBody({
shell: ctx.payload.shell,
trigger: ctx.payload.event.trigger,
t,
outputSchema: ctx.outputSchema,
});
// build TOC from present sections (PROCEDURE, SYSTEM, RUNTIME are always present) // build TOC from present sections (PROCEDURE, SYSTEM, RUNTIME are always present)
const tocEntries: TocEntry[] = []; const tocEntries: TocEntry[] = [];
@@ -417,7 +383,7 @@ export function resolveInstructions(ctx: InstructionsContext): ResolvedInstructi
if (eventContext) if (eventContext)
tocEntries.push({ label: "EVENT CONTEXT", description: "related PR/issue data" }); tocEntries.push({ label: "EVENT CONTEXT", description: "related PR/issue data" });
tocEntries.push({ label: "SYSTEM", description: "persona, security, tools, workflow rules" }); tocEntries.push({ label: "SYSTEM", description: "persona, security, tools, workflow rules" });
if (ctx.learnings) if (pctx.learnings)
tocEntries.push({ label: "LEARNINGS", description: "repo-specific knowledge" }); tocEntries.push({ label: "LEARNINGS", description: "repo-specific knowledge" });
tocEntries.push({ label: "RUNTIME", description: "environment metadata" }); tocEntries.push({ label: "RUNTIME", description: "environment metadata" });
@@ -429,16 +395,18 @@ export function resolveInstructions(ctx: InstructionsContext): ResolvedInstructi
procedure, procedure,
eventContext, eventContext,
system, system,
learnings: ctx.learnings, learnings: pctx.learnings,
runtime: inputs.runtime, runtime: pctx.runtime,
}); });
const event = [pctx.eventTitle, pctx.eventMetadata].filter(Boolean).join("\n\n---\n\n");
return { return {
full, full,
system, system,
user: inputs.user, user: pctx.payload.prompt,
eventInstructions: inputs.eventInstructions, eventInstructions: pctx.payload.eventInstructions ?? "",
event: inputs.event, event,
runtime: inputs.runtime, runtime: pctx.runtime,
}; };
} }
+40 -57
View File
@@ -8,65 +8,61 @@ import { getJobToken } from "./token.ts";
type JsonPromptInput = Extract<ResolvedPromptInput, object>; // not string type JsonPromptInput = Extract<ResolvedPromptInput, object>; // not string
interface PostCleanupContext {
repoContext: ReturnType<typeof parseRepoContext>;
octokit: ReturnType<typeof createOctokit>;
runId: number | undefined;
promptInput: JsonPromptInput | null;
}
// controls whether the script should check the reason for the workflow termination. // controls whether the script should check the reason for the workflow termination.
// it can be either canceled or failed. // it can be either canceled or failed.
// YAML file cannot supply it (not in ENV), so an extra request is required to check it. // YAML file cannot supply it (not in ENV), so an extra request is required to check it.
const SHOULD_CHECK_REASON = true; const SHOULD_CHECK_REASON = true;
type BuildErrorCommentBodyParams = { function buildErrorCommentBody(ctx: PostCleanupContext, isCancellation: boolean): string {
owner: string; let errorMessage = isCancellation
repo: string;
runId: number | undefined;
isCancellation: boolean;
};
function buildErrorCommentBody(params: BuildErrorCommentBodyParams): string {
let errorMessage = params.isCancellation
? `This run was cancelled 🛑\n\nThe workflow was cancelled before completion.` ? `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.`; : `This run croaked 😵\n\nThe workflow encountered an error before any progress could be reported.`;
if (params.runId) { if (ctx.runId) {
errorMessage += " Please check the link below for details."; errorMessage += " Please check the link below for details.";
} }
const customParts: string[] = []; const customParts: string[] = [];
if (!params.isCancellation && params.runId) { if (!isCancellation && ctx.runId) {
const apiUrl = getApiUrl(); const apiUrl = getApiUrl();
customParts.push( customParts.push(
`[Rerun failed job ➔](${apiUrl}/trigger/${params.owner}/${params.repo}/${params.runId}?action=rerun)` `[Rerun failed job ➔](${apiUrl}/trigger/${ctx.repoContext.owner}/${ctx.repoContext.name}/${ctx.runId}?action=rerun)`
); );
} }
const footer = buildPullfrogFooter({ const footer = buildPullfrogFooter({
triggeredBy: true, triggeredBy: true,
workflowRun: params.runId workflowRun: ctx.runId
? { owner: params.owner, repo: params.repo, runId: params.runId } ? {
owner: ctx.repoContext.owner,
repo: ctx.repoContext.name,
runId: ctx.runId,
}
: undefined, : undefined,
customParts, customParts,
}); });
return `${errorMessage}${footer}`; return `${errorMessage}${footer}`;
} }
type ValidateStuckCommentParams = { async function validateStuckProgressComment(ctx: PostCleanupContext): Promise<number | null> {
promptInput: JsonPromptInput | null; if (!ctx.promptInput?.progressCommentId) {
octokit: ReturnType<typeof createOctokit>;
owner: string;
repo: string;
};
async function validateStuckProgressComment(
params: ValidateStuckCommentParams
): Promise<number | null> {
if (!params.promptInput?.progressCommentId) {
log.info("[post] no progressCommentId in prompt input, skipping cleanup"); log.info("[post] no progressCommentId in prompt input, skipping cleanup");
return null; return null;
} }
const commentId = parseInt(params.promptInput.progressCommentId, 10); const commentId = parseInt(ctx.promptInput.progressCommentId, 10);
log.info(`[post] validating progressCommentId from prompt input: ${commentId}`); log.info(`[post] validating progressCommentId from prompt input: ${commentId}`);
try { try {
const commentResult = await params.octokit.rest.issues.getComment({ const commentResult = await ctx.octokit.rest.issues.getComment({
owner: params.owner, owner: ctx.repoContext.owner,
repo: params.repo, repo: ctx.repoContext.name,
comment_id: commentId, comment_id: commentId,
}); });
@@ -93,19 +89,13 @@ async function validateStuckProgressComment(
} }
} }
type GetIsCancelledParams = { async function getIsCancelled(ctx: PostCleanupContext): Promise<boolean> {
repoContext: ReturnType<typeof parseRepoContext>; if (!ctx.runId) return false; // can't check without a run ID — assume failure
octokit: ReturnType<typeof createOctokit>;
runId: number | undefined;
};
async function getIsCancelled(params: GetIsCancelledParams): Promise<boolean> {
if (!params.runId) return false; // can't check without a run ID — assume failure
try { try {
const jobsResult = await params.octokit.rest.actions.listJobsForWorkflowRun({ const jobsResult = await ctx.octokit.rest.actions.listJobsForWorkflowRun({
owner: params.repoContext.owner, owner: ctx.repoContext.owner,
repo: params.repoContext.name, repo: ctx.repoContext.name,
run_id: params.runId, run_id: ctx.runId,
}); });
// find current job by matching GITHUB_JOB env var. // find current job by matching GITHUB_JOB env var.
@@ -166,30 +156,23 @@ export async function runPostCleanup(): Promise<void> {
const repoContext = parseRepoContext(); const repoContext = parseRepoContext();
const octokit = createOctokit(token); const octokit = createOctokit(token);
const commentId = await validateStuckProgressComment({ const ctx: PostCleanupContext = { repoContext, octokit, runId, promptInput };
promptInput,
octokit, const commentId = await validateStuckProgressComment(ctx);
owner: repoContext.owner,
repo: repoContext.name,
});
if (!commentId) return log.info("» [post] no stuck progress comment to update, skipping cleanup"); if (!commentId) 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: ${commentId}, updating with error message`);
try { try {
const body = buildErrorCommentBody({ const body = buildErrorCommentBody(
owner: repoContext.owner, ctx,
repo: repoContext.name, SHOULD_CHECK_REASON ? await getIsCancelled(ctx) : false
runId, );
isCancellation: SHOULD_CHECK_REASON
? await getIsCancelled({ octokit, repoContext, runId })
: false,
});
await octokit.rest.issues.updateComment({ await ctx.octokit.rest.issues.updateComment({
owner: repoContext.owner, owner: ctx.repoContext.owner,
repo: repoContext.name, repo: ctx.repoContext.name,
comment_id: commentId, comment_id: commentId,
body, body,
}); });