Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 8 additions & 2 deletions src/services/agent-action-executor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
19 changes: 19 additions & 0 deletions src/services/agent-approval-queue.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
{
Expand Down
5 changes: 5 additions & 0 deletions src/settings/agent-actions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 →
Expand Down
9 changes: 9 additions & 0 deletions test/unit/agent-action-executor.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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" };
Expand Down
5 changes: 5 additions & 0 deletions test/unit/agent-actions.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down
45 changes: 45 additions & 0 deletions test/unit/agent-approval-queue.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 });
Expand Down
Loading