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
2 changes: 2 additions & 0 deletions src/queue/processors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1712,6 +1712,7 @@ async function maybeRunAgentMaintenance(
pr,
settings,
otherOpenPullRequests,
deliveryId: args.deliveryId,
gate,
liveFacts: args.liveFacts,
});
Expand All @@ -1731,6 +1732,7 @@ async function runAgentMaintenancePlanAndExecute(
pr: PullRequestRecord;
settings: RepositorySettings;
otherOpenPullRequests: PullRequestRecord[];
deliveryId: string;
gate: ReturnType<typeof evaluateGateCheck>;
liveFacts: LiveGithubFacts;
},
Expand Down
14 changes: 7 additions & 7 deletions test/unit/agent-action-executor.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ vi.mock("../../src/github/app", async (importOriginal) => ({
// deterministic; individual tests below override this to exercise the staleness-denial path.
vi.mock("../../src/github/backfill", async (importOriginal) => ({
...(await importOriginal<typeof import("../../src/github/backfill")>()),
fetchLiveCiAggregate: vi.fn(async () => ({ ciState: "passed" as const, hasPending: false, hasVisiblePending: false, failingDetails: [], nonRequiredFailingDetails: [] })),
fetchLiveCiAggregate: vi.fn(async () => ({ ciState: "passed" as const, hasPending: false, hasVisiblePending: false, failingDetails: [], nonRequiredFailingDetails: [], ciCompletenessWarning: null })),
}));

import { closePullRequest, createIssueComment, createPullRequestReview, dismissLatestBotApproval, mergePullRequest, updatePullRequestBranch } from "../../src/github/pr-actions";
Expand Down Expand Up @@ -185,7 +185,7 @@ describe("executeAgentMaintenanceActions (#778 gate stack)", () => {
it("LIVE heuristic close is denied when live CI has since turned green (#2128)", async () => {
const env = createTestEnv({});
const heuristicClose: PlannedAgentAction = { actionClass: "close", requiresApproval: false, reason: "CI failed", closeComment: "closing", closeKind: "heuristic" };
vi.mocked(fetchLiveCiAggregate).mockResolvedValueOnce({ ciState: "passed", hasPending: false, hasVisiblePending: false, failingDetails: [], nonRequiredFailingDetails: [] });
vi.mocked(fetchLiveCiAggregate).mockResolvedValueOnce({ ciState: "passed", hasPending: false, hasVisiblePending: false, failingDetails: [], nonRequiredFailingDetails: [], ciCompletenessWarning: null });
const outcomes = await executeAgentMaintenanceActions(env, ctx(), [heuristicClose]);
expect(outcomes[0]?.outcome).toBe("denied");
expect(outcomes[0]?.detail).toContain("CI state changed since planning (now: passed)");
Expand All @@ -195,7 +195,7 @@ describe("executeAgentMaintenanceActions (#778 gate stack)", () => {
it("LIVE heuristic close proceeds when live CI is still failing (#2128)", async () => {
const env = createTestEnv({});
const heuristicClose: PlannedAgentAction = { actionClass: "close", requiresApproval: false, reason: "CI failed", closeComment: "closing", closeKind: "heuristic" };
vi.mocked(fetchLiveCiAggregate).mockResolvedValueOnce({ ciState: "failed", hasPending: false, hasVisiblePending: false, failingDetails: [], nonRequiredFailingDetails: [] });
vi.mocked(fetchLiveCiAggregate).mockResolvedValueOnce({ ciState: "failed", hasPending: false, hasVisiblePending: false, failingDetails: [], nonRequiredFailingDetails: [], ciCompletenessWarning: null });
const outcomes = await executeAgentMaintenanceActions(env, ctx(), [heuristicClose]);
expect(outcomes[0]?.outcome).toBe("completed");
expect(closePullRequest).toHaveBeenCalledWith(env, 123, "owner/repo", 7);
Expand All @@ -210,7 +210,7 @@ describe("executeAgentMaintenanceActions (#778 gate stack)", () => {
const persisted = actionParams(heuristicClose);
const replayed = pendingActionToPlanned({ actionClass: "close", params: persisted, reason: heuristicClose.reason });
expect(replayed.closeKind).toBe("heuristic");
vi.mocked(fetchLiveCiAggregate).mockResolvedValueOnce({ ciState: "passed", hasPending: false, hasVisiblePending: false, failingDetails: [], nonRequiredFailingDetails: [] });
vi.mocked(fetchLiveCiAggregate).mockResolvedValueOnce({ ciState: "passed", hasPending: false, hasVisiblePending: false, failingDetails: [], nonRequiredFailingDetails: [], ciCompletenessWarning: null });
const outcomes = await executeAgentMaintenanceActions(env, ctx(), [replayed]);
expect(outcomes[0]?.outcome).toBe("denied");
expect(outcomes[0]?.detail).toContain("CI state changed since planning (now: passed)");
Expand All @@ -227,7 +227,7 @@ describe("executeAgentMaintenanceActions (#778 gate stack)", () => {

it("LIVE merge is denied when live CI has since turned failing (#2128)", async () => {
const env = createTestEnv({});
vi.mocked(fetchLiveCiAggregate).mockResolvedValueOnce({ ciState: "failed", hasPending: false, hasVisiblePending: false, failingDetails: [], nonRequiredFailingDetails: [] });
vi.mocked(fetchLiveCiAggregate).mockResolvedValueOnce({ ciState: "failed", hasPending: false, hasVisiblePending: false, failingDetails: [], nonRequiredFailingDetails: [], ciCompletenessWarning: null });
const outcomes = await executeAgentMaintenanceActions(env, ctx(), [merge]);
expect(outcomes[0]?.outcome).toBe("denied");
expect(outcomes[0]?.detail).toContain("live CI is no longer passing (now: failed)");
Expand All @@ -236,7 +236,7 @@ describe("executeAgentMaintenanceActions (#778 gate stack)", () => {

it("REGRESSION (#2364): LIVE merge is denied when live CI has since become pending, not just failed", async () => {
const env = createTestEnv({});
vi.mocked(fetchLiveCiAggregate).mockResolvedValueOnce({ ciState: "pending", hasPending: true, hasVisiblePending: true, failingDetails: [], nonRequiredFailingDetails: [] });
vi.mocked(fetchLiveCiAggregate).mockResolvedValueOnce({ ciState: "pending", hasPending: true, hasVisiblePending: true, failingDetails: [], nonRequiredFailingDetails: [], ciCompletenessWarning: null });
const outcomes = await executeAgentMaintenanceActions(env, ctx(), [merge]);
expect(outcomes[0]?.outcome).toBe("denied");
expect(outcomes[0]?.detail).toContain("live CI is no longer passing (now: pending)");
Expand All @@ -245,7 +245,7 @@ describe("executeAgentMaintenanceActions (#778 gate stack)", () => {

it("REGRESSION (#2364): LIVE merge is denied when live CI has since become unverified (unreadable), not just failed", async () => {
const env = createTestEnv({});
vi.mocked(fetchLiveCiAggregate).mockResolvedValueOnce({ ciState: "unverified", hasPending: false, hasVisiblePending: false, failingDetails: [], nonRequiredFailingDetails: [] });
vi.mocked(fetchLiveCiAggregate).mockResolvedValueOnce({ ciState: "unverified", hasPending: false, hasVisiblePending: false, failingDetails: [], nonRequiredFailingDetails: [], ciCompletenessWarning: null });
const outcomes = await executeAgentMaintenanceActions(env, ctx(), [merge]);
expect(outcomes[0]?.outcome).toBe("denied");
expect(outcomes[0]?.detail).toContain("live CI is no longer passing (now: unverified)");
Expand Down
8 changes: 4 additions & 4 deletions test/unit/agent-approval-queue.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ vi.mock("../../src/github/app", async (importOriginal) => ({
// override these to exercise the staleness-supersede / staleness-denial paths.
vi.mock("../../src/github/backfill", async (importOriginal) => ({
...(await importOriginal<typeof import("../../src/github/backfill")>()),
fetchLiveCiAggregate: vi.fn(async () => ({ ciState: "passed" as const, hasPending: false, hasVisiblePending: false, failingDetails: [], nonRequiredFailingDetails: [] })),
fetchLiveCiAggregate: vi.fn(async () => ({ ciState: "passed" as const, hasPending: false, hasVisiblePending: false, failingDetails: [], nonRequiredFailingDetails: [], ciCompletenessWarning: null })),
fetchLivePullRequestMergeState: vi.fn(async () => "clean"),
fetchLivePullRequestReviewDecision: vi.fn(async () => undefined),
}));
Expand Down Expand Up @@ -247,7 +247,7 @@ describe("agent approval queue (#779)", () => {
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" });
vi.mocked(fetchLiveCiAggregate).mockResolvedValueOnce({ ciState: "failed", hasPending: false, hasVisiblePending: false, failingDetails: [], nonRequiredFailingDetails: [] });
vi.mocked(fetchLiveCiAggregate).mockResolvedValueOnce({ ciState: "failed", hasPending: false, hasVisiblePending: false, failingDetails: [], nonRequiredFailingDetails: [], ciCompletenessWarning: null });
// Also exercise a best-effort-failed mergeable/review read (undefined) alongside the CI failure — the
// audit metadata's nullish fallback must not throw, and ciState alone is still sufficient to deny.
vi.mocked(fetchLivePullRequestMergeState).mockResolvedValueOnce(undefined);
Expand All @@ -269,7 +269,7 @@ describe("agent approval queue (#779)", () => {
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" });
// A FULFILLED "pending" read is a genuine non-passing signal — distinct from a REJECTED read (fail-open,
// covered by the "ITSELF rejects" test below), which must NOT supersede.
vi.mocked(fetchLiveCiAggregate).mockResolvedValueOnce({ ciState: "pending", hasPending: true, hasVisiblePending: true, failingDetails: [], nonRequiredFailingDetails: [] });
vi.mocked(fetchLiveCiAggregate).mockResolvedValueOnce({ ciState: "pending", hasPending: true, hasVisiblePending: true, failingDetails: [], nonRequiredFailingDetails: [], ciCompletenessWarning: null });

const result = await decidePendingAgentAction(env, { id: action.id, decision: "accept", decidedBy: "owner" });
expect(result.status).toBe("rejected");
Expand Down Expand Up @@ -423,7 +423,7 @@ describe("agent approval queue (#779)", () => {
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" });
vi.mocked(fetchLivePullRequestMergeState).mockRejectedValueOnce(new Error("GitHub API transient 502"));
vi.mocked(fetchLiveCiAggregate).mockResolvedValueOnce({ ciState: "failed", hasPending: false, hasVisiblePending: false, failingDetails: [], nonRequiredFailingDetails: [] });
vi.mocked(fetchLiveCiAggregate).mockResolvedValueOnce({ ciState: "failed", hasPending: false, hasVisiblePending: false, failingDetails: [], nonRequiredFailingDetails: [], ciCompletenessWarning: null });

const result = await decidePendingAgentAction(env, { id: action.id, decision: "accept", decidedBy: "owner" });
expect(result.status).toBe("rejected");
Expand Down
6 changes: 5 additions & 1 deletion test/unit/queue.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1368,9 +1368,13 @@ describe("queue processors", () => {
try {
await processJob(env, { type: "agent-regate-pr", deliveryId: "ci-completeness-unverified", repoFullName: "owner/agent-repo", prNumber: 7, installationId: 9001 });

const audit = await env.DB.prepare("select outcome, detail from audit_events where event_type = ?").bind("github_app.ci_completeness_unverified").first<{ outcome: string; detail: string }>();
const audit = await env.DB.prepare("select outcome, detail, metadata_json from audit_events where event_type = ?").bind("github_app.ci_completeness_unverified").first<{ outcome: string; detail: string; metadata_json: string }>();
expect(audit?.outcome).toBe("completed"); // informational only — never a denial, never changes the disposition
expect(audit?.detail).toContain("branch-protection required checks");
// REGRESSION: deliveryId must actually thread through from maybeRunAgentMaintenance's args down into
// runAgentMaintenancePlanAndExecute — a prior version referenced args.deliveryId on a type that never
// declared or received the field (a typecheck break that slipped past CI on the commit that added #2137).
expect(JSON.parse(audit?.metadata_json ?? "{}")).toMatchObject({ deliveryId: "ci-completeness-unverified" });
} finally {
liveCiSpy.mockRestore();
requiredContextsSpy.mockRestore();
Expand Down
Loading