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
8 changes: 8 additions & 0 deletions src/queue/processors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
78 changes: 77 additions & 1 deletion test/unit/queue.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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();
Expand Down
Loading