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
4 changes: 4 additions & 0 deletions src/services/agent-action-executor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -234,6 +234,10 @@ export function actionParams(action: PlannedAgentAction): AgentPendingActionPara
...(action.mergeMethod !== undefined ? { mergeMethod: action.mergeMethod } : {}),
...(action.closeComment !== undefined ? { closeComment: action.closeComment } : {}),
...(action.expectedHeadSha !== undefined ? { expectedHeadSha: action.expectedHeadSha } : {}),
// Round-trip closeKind so a staged close's kind survives to accept-time — without it, the close-precision
// breaker's isHeuristicClose check (which matches on closeKind === "heuristic") could never fire for any
// staged close, silently defeating the breaker for the entire approval-queue accept path (#2127).
...(action.closeKind !== undefined ? { closeKind: action.closeKind } : {}),
};
}

Expand Down
71 changes: 69 additions & 2 deletions src/services/agent-approval-queue.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,11 @@
import { getInstallation, getPullRequest, getRepositorySettings, getPendingAgentAction, recordAuditEvent, setPendingAgentActionStatus } from "../db/repositories";
import { executeAgentMaintenanceActions, pendingActionToPlanned } from "./agent-action-executor";
import type { AgentPendingActionRecord } from "../types";
import { downgradeCloseToHold, downgradeMergeToHold, type PlannedAgentAction } from "../settings/agent-actions";
import { isCloseHoldOnly, isHoldOnly } from "../review/outcomes-wire";
import { createInstallationToken } from "../github/app";
import { fetchLiveCiAggregate, fetchLivePullRequestMergeState, fetchLivePullRequestReviewDecision } from "../github/backfill";
import { githubRateLimitAdmissionKeyForToken } from "../github/client";
import type { AgentPendingActionParams, AgentPendingActionRecord } from "../types";

export type ApprovalDecision = "accept" | "reject";

Expand Down Expand Up @@ -55,6 +60,68 @@ export async function decidePendingAgentAction(env: Env, input: { id: string; de
return { status: "rejected", action: { ...pending, status: "rejected", decidedBy: input.decidedBy }, executionOutcome: "head_moved" };
}

// Re-derive live justification for a staged MERGE at accept time. auto_with_approval rows have no expiry, so
// CI can flip red, the base can go dirty, or a reviewer can request changes while the row just sits waiting for
// a maintainer — none of which move the head SHA, so the check above alone would not catch it. Best-effort: a
// failed live read fails OPEN on that specific check (the executor's own mutation call independently needs a
// valid token/state and will fail cleanly if something is actually wrong). (#2126)
let liveParams: AgentPendingActionParams = pending.params;
if (pending.actionClass === "merge" && pr?.headSha) {
const token = await createInstallationToken(env, pending.installationId).catch(() => undefined);
const admissionKey = githubRateLimitAdmissionKeyForToken(env, token, pending.installationId);
// Promise.allSettled, not Promise.all: each live re-check is independently best-effort (per the comment
// above), so ONE transient rejection must fail open on that specific check, not throw the whole accept
// out of decidePendingAgentAction. A settled-rejected check is treated the same as "nothing concerning
// found" -- exactly what each function's own internal fail-safe catch already resolves to on success.
const [ciResult, mergeableResult, reviewResult] = await Promise.allSettled([
fetchLiveCiAggregate(env, pending.repoFullName, pr.headSha, token, undefined, admissionKey),
fetchLivePullRequestMergeState(env, pending.repoFullName, pending.pullNumber, token, admissionKey),
fetchLivePullRequestReviewDecision(env, pending.repoFullName, pending.pullNumber, token, admissionKey),
]);
// A REJECTED promise stays undefined (fail-open — the read itself failed, not a genuine CI signal); a
// FULFILLED promise reporting anything other than "passed" (failed, pending, or unverified) is a real,
// non-stale-tolerant signal that the staged merge's justification no longer holds (#2126).
const ciState = ciResult.status === "fulfilled" ? ciResult.value.ciState : undefined;
const mergeableState = mergeableResult.status === "fulfilled" ? mergeableResult.value : undefined;
const reviewDecision = reviewResult.status === "fulfilled" ? reviewResult.value : undefined;
const staleReason =
ciState !== undefined && ciState !== "passed"
? `live CI is no longer passing (now: ${ciState})`
: mergeableState === "dirty"
? "the base branch now conflicts (mergeable_state: dirty)"
: reviewDecision === "CHANGES_REQUESTED"
? "a reviewer has since requested changes"
: null;
if (staleReason) {
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}: ${staleReason} since staging`,
metadata: { ...baseMetadata, ciState: ciState ?? null, mergeableState: mergeableState ?? null, reviewDecision: reviewDecision ?? null },
});
return { status: "rejected", action: { ...pending, status: "rejected", decidedBy: input.decidedBy }, executionOutcome: "stale_disposition" };
}
// Re-sync the merge method to the CURRENT repo config, not the staging-time snapshot — the head-SHA pin
// above should stay frozen (that's the reviewed commit), but the merge method is a live preference with no
// reason to be frozen. (#2131)
/* v8 ignore next -- getRepositorySettings always resolves autoMaintain via its own default policy; this
* guard exists only because RepositorySettings' type allows autoMaintain to be undefined. */
if (settings.autoMaintain?.mergeMethod) {
liveParams = { ...pending.params, mergeMethod: settings.autoMaintain.mergeMethod };
}
}

// Re-apply the SAME merge/close precision circuit-breakers the live webhook path applies before executing, so
// a breaker engaged AFTER staging (an operator halting a runaway auto-merge, or the auto-tuner tripping on a
// precision drop) still holds this sticky pending row instead of executing it unmodified. (#2127)
const [holdOnly, closeHoldOnly] = await Promise.all([isHoldOnly(env, pending.repoFullName), isCloseHoldOnly(env, pending.repoFullName)]);
let plan: PlannedAgentAction[] = [pendingActionToPlanned({ actionClass: pending.actionClass, params: liveParams, reason: pending.reason })];
if (holdOnly) plan = downgradeMergeToHold(plan, true);
if (closeHoldOnly) plan = downgradeCloseToHold(plan, true);

const outcomes = await executeAgentMaintenanceActions(
env,
{
Expand All @@ -67,7 +134,7 @@ export async function decidePendingAgentAction(env: Env, input: { id: string; de
agentDryRun: settings.agentDryRun,
installationPermissions: installation ? installation.permissions : null,
},
[pendingActionToPlanned({ actionClass: pending.actionClass, params: pending.params, reason: pending.reason })],
plan,
);
/* v8 ignore next -- the executor returns one outcome per planned action, so the fallback is defensive. */
const execOutcome = outcomes[0]?.outcome ?? "no_outcome";
Expand Down
3 changes: 3 additions & 0 deletions src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -683,6 +683,9 @@ export type AgentPendingActionParams = {
mergeMethod?: AutoMergeMethod;
closeComment?: string;
expectedHeadSha?: string;
// WHICH kind of close this is (see PlannedAgentAction.closeKind) — must round-trip through staging so the
// close-precision circuit-breaker can still scope itself correctly when a staged close is later accepted (#2127).
closeKind?: "linked-issue-hard-rule" | "blacklist" | "heuristic";
};

export type AgentPendingActionStatus = "pending" | "accepted" | "rejected";
Expand Down
Loading
Loading