diff --git a/src/queue/processors.ts b/src/queue/processors.ts index 385fd80027..df8df255b4 100644 --- a/src/queue/processors.ts +++ b/src/queue/processors.ts @@ -1151,6 +1151,10 @@ export function hasVerifiedRequiredContexts( return requiredContexts != null && requiredContexts.size > 0; } +export function agentMaintenanceHeadMatchesGate(reviewedHeadSha: string | null | undefined, currentHeadSha: string | null | undefined): boolean { + return reviewedHeadSha == null || currentHeadSha == null || currentHeadSha === reviewedHeadSha; +} + /** * #778 maintainer auto-maintain trigger. After the gate runs on a PR webhook, if the repo opted the agent in * (an acting autonomy level), reuse the CANONICAL verdict produced by the full gate evaluation, plan the @@ -1186,6 +1190,10 @@ async function maybeRunAgentMaintenance( /* v8 ignore next -- defensive: the PR was upserted earlier in this same webhook, so it is always present. */ if (!pr) return; if (pr.state !== "open") return; + // The gate verdict belongs to the webhook/re-review head that produced it. Under concurrent self-host queues, a + // newer synchronize can advance the stored row before this job acts; fail closed rather than pairing a stale + // passing gate with a newer, unreviewed head for CI, planning, or merge execution. + if (!agentMaintenanceHeadMatchesGate(args.pr.headSha, pr.headSha)) return; // Drafts are work-in-progress: never auto-approve / merge / close / label a draft. Symmetric with the re-gate // sweep, which drops drafts (agent-sweep.ts). A draft signals "not ready"; the agent acts once it is marked // ready_for_review (which re-triggers this path on the now-undrafted PR). The converted_to_draft draft-dodge diff --git a/test/unit/queue.test.ts b/test/unit/queue.test.ts index 0447fea8e3..5406b76b09 100644 --- a/test/unit/queue.test.ts +++ b/test/unit/queue.test.ts @@ -39,7 +39,7 @@ import { upsertRepositoryFromGitHub, putCachedAiReview, } from "../../src/db/repositories"; -import { changedPathsForGuardrail, processJob } from "../../src/queue/processors"; +import { agentMaintenanceHeadMatchesGate, changedPathsForGuardrail, processJob } from "../../src/queue/processors"; import { upsertRepoFocusManifest } from "../../src/signals/focus-manifest-loader"; import { normalizeRegistryPayload } from "../../src/registry/normalize"; import { persistRegistrySnapshot } from "../../src/registry/sync"; @@ -6944,6 +6944,82 @@ describe("changedPathsForGuardrail", () => { }); }); +describe("agentMaintenanceHeadMatchesGate", () => { + it("allows maintenance only when the stored PR head still matches the reviewed gate head", () => { + expect(agentMaintenanceHeadMatchesGate("reviewed", "reviewed")).toBe(true); + expect(agentMaintenanceHeadMatchesGate("reviewed", "new-unreviewed")).toBe(false); + }); + + it("keeps legacy no-SHA paths fail-open because no exact reviewed head can be pinned", () => { + expect(agentMaintenanceHeadMatchesGate(undefined, "current")).toBe(true); + expect(agentMaintenanceHeadMatchesGate("reviewed", null)).toBe(true); + }); + + it("REGRESSION (#stale-head): a newer synchronize that advances the stored head before maintenance acts blocks the stale-gate auto-merge", async () => { + const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem() }); + await upsertInstallation(env, { + action: "created", + installation: { + id: 123, + account: { login: "JSONbored", id: 1, type: "User" }, + target_type: "User", + repository_selection: "all", + permissions: { metadata: "read", pull_requests: "write", issues: "write" }, + events: ["pull_request"], + }, + repositories: [{ name: "gittensory", full_name: "JSONbored/gittensory", private: false, owner: { login: "JSONbored" } }], + }); + // Clean, mergeable, approved, green CI + merge:auto + approve:auto + close:auto — this PR WOULD be auto-acted. + // The ONLY thing that must stop it is the stale-head guard in maybeRunAgentMaintenance. + await upsertRepositorySettings(env, { + repoFullName: "JSONbored/gittensory", + commentMode: "off", + publicSurface: "off", + autoLabelEnabled: false, + checkRunMode: "off", + gateCheckMode: "enabled", + autonomy: { merge: "auto", approve: "auto", close: "auto" }, + }); + vi.stubGlobal("fetch", async (input: RequestInfo | URL) => { + const url = input.toString(); + if (url === "https://api.gittensor.io/miners") return Response.json([]); + if (url.includes("/access_tokens")) return Response.json({ token: "installation-token" }); + if (url.includes("/commits/stale1/check-runs")) return Response.json({ total_count: 0, check_runs: [] }); + if (url.includes("/commits/stale1/status")) return Response.json({ statuses: [] }); + if (url.includes("/check-runs")) return Response.json({ id: 902 }, { status: 201 }); + return new Response("not found", { status: 404 }); + }); + + // Simulate the concurrent-queue race the guard defends against: the gate evaluated head "stale1", but by the + // time maintenance re-reads the persisted row a newer `synchronize` has advanced the stored head to "newer2". + // maybeRunAgentMaintenance re-reads via getPullRequest, so divert that read to the advanced head. + const realGetPullRequest = repositoriesModule.getPullRequest; + const spy = vi.spyOn(repositoriesModule, "getPullRequest").mockImplementation(async (...callArgs) => { + const row = await realGetPullRequest(...callArgs); + return row ? { ...row, headSha: "newer2" } : row; + }); + try { + await processJob(env, { + type: "github-webhook", + deliveryId: "stale-head-no-maintenance", + eventName: "pull_request", + payload: { + action: "opened", + installation: { id: 123, account: { login: "JSONbored", id: 1, type: "User" } }, + repository: { name: "gittensory", full_name: "JSONbored/gittensory", private: false, owner: { login: "JSONbored" } }, + pull_request: { number: 71, title: "Clean PR", state: "open", user: { login: "contributor" }, head: { sha: "stale1" }, labels: [], body: "Closes #1", mergeable_state: "clean", reviewDecision: "APPROVED" }, + }, + }); + } finally { + spy.mockRestore(); + } + + // No terminal maintenance action of ANY class fires: the gate verdict belonged to the now-stale head. + const acted = await env.DB.prepare("select count(*) as n from audit_events where event_type in ('agent.action.merge','agent.action.approve','agent.action.close')").first<{ n: number }>(); + expect(acted?.n).toBe(0); + }); +}); + describe("one-shot reopen prevention", () => { beforeEach(() => { clearInstallationTokenCacheForTest();