diff --git a/src/services/agent-action-executor.ts b/src/services/agent-action-executor.ts index 6ca87c7d91..aa22f0de20 100644 --- a/src/services/agent-action-executor.ts +++ b/src/services/agent-action-executor.ts @@ -180,9 +180,15 @@ async function performAction(env: Env, ctx: AgentActionExecutionContext, action: case "approve": await createPullRequestReview(env, ctx.installationId, ctx.repoFullName, ctx.pullNumber, "APPROVE", action.reviewBody ?? ""); return; - case "merge": - await mergePullRequest(env, ctx.installationId, ctx.repoFullName, ctx.pullNumber, { mergeMethod: action.mergeMethod ?? "squash", ...(ctx.headSha ? { sha: ctx.headSha } : {}) }); + case "merge": { + // Pin the merge to the REVIEWED head (action.expectedHeadSha) when present — for an approval-queue replay + // this is the commit the maintainer reviewed, not necessarily the current head, so a force-push after + // staging fails safe with a 409 (→ terminal hold) instead of merging un-reviewed code. A live sweep plans + // expectedHeadSha == ctx.headSha, so its behavior is unchanged; the fallback covers any unpinned plan. + const mergeSha = action.expectedHeadSha ?? ctx.headSha; + await mergePullRequest(env, ctx.installationId, ctx.repoFullName, ctx.pullNumber, { mergeMethod: action.mergeMethod ?? "squash", ...(mergeSha ? { sha: mergeSha } : {}) }); return; + } case "close": if (action.closeComment) await createIssueComment(env, ctx.installationId, ctx.repoFullName, ctx.pullNumber, action.closeComment); await closePullRequest(env, ctx.installationId, ctx.repoFullName, ctx.pullNumber); diff --git a/src/services/agent-approval-queue.ts b/src/services/agent-approval-queue.ts index 97dede42e0..a391b5c106 100644 --- a/src/services/agent-approval-queue.ts +++ b/src/services/agent-approval-queue.ts @@ -36,6 +36,25 @@ export async function decidePendingAgentAction(env: Env, input: { id: string; de getPullRequest(env, pending.repoFullName, pending.pullNumber), getInstallation(env, pending.installationId), ]); + + // Re-validate the staged action against the LIVE head before executing. A staged merge records the reviewed + // head (expectedHeadSha); if the contributor force-pushed after staging, the live head has moved and replaying + // the action would act on un-reviewed code. Refuse, supersede the sticky row, and record it. This is the + // application-level fail-safe; the executor additionally pins the GitHub merge to the reviewed SHA as a backstop. + const stagedHead = pending.params.expectedHeadSha; + if (stagedHead && pr?.headSha && stagedHead !== pr.headSha) { + await setPendingAgentActionStatus(env, pending.id, { status: "rejected", decidedBy: input.decidedBy }); + await recordAuditEvent(env, { + eventType: "agent.pending_action.superseded", + actor: input.decidedBy, + targetKey, + outcome: "denied", + detail: `superseded ${pending.actionClass}: staged head ${stagedHead.slice(0, 12)} no longer matches live head ${pr.headSha.slice(0, 12)} (force-push after staging)`, + metadata: { ...baseMetadata, stagedHeadSha: stagedHead, liveHeadSha: pr.headSha }, + }); + return { status: "rejected", action: { ...pending, status: "rejected", decidedBy: input.decidedBy }, executionOutcome: "head_moved" }; + } + const outcomes = await executeAgentMaintenanceActions( env, { diff --git a/src/settings/agent-actions.ts b/src/settings/agent-actions.ts index 3e97420d1e..76755c325c 100644 --- a/src/settings/agent-actions.ts +++ b/src/settings/agent-actions.ts @@ -414,6 +414,11 @@ export function planAgentMaintenanceActions(input: AgentActionPlanInput): Planne requiresApproval: approval("merge"), reason: `gate passed, CI green, mergeable, ${autoMaintain.requireApprovals} approval(s) satisfied`, mergeMethod: autoMaintain.mergeMethod, + // Pin the merge to the EXACT reviewed head. For an `auto_with_approval` stage this travels into the pending + // row (actionParams persists expectedHeadSha), so a force-push after staging can never be merged: the + // executor pins GitHub's merge `sha` to this commit → a moved head yields a 409 (terminal hold) instead of + // merging un-reviewed code. A live sweep sets this == ctx.headSha, so its behavior is unchanged. + ...(input.pr.headSha ? { expectedHeadSha: input.pr.headSha } : {}), }); } else if (willClose) { // Contributor PR that is NOT review-good (gate blockers / red / unverified CI) OR conflicts with base → diff --git a/test/unit/agent-action-executor.test.ts b/test/unit/agent-action-executor.test.ts index f9be0686d5..6a3eea3157 100644 --- a/test/unit/agent-action-executor.test.ts +++ b/test/unit/agent-action-executor.test.ts @@ -69,6 +69,15 @@ describe("executeAgentMaintenanceActions (#778 gate stack)", () => { expect((await auditFor(env, "merge"))?.outcome).toBe("completed"); }); + it("LIVE merge pins the GitHub merge to the action's reviewed head (expectedHeadSha) over the context head", async () => { + const env = createTestEnv({}); + // A staged merge replayed on accept carries the REVIEWED head. Even when ctx.headSha is a newer live head, + // the merge must pin to the reviewed commit so a force-pushed (un-reviewed) head can never be merged. + const pinnedMerge: PlannedAgentAction = { actionClass: "merge", requiresApproval: false, reason: "clean", mergeMethod: "squash", expectedHeadSha: "reviewed-sha" }; + await executeAgentMaintenanceActions(env, ctx({ headSha: "live-sha" }), [pinnedMerge]); + expect(mergePullRequest).toHaveBeenCalledWith(env, 123, "owner/repo", 7, { mergeMethod: "squash", sha: "reviewed-sha" }); + }); + it("LIVE label with labelOp=add + comment: adds the label AND posts the comment", async () => { const env = createTestEnv({}); const flag: PlannedAgentAction = { actionClass: "label", requiresApproval: false, reason: "flag", label: "gittensory:pending-closure", labelOp: "add", comment: "⚠️ flagged" }; diff --git a/test/unit/agent-actions.test.ts b/test/unit/agent-actions.test.ts index 808bf9e73b..e30a32f957 100644 --- a/test/unit/agent-actions.test.ts +++ b/test/unit/agent-actions.test.ts @@ -116,6 +116,11 @@ describe("planAgentMaintenanceActions (#778)", () => { expect(plan.find((a) => a.actionClass === "merge")).toMatchObject({ mergeMethod: "rebase" }); }); + it("pins the planned merge to the PR's reviewed head SHA so a staged merge cannot replay against a moved head", () => { + const plan = planAgentMaintenanceActions(input({ conclusion: "success", autonomy: { merge: "auto" }, autoMaintain: { requireApprovals: 0, mergeMethod: "squash" }, pr: { labels: [], mergeableState: "clean", headSha: "reviewed-abc" } })); + expect(plan.find((a) => a.actionClass === "merge")).toMatchObject({ mergeMethod: "squash", expectedHeadSha: "reviewed-abc" }); + }); + it("applies conservative defaults when autoMaintain / slopGateMinScore are omitted", () => { // no autoMaintain → requireApprovals defaults to 1 → a clean passing PR without APPROVED does NOT merge expect(classes(planAgentMaintenanceActions({ conclusion: "success", blockerTitles: [], autonomy: { merge: "auto" }, changedPaths: [], hardGuardrailGlobs: [], authorIsOwner: false, authorIsAutomationBot: false, ciState: "passed", pr: { labels: [], mergeableState: "clean" } }))).not.toContain("merge"); diff --git a/test/unit/agent-approval-queue.test.ts b/test/unit/agent-approval-queue.test.ts index 01e2bef1ad..e80d3243f4 100644 --- a/test/unit/agent-approval-queue.test.ts +++ b/test/unit/agent-approval-queue.test.ts @@ -113,6 +113,51 @@ describe("agent approval queue (#779)", () => { expect(audit).toMatchObject({ outcome: "completed", actor: "owner" }); }); + it("accept supersedes a staged merge when the live head moved after staging (force-push fail-safe)", async () => { + const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: "x" }); + await upsertRepositorySettings(env, { repoFullName: "owner/repo", autonomy: { merge: "auto_with_approval" } }); + await seedInstallation(env); + // The PR head is now h-NEW: the contributor force-pushed after the merge was staged against h-OLD. + await upsertPullRequestFromGitHub(env, "owner/repo", { number: 7, title: "PR", state: "open", user: { login: "contributor" }, head: { sha: "h-NEW" }, labels: [], body: "x" }); + const { action } = await createPendingAgentActionIfAbsent(env, { repoFullName: "owner/repo", pullNumber: 7, installationId: 5, actionClass: "merge", autonomyLevel: "auto_with_approval", params: { mergeMethod: "squash", expectedHeadSha: "h-OLD" }, reason: "clean" }); + + const result = await decidePendingAgentAction(env, { id: action.id, decision: "accept", decidedBy: "owner" }); + expect(result.status).toBe("rejected"); + expect(result.executionOutcome).toBe("head_moved"); + expect(mergePullRequest).not.toHaveBeenCalled(); + expect((await getPendingAgentAction(env, action.id))?.status).toBe("rejected"); + const audit = await env.DB.prepare("select outcome, detail from audit_events where event_type = ?").bind("agent.pending_action.superseded").first<{ outcome: string; detail: string }>(); + expect(audit?.outcome).toBe("denied"); + expect(audit?.detail).toContain("force-push after staging"); + }); + + it("accept executes a staged merge when the staged head still matches the live head (pinned to the reviewed SHA)", async () => { + const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: "x" }); + await upsertRepositorySettings(env, { repoFullName: "owner/repo", autonomy: { merge: "auto_with_approval" } }); + await seedInstallation(env); + await upsertPullRequestFromGitHub(env, "owner/repo", { number: 7, title: "PR", state: "open", user: { login: "contributor" }, head: { sha: "h7" }, labels: [], body: "x" }); + const { action } = await createPendingAgentActionIfAbsent(env, { repoFullName: "owner/repo", pullNumber: 7, installationId: 5, actionClass: "merge", autonomyLevel: "auto_with_approval", params: { mergeMethod: "squash", expectedHeadSha: "h7" }, reason: "clean" }); + + const result = await decidePendingAgentAction(env, { id: action.id, decision: "accept", decidedBy: "owner" }); + expect(result.status).toBe("accepted"); + expect(result.executionOutcome).toBe("completed"); + // Pinned to the REVIEWED head from the staged params — not merely whatever the current head happens to be. + expect(mergePullRequest).toHaveBeenCalledWith(env, 5, "owner/repo", 7, { mergeMethod: "squash", sha: "h7" }); + }); + + it("accept does not supersede when the PR record is absent (no live head to compare) — proceeds to the executor", async () => { + const env = createTestEnv({}); + // No PR seeded → getPullRequest returns null → pr?.headSha is undefined, so the staleness guard is skipped + // even though the staged action carries an expectedHeadSha. No settings/install → the merge denies downstream. + const { action } = await createPendingAgentActionIfAbsent(env, { repoFullName: "owner/repo", pullNumber: 7, installationId: 5, actionClass: "merge", autonomyLevel: "auto_with_approval", params: { mergeMethod: "squash", expectedHeadSha: "h-OLD" }, reason: "clean" }); + const result = await decidePendingAgentAction(env, { id: action.id, decision: "accept", decidedBy: "owner" }); + expect(result.status).toBe("accepted"); + expect(result.executionOutcome).toBe("denied"); + expect(mergePullRequest).not.toHaveBeenCalled(); + const superseded = await env.DB.prepare("select count(*) as n from audit_events where event_type = ?").bind("agent.pending_action.superseded").first<{ n: number }>(); + expect(superseded?.n).toBe(0); + }); + it("accept honors current dry-run setting instead of forcing a live mutation", async () => { const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: "x" }); await upsertRepositorySettings(env, { repoFullName: "owner/repo", autonomy: { merge: "auto_with_approval" }, agentDryRun: true });