diff --git a/entry b/entry index eb81fc9..e5210b4 100755 --- a/entry +++ b/entry @@ -140142,7 +140142,6 @@ ${params.instructions}`; system: subagentSystemPreamble, user: params.instructions, eventInstructions: "", - repo: "", event: "", runtime: "" }; @@ -140182,6 +140181,9 @@ async function runSubagent(params) { completeSubagent({ ctx: params.ctx, subagent: params.subagent, success: false }); return { success: false, error: errorMessage }; } finally { + if (mcpServer.toolState.review) { + params.ctx.toolState.review = mcpServer.toolState.review; + } await mcpServer.stop(); } }); @@ -144834,6 +144836,7 @@ function CheckoutPrTool(ctx) { if (!headRepo) { throw new Error(`PR #${pull_number} source repository was deleted`); } + ctx.toolState.checkoutSha = pr.data.head.sha; const formatResult = await fetchAndFormatPrDiff({ octokit: ctx.octokit, owner: ctx.repo.owner, @@ -147233,8 +147236,34 @@ function CreatePullRequestReviewTool(ctx) { } const reviewId = result.data.id; const reviewNodeId = result.data.node_id; - await reportReviewNodeId(ctx, reviewNodeId); - await deleteProgressComment(ctx); + const actuallyReviewedSha = ctx.toolState.checkoutSha ?? params.commit_id; + ctx.toolState.review = { + id: reviewId, + nodeId: reviewNodeId, + reviewedSha: actuallyReviewedSha + }; + const headMovedDuringReview = ctx.toolState.checkoutSha && params.commit_id !== ctx.toolState.checkoutSha; + if (headMovedDuringReview) { + const fromSha = ctx.toolState.checkoutSha; + const toSha = params.commit_id; + ctx.toolState.checkoutSha = toSha; + log.info( + `new commits detected during review: ${fromSha.slice(0, 7)}..${toSha.slice(0, 7)}` + ); + return { + success: true, + reviewId, + html_url: result.data.html_url, + state: result.data.state, + user: result.data.user?.login, + submitted_at: result.data.submitted_at, + newCommits: { + from: fromSha, + to: toSha, + instructions: `New commits were pushed while you were reviewing. Run \`git pull\` to fetch them, then review the incremental diff with \`git diff ${fromSha}...HEAD\`. Submit another review covering only the new changes. Do not repeat feedback from your previous review.` + } + }; + } return { success: true, reviewId, @@ -147912,9 +147941,9 @@ Each task in the \`tasks\` array should include: After all tasks complete, consolidate into a **single** review: - merge the \`comments\` arrays from all subagent outputs -- if subagents found actionable issues: submit one \`${ghPullfrogMcpName}/create_pull_request_review\` with \`approved: false\`, the merged comments, and a unified summary body -- if no subagent found actionable issues: submit with \`approved: true\` and a brief positive summary (no inline comments) -- call \`${ghPullfrogMcpName}/report_progress\` with the summary +- if subagents found actionable issues: submit one \`${ghPullfrogMcpName}/create_pull_request_review\` with \`approved: false\`, the merged comments, and an **empty body** (do NOT include a summary \u2014 inline comments speak for themselves and a top-level comment clutters the PR conversation on every re-review) +- if no subagent found actionable issues: submit with \`approved: true\` and an **empty body** (no inline comments, no summary) +- do NOT call \`${ghPullfrogMcpName}/report_progress\` \u2014 incremental reviews should be silent Use max effort for thorough reviews.`, Plan: `### Checklist @@ -147982,8 +148011,15 @@ Use auto effort.`, - if the task involved labeling, commenting, or other GitHub operations, perform those directly 5. Use mini effort for simple research tasks, auto for typical tasks, max for complex multi-file changes.` }; -function buildOrchestratorGuidance(mode, overrideGuidance) { - const guidance = overrideGuidance ?? modeGuidance[mode.name] ?? ""; +var modeInstructionParent = { + IncrementalReview: "Review", + Fix: "Build" +}; +function buildOrchestratorGuidance(mode, opts = {}) { + const hardcoded = opts.overrideGuidance ?? modeGuidance[mode.name] ?? mode.prompt ?? ""; + const lookupKey = modeInstructionParent[mode.name] ?? mode.name; + const userInstructions = opts.modeInstructions?.[lookupKey] ?? ""; + const guidance = [hardcoded, userInstructions].filter(Boolean).join("\n\n"); return { modeName: mode.name, description: mode.description, @@ -148029,6 +148065,7 @@ function SelectModeTool(ctx) { }; } ctx.toolState.selectedMode = selectedMode.name; + const guidanceOpts = { modeInstructions: ctx.modeInstructions }; if (selectedMode.name === "Plan") { const issueNumber = params.issue_number ?? ctx.payload.event.issue_number; if (issueNumber !== void 0) { @@ -148037,13 +148074,16 @@ function SelectModeTool(ctx) { ctx.toolState.existingPlanCommentId = existing.commentId; ctx.toolState.previousPlanBody = existing.body; return { - ...buildOrchestratorGuidance(selectedMode, modeGuidance.PlanEdit), + ...buildOrchestratorGuidance(selectedMode, { + ...guidanceOpts, + overrideGuidance: modeGuidance.PlanEdit + }), previousPlanBody: existing.body }; } } } - return buildOrchestratorGuidance(selectedMode); + return buildOrchestratorGuidance(selectedMode, guidanceOpts); }) }); } @@ -148546,7 +148586,11 @@ async function startSubagentMcpServer(params) { const subagentCtx = { ...params.ctx, toolState: subagentToolState }; const tools = buildSubagentTools(subagentCtx); const startResult = await selectMcpPort(subagentCtx, tools); - return { url: startResult.url, stop: () => startResult.server.stop() }; + return { + url: startResult.url, + stop: () => startResult.server.stop(), + toolState: subagentToolState + }; } // modes.ts @@ -151032,7 +151076,6 @@ function buildRuntimeContext(ctx) { "~pullfrog": _, prompt: _p, eventInstructions: _ei, - repoInstructions: _r, event: _e, ...payloadRest } = ctx.payload; @@ -151220,14 +151263,10 @@ var orchestratorPriorityOrder = `## Priority Order In case of conflict between instructions, follow this precedence (highest to lowest): 1. Security rules and system instructions (non-overridable) 2. User prompt -3. Event-level instructions -4. Repo-level instructions`; +3. Event-level instructions`; function buildContextSections(ctx) { const isPr = ctx.payload.event.is_pr === true; const relatedLabel = isPr ? "--- related PR ---" : "--- related issue ---"; - const repoSection = ctx.repo ? `************* REPO-LEVEL INSTRUCTIONS ************* - -${ctx.repo}` : ""; const eventInstructionsSection = ctx.eventInstructions ? `************* EVENT-LEVEL INSTRUCTIONS ************* ${ctx.eventInstructions}` : ""; @@ -151248,7 +151287,7 @@ ${metadataSection}` : `************* EVENT CONTEXT ************* ${titleBodySection} ${metadataSection}`; - return [repoSection, eventInstructionsSection, userSection].filter(Boolean).join("\n\n"); + return [eventInstructionsSection, userSection].filter(Boolean).join("\n\n"); } function buildCommonInputs(ctx) { const eventTitleBody = buildEventTitleBody(ctx.payload.event); @@ -151256,7 +151295,6 @@ function buildCommonInputs(ctx) { const runtime = buildRuntimeContext(ctx); const user = ctx.payload.prompt; const eventInstructions = ctx.payload.eventInstructions ?? ""; - const repo = ctx.payload.repoInstructions ?? ""; const event = [eventTitleBody, eventMetadata].filter(Boolean).join("\n\n---\n\n"); const userQuoted = user ? user.split("\n").map((line) => `> ${line}`).join("\n") : ""; return { @@ -151265,7 +151303,6 @@ function buildCommonInputs(ctx) { runtime, user, eventInstructions, - repo, event, userQuoted }; @@ -151336,7 +151373,6 @@ If the task clearly requires no work, skip delegation. Call \`${ghPullfrogMcpNam }); const contextSections = buildContextSections({ payload: ctx.payload, - repo: inputs.repo, eventInstructions: inputs.eventInstructions, eventTitleBody: inputs.eventTitleBody, eventMetadata: inputs.eventMetadata, @@ -151352,7 +151388,6 @@ If the task clearly requires no work, skip delegation. Call \`${ghPullfrogMcpNam system, user: inputs.user, eventInstructions: inputs.eventInstructions, - repo: inputs.repo, event: inputs.event, runtime: inputs.runtime }; @@ -151435,7 +151470,6 @@ var JsonPayload = type({ prompt: "string", "triggerer?": "string | undefined", "eventInstructions?": "string", - "repoInstructions?": "string", "event?": "object", "effort?": Effort.or("undefined"), "timeout?": "string | undefined", @@ -151530,7 +151564,6 @@ function resolvePayload(resolvedPromptInput, repoSettings) { triggerer: jsonPayload?.triggerer ?? // it's not a common use case but GITHUB_ACTOR can be a user when the workflow is manually triggered by a user through GitHub Actions UI (!isPullfrog(process.env.GITHUB_ACTOR) ? process.env.GITHUB_ACTOR : void 0), eventInstructions: jsonPayload?.eventInstructions, - repoInstructions: jsonPayload?.repoInstructions, event, effort: inputs.effort ?? jsonPayload?.effort ?? "auto", timeout: inputs.timeout ?? jsonPayload?.timeout, @@ -151545,6 +151578,73 @@ function resolvePayload(resolvedPromptInput, repoSettings) { }; } +// utils/reviewCleanup.ts +var RE_REVIEW_PREAMBLE = "Incrementally re-review the new commits on this pull request. Use the IncrementalReview mode."; +async function postReviewCleanup(ctx) { + const review = ctx.toolState.review; + if (!review) return; + delete ctx.toolState.review; + await bestEffort(() => reportReviewNodeId(ctx, review.nodeId), "reportReviewNodeId"); + if (review.reviewedSha) { + await bestEffort( + () => dispatchFollowUpReReview(ctx, review.reviewedSha), + "follow-up re-review dispatch" + ); + } + await bestEffort(() => deleteProgressComment(ctx), "delete progress comment"); +} +async function bestEffort(fn2, label) { + try { + await fn2(); + } catch (error49) { + log.debug(`${label} failed: ${error49}`); + } +} +async function dispatchFollowUpReReview(ctx, reviewedSha) { + const issueNumber = ctx.payload.event.issue_number; + if (!issueNumber) return; + const pr = await ctx.octokit.rest.pulls.get({ + owner: ctx.repo.owner, + repo: ctx.repo.name, + pull_number: issueNumber + }); + if (pr.data.head.sha === reviewedSha) return; + if (pr.data.state !== "open") return; + if (pr.data.draft) return; + log.info( + `safety net: pr HEAD moved from ${reviewedSha.slice(0, 7)} to ${pr.data.head.sha.slice(0, 7)} and agent did not review inline \u2014 dispatching follow-up re-review` + ); + const event = { + trigger: "pull_request_synchronize", + issue_number: issueNumber, + is_pr: true, + title: pr.data.title, + body: null, + branch: pr.data.head.ref, + before_sha: reviewedSha, + silent: true + }; + if (ctx.payload.event.authorPermission) { + event.authorPermission = ctx.payload.event.authorPermission; + } + const payload = { + "~pullfrog": true, + version: ctx.payload.version, + agent: ctx.payload.agent, + prompt: "", + eventInstructions: RE_REVIEW_PREAMBLE, + event, + effort: "max" + }; + await ctx.octokit.rest.actions.createWorkflowDispatch({ + owner: ctx.repo.owner, + repo: ctx.repo.name, + workflow_id: "pullfrog.yml", + ref: pr.data.base.repo.default_branch, + inputs: { prompt: JSON.stringify(payload) } + }); +} + // utils/run.ts async function handleAgentResult(ctx) { if (!ctx.result.success) { @@ -151583,12 +151683,12 @@ var defaultSettings = { modes: [], setupScript: null, postCheckoutScript: null, - repoInstructions: "", web: "enabled", search: "enabled", push: "restricted", shell: "restricted", - prApproveEnabled: false + prApproveEnabled: false, + modeInstructions: {} }; var defaultRunContext = { settings: defaultSettings, @@ -151820,6 +151920,7 @@ async function main() { } const octokit = createOctokit(tokenRef.mcpToken); const runInfo = await resolveRun({ octokit }); + let toolContext; try { var _stack = []; try { @@ -151866,7 +151967,7 @@ async function main() { timer.checkpoint("lifecycleHooks::setup"); const modes2 = [...computeModes(), ...runContext.repoSettings.modes]; const outputSchema = resolveOutputSchema(); - const toolContext = { + toolContext = { repo: runContext.repo, payload, octokit, @@ -151877,6 +151978,7 @@ async function main() { modes: modes2, postCheckoutScript: runContext.repoSettings.postCheckoutScript, prApproveEnabled: runContext.repoSettings.prApproveEnabled, + modeInstructions: runContext.repoSettings.modeInstructions, toolState, runId: runInfo.runId, jobId: runInfo.jobId, @@ -151947,8 +152049,14 @@ ${instructions.user}` : null, "output_schema was provided but agent did not call set_output \u2014 structured output is required" ); } + if (toolContext) { + await postReviewCleanup(toolContext).catch((error49) => { + log.debug(`post-review cleanup failed: ${error49}`); + }); + } await writeJobSummary(toolState); if (toolState.output) { + log.info(`::pullfrog-output::${Buffer.from(toolState.output).toString("base64")}`); core5.setOutput("result", toolState.output); } return await handleAgentResult({ @@ -151974,6 +152082,11 @@ ${instructions.user}` : null, await reportErrorToComment({ toolState, error: errorMessage }); } catch { } + if (toolContext) { + await postReviewCleanup(toolContext).catch((error50) => { + log.debug(`post-review cleanup failed: ${error50}`); + }); + } return { success: false, error: errorMessage diff --git a/external.ts b/external.ts index 3a25734..eda3d82 100644 --- a/external.ts +++ b/external.ts @@ -288,8 +288,6 @@ export interface WriteablePayload { triggerer?: string | undefined; /** event-level instructions for this trigger type (flag-expanded server-side) */ eventInstructions?: string | undefined; - /** repo-level instructions (flag-expanded server-side) */ - repoInstructions?: string | undefined; /** event data from webhook payload - discriminated union based on trigger field */ event: PayloadEvent; /** effort level for model selection (mini, auto, max) - defaults to "auto" */ diff --git a/main.ts b/main.ts index d20eb59..745e3fb 100644 --- a/main.ts +++ b/main.ts @@ -1,7 +1,12 @@ // changes to tool permissions should be reflected in wiki/granular-tools.md import * as core from "@actions/core"; -import { initToolState, startMcpHttpServer, type ToolState } from "./mcp/server.ts"; +import { + initToolState, + startMcpHttpServer, + type ToolContext, + type ToolState, +} from "./mcp/server.ts"; import { computeModes } from "./modes.ts"; import { type ActivityTimeout, @@ -21,6 +26,7 @@ import { resolveInstructions } from "./utils/instructions.ts"; import { executeLifecycleHook } from "./utils/lifecycle.ts"; import { normalizeEnv } from "./utils/normalizeEnv.ts"; import { resolvePayload, resolvePromptInput } from "./utils/payload.ts"; +import { postReviewCleanup } from "./utils/reviewCleanup.ts"; import { handleAgentResult } from "./utils/run.ts"; import { resolveRunContextData } from "./utils/runContextData.ts"; import { createTempDirectory, setupGit } from "./utils/setup.ts"; @@ -112,6 +118,7 @@ export async function main(): Promise { const octokit = createOctokit(tokenRef.mcpToken); const runInfo = await resolveRun({ octokit }); + let toolContext: ToolContext | undefined; try { // enable debug logging if --debug flag was used @@ -173,7 +180,7 @@ export async function main(): Promise { const outputSchema = resolveOutputSchema(); // mcpServerUrl and tmpdir are set after server starts — delegate tool reads them at call time - const toolContext = { + toolContext = { repo: runContext.repo, payload, octokit, @@ -184,6 +191,7 @@ export async function main(): Promise { modes, postCheckoutScript: runContext.repoSettings.postCheckoutScript, prApproveEnabled: runContext.repoSettings.prApproveEnabled, + modeInstructions: runContext.repoSettings.modeInstructions, toolState, runId: runInfo.runId, jobId: runInfo.jobId, @@ -265,9 +273,20 @@ export async function main(): Promise { ); } + // post-agent review cleanup: reportReviewNodeId → follow-up dispatch → delete progress comment. + // runs after the agent exits so ordering is architecturally guaranteed (no LLM involvement). + // best-effort: cleanup failures must not turn a successful agent run into a failure. + if (toolContext) { + await postReviewCleanup(toolContext).catch((error) => { + log.debug(`post-review cleanup failed: ${error}`); + }); + } + await writeJobSummary(toolState); + // emit structured output marker for test validation if (toolState.output) { + log.info(`::pullfrog-output::${Buffer.from(toolState.output).toString("base64")}`); core.setOutput("result", toolState.output); } @@ -291,6 +310,14 @@ export async function main(): Promise { } catch { // error reporting failed, but don't let it mask the original error } + + // best-effort review cleanup (e.g., agent timed out after submitting a review) + if (toolContext) { + await postReviewCleanup(toolContext).catch((error) => { + log.debug(`post-review cleanup failed: ${error}`); + }); + } + return { success: false, error: errorMessage, diff --git a/mcp/checkout.ts b/mcp/checkout.ts index baaaf5b..d06d95e 100644 --- a/mcp/checkout.ts +++ b/mcp/checkout.ts @@ -355,7 +355,6 @@ export function CheckoutPrTool(ctx: ToolContext) { postCheckoutScript: ctx.postCheckoutScript, }); - // fetch PR metadata to return result const pr = await ctx.octokit.rest.pulls.get({ owner: ctx.repo.owner, repo: ctx.repo.name, @@ -367,6 +366,8 @@ export function CheckoutPrTool(ctx: ToolContext) { throw new Error(`PR #${pull_number} source repository was deleted`); } + ctx.toolState.checkoutSha = pr.data.head.sha; + // fetch PR files and format with line numbers const formatResult = await fetchAndFormatPrDiff({ octokit: ctx.octokit, diff --git a/mcp/review.ts b/mcp/review.ts index 98be317..dbbe3b6 100644 --- a/mcp/review.ts +++ b/mcp/review.ts @@ -4,7 +4,6 @@ import { apiFetch } from "../utils/apiFetch.ts"; import { getApiUrl } from "../utils/apiUrl.ts"; import { buildPullfrogFooter } from "../utils/buildPullfrogFooter.ts"; import { log } from "../utils/cli.ts"; -import { deleteProgressComment } from "./comment.ts"; import type { ToolContext } from "./server.ts"; import { execute, tool } from "./shared.ts"; @@ -81,7 +80,6 @@ export function CreatePullRequestReviewTool(ctx: ToolContext) { event = "COMMENT"; } - // compose the request const params: RestEndpointMethodTypes["pulls"]["createReview"]["parameters"] = { owner: ctx.repo.owner, repo: ctx.repo.name, @@ -91,7 +89,6 @@ export function CreatePullRequestReviewTool(ctx: ToolContext) { if (commit_id) { params.commit_id = commit_id; } else { - // get the PR to determine the head commit if commit_id not provided const pr = await ctx.octokit.rest.pulls.get({ owner: ctx.repo.owner, repo: ctx.repo.name, @@ -101,9 +98,7 @@ export function CreatePullRequestReviewTool(ctx: ToolContext) { } if (comments.length > 0) { type ReviewComment = (typeof params.comments & {})[number]; - // convert comments to the format expected by GitHub API params.comments = comments.map((comment) => { - // build comment body with suggestion block if provided let commentBody = comment.body || ""; if (comment.suggestion !== undefined) { const suggestionBlock = "```suggestion\n" + comment.suggestion + "\n```"; @@ -139,12 +134,49 @@ export function CreatePullRequestReviewTool(ctx: ToolContext) { const reviewId = result.data.id; const reviewNodeId = result.data.node_id; - // report review node ID to server so the in-flight dedup check - // in the synchronize webhook handler sees this run as "review submitted." - // awaited (not fire-and-forget) to guarantee the signal lands before - // any subsequent push's webhook checks for in-flight runs. - await reportReviewNodeId(ctx, reviewNodeId); - await deleteProgressComment(ctx); + // reviewedSha = what the agent actually reviewed (checkout SHA), not the + // submission anchor (current HEAD). this ensures postReviewCleanup dispatches + // a follow-up if the agent doesn't handle new commits inline. + const actuallyReviewedSha = ctx.toolState.checkoutSha ?? params.commit_id; + ctx.toolState.review = { + id: reviewId, + nodeId: reviewNodeId, + reviewedSha: actuallyReviewedSha, + }; + + // detect commits pushed since checkout and guide the agent to review them + // inline instead of dispatching a separate workflow run + const headMovedDuringReview = + ctx.toolState.checkoutSha && params.commit_id !== ctx.toolState.checkoutSha; + + if (headMovedDuringReview) { + const fromSha = ctx.toolState.checkoutSha!; + const toSha = params.commit_id!; + // advance checkoutSha so the next review submission tracks correctly + ctx.toolState.checkoutSha = toSha; + + log.info( + `new commits detected during review: ${fromSha.slice(0, 7)}..${toSha.slice(0, 7)}` + ); + + return { + success: true, + reviewId, + html_url: result.data.html_url, + state: result.data.state, + user: result.data.user?.login, + submitted_at: result.data.submitted_at, + newCommits: { + from: fromSha, + to: toSha, + instructions: + `New commits were pushed while you were reviewing. ` + + `Run \`git pull\` to fetch them, then review the incremental diff ` + + `with \`git diff ${fromSha}...HEAD\`. Submit another review covering ` + + `only the new changes. Do not repeat feedback from your previous review.`, + }, + }; + } return { success: true, @@ -202,7 +234,11 @@ async function createAndSubmitWithFooter( }); } -async function reportReviewNodeId(ctx: ToolContext, reviewNodeId: string): Promise { +/** + * report the review node ID to the server so the WorkflowRun is marked as "review submitted". + * exported for use in main.ts post-agent cleanup. + */ +export async function reportReviewNodeId(ctx: ToolContext, reviewNodeId: string): Promise { for (let remaining = 2; remaining >= 0; remaining--) { try { const response = await apiFetch({ diff --git a/mcp/selectMode.ts b/mcp/selectMode.ts index 28ee91b..952d5ba 100644 --- a/mcp/selectMode.ts +++ b/mcp/selectMode.ts @@ -153,9 +153,9 @@ Each task in the \`tasks\` array should include: After all tasks complete, consolidate into a **single** review: - merge the \`comments\` arrays from all subagent outputs -- if subagents found actionable issues: submit one \`${ghPullfrogMcpName}/create_pull_request_review\` with \`approved: false\`, the merged comments, and a unified summary body -- if no subagent found actionable issues: submit with \`approved: true\` and a brief positive summary (no inline comments) -- call \`${ghPullfrogMcpName}/report_progress\` with the summary +- if subagents found actionable issues: submit one \`${ghPullfrogMcpName}/create_pull_request_review\` with \`approved: false\`, the merged comments, and an **empty body** (do NOT include a summary — inline comments speak for themselves and a top-level comment clutters the PR conversation on every re-review) +- if no subagent found actionable issues: submit with \`approved: true\` and an **empty body** (no inline comments, no summary) +- do NOT call \`${ghPullfrogMcpName}/report_progress\` — incremental reviews should be silent Use max effort for thorough reviews.`, @@ -234,8 +234,21 @@ type OrchestratorGuidance = { orchestratorGuidance: string; }; -function buildOrchestratorGuidance(mode: Mode, overrideGuidance?: string): OrchestratorGuidance { - const guidance = overrideGuidance ?? modeGuidance[mode.name] ?? ""; +const modeInstructionParent: Record = { + IncrementalReview: "Review", + Fix: "Build", +}; + +type BuildGuidanceOpts = { + modeInstructions?: Record; + overrideGuidance?: string; +}; + +function buildOrchestratorGuidance(mode: Mode, opts: BuildGuidanceOpts = {}): OrchestratorGuidance { + const hardcoded = opts.overrideGuidance ?? modeGuidance[mode.name] ?? mode.prompt ?? ""; + const lookupKey = modeInstructionParent[mode.name] ?? mode.name; + const userInstructions = opts.modeInstructions?.[lookupKey] ?? ""; + const guidance = [hardcoded, userInstructions].filter(Boolean).join("\n\n"); return { modeName: mode.name, description: mode.description, @@ -295,6 +308,8 @@ export function SelectModeTool(ctx: ToolContext) { ctx.toolState.selectedMode = selectedMode.name; + const guidanceOpts: BuildGuidanceOpts = { modeInstructions: ctx.modeInstructions }; + if (selectedMode.name === "Plan") { const issueNumber = params.issue_number ?? ctx.payload.event.issue_number; if (issueNumber !== undefined) { @@ -303,14 +318,17 @@ export function SelectModeTool(ctx: ToolContext) { ctx.toolState.existingPlanCommentId = existing.commentId; ctx.toolState.previousPlanBody = existing.body; return { - ...buildOrchestratorGuidance(selectedMode, modeGuidance.PlanEdit), + ...buildOrchestratorGuidance(selectedMode, { + ...guidanceOpts, + overrideGuidance: modeGuidance.PlanEdit, + }), previousPlanBody: existing.body, }; } } } - return buildOrchestratorGuidance(selectedMode); + return buildOrchestratorGuidance(selectedMode, guidanceOpts); }), }); } diff --git a/mcp/server.ts b/mcp/server.ts index b2e9926..ee7a34d 100644 --- a/mcp/server.ts +++ b/mcp/server.ts @@ -44,6 +44,8 @@ export interface ToolState { pushDest?: StoredPushDest; // issue or PR number (same number space in GitHub) issueNumber?: number; + // PR HEAD sha at checkout time — used to detect new commits pushed during a review + checkoutSha?: string; selectedMode?: string; // per-subagent lifecycle tracking (keyed by subagent uuid) subagents: Map; @@ -54,6 +56,7 @@ export interface ToolState { review?: { id: number; nodeId: string; + reviewedSha: string | undefined; }; dependencyInstallation?: { status: "not_started" | "in_progress" | "completed" | "failed"; @@ -103,6 +106,7 @@ export interface ToolContext { modes: Mode[]; postCheckoutScript: string | null; prApproveEnabled: boolean; + modeInstructions: Record; toolState: ToolState; runId: number | undefined; jobId: string | undefined; @@ -384,6 +388,7 @@ export async function startMcpHttpServer( export type ManagedMcpServer = { url: string; stop: () => Promise; + toolState: ToolState; }; type StartSubagentMcpServerParams = { @@ -412,5 +417,9 @@ export async function startSubagentMcpServer( const subagentCtx: ToolContext = { ...params.ctx, toolState: subagentToolState }; const tools = buildSubagentTools(subagentCtx); const startResult = await selectMcpPort(subagentCtx, tools); - return { url: startResult.url, stop: () => startResult.server.stop() }; + return { + url: startResult.url, + stop: () => startResult.server.stop(), + toolState: subagentToolState, + }; } diff --git a/mcp/subagent.ts b/mcp/subagent.ts index 2fa0e92..e40978e 100644 --- a/mcp/subagent.ts +++ b/mcp/subagent.ts @@ -114,7 +114,6 @@ export function buildSubagentInstructions( system: subagentSystemPreamble, user: params.instructions, eventInstructions: "", - repo: "", event: "", runtime: "", }; @@ -169,6 +168,10 @@ export async function runSubagent(params: RunSubagentParams): Promise { + const review = ctx.toolState.review; + if (!review) return; + delete ctx.toolState.review; + + // mark review as submitted — unlocks webhook dedup for new pushes + await bestEffort(() => reportReviewNodeId(ctx, review.nodeId), "reportReviewNodeId"); + + // dispatch follow-up if PR HEAD moved past the reviewed commit + if (review.reviewedSha) { + await bestEffort( + () => dispatchFollowUpReReview(ctx, review.reviewedSha!), + "follow-up re-review dispatch" + ); + } + + await bestEffort(() => deleteProgressComment(ctx), "delete progress comment"); +} + +async function bestEffort(fn: () => Promise, label: string): Promise { + try { + await fn(); + } catch (error) { + log.debug(`${label} failed: ${error}`); + } +} + +async function dispatchFollowUpReReview(ctx: ToolContext, reviewedSha: string): Promise { + const issueNumber = ctx.payload.event.issue_number; + if (!issueNumber) return; + + const pr = await ctx.octokit.rest.pulls.get({ + owner: ctx.repo.owner, + repo: ctx.repo.name, + pull_number: issueNumber, + }); + + if (pr.data.head.sha === reviewedSha) return; + if (pr.data.state !== "open") return; + if (pr.data.draft) return; + + log.info( + `safety net: pr HEAD moved from ${reviewedSha.slice(0, 7)} to ${pr.data.head.sha.slice(0, 7)} ` + + `and agent did not review inline — dispatching follow-up re-review` + ); + + const event: WriteablePayload["event"] = { + trigger: "pull_request_synchronize", + issue_number: issueNumber, + is_pr: true, + title: pr.data.title, + body: null, + branch: pr.data.head.ref, + before_sha: reviewedSha, + silent: true, + }; + if (ctx.payload.event.authorPermission) { + event.authorPermission = ctx.payload.event.authorPermission; + } + + const payload: WriteablePayload = { + "~pullfrog": true, + version: ctx.payload.version, + agent: ctx.payload.agent, + prompt: "", + eventInstructions: RE_REVIEW_PREAMBLE, + event, + effort: "max", + }; + + await ctx.octokit.rest.actions.createWorkflowDispatch({ + owner: ctx.repo.owner, + repo: ctx.repo.name, + workflow_id: "pullfrog.yml", + ref: pr.data.base.repo.default_branch, + inputs: { prompt: JSON.stringify(payload) }, + }); +} diff --git a/utils/runContext.ts b/utils/runContext.ts index 3f123f4..1cee2ec 100644 --- a/utils/runContext.ts +++ b/utils/runContext.ts @@ -14,12 +14,12 @@ export interface RepoSettings { modes: Mode[]; setupScript: string | null; postCheckoutScript: string | null; - repoInstructions: string; web: ToolPermission; search: ToolPermission; push: PushPermission; shell: ShellPermission; prApproveEnabled: boolean; + modeInstructions: Record; } export interface RunContext { @@ -32,12 +32,12 @@ const defaultSettings: RepoSettings = { modes: [], setupScript: null, postCheckoutScript: null, - repoInstructions: "", web: "enabled", search: "enabled", push: "restricted", shell: "restricted", prApproveEnabled: false, + modeInstructions: {}, }; const defaultRunContext: RunContext = {