fix(review): stop the re-gate sweep from reordering PRs by repair status

src/settings/agent-sweep.ts

@@ -79,6 +79,17 @@ export type RegateSweepOrderMode = "staleness" | "oldest-first";
  * staleness key so continued periodic re-gating keeps converging instead of pinning the oldest PRs forever.
  * Selection-time only: real-time webhook-driven review is not gated by this sort and can still process any PR
  * out of order at any moment.
+ *
+ * `priorityPullNumbers` (outage repair -- surfaceRepairPriorityPullNumbers, processors.ts) affects ELIGIBILITY
+ * only, never final order (#selfhost-fifo-ordering): a repair candidate bypasses the freshness guard
+ * (`priorityBypassesFreshness`) and stays in the oldest-first pool even once it already has a `lastRegatedAt`
+ * stamp (`hasRepairPriority` in the pool filter below) -- but it is NOT sorted ahead of the rest of the queue.
+ * An earlier revision additionally sorted repair candidates first, which let a newer PR needing repair (e.g.
+ * opened during an extended pause, so it has never published anything) jump ahead of older PRs that merely
+ * went stale -- observed live as PRs dispatching out of their creation/staleness order ("spraying") whenever a
+ * repo had a mixed backlog of repaired and ordinary candidates. Every eligible PR -- repair or not -- is now
+ * ordered by the SAME `orderKey` (+ PR-number tiebreak), so a sweep processes its queue in one deterministic
+ * order every time, regardless of how many candidates happen to need repair.
  */
 export function selectRegateCandidates(input: {
   pulls: PullRequestRecord[];
@@ -125,8 +136,6 @@ export function selectRegateCandidates(input: {
     input.priorityPullNumbers instanceof Set
       ? input.priorityPullNumbers
       : new Set(input.priorityPullNumbers ?? []);
-  const repairPriority = (pr: PullRequestRecord): number =>
-    priorityPullNumbers.has(pr.number) ? 0 : 1;
   const eligible = input.pulls
     .filter((pr) => pr.state === "open" && !pr.isDraft)
     .filter((pr) => {
@@ -154,12 +163,7 @@ export function selectRegateCandidates(input: {
       ? creationOrder
       : regateProgress;
   return candidates
-    .sort(
-      (a, b) =>
-        repairPriority(a) - repairPriority(b) ||
-        orderKey(a) - orderKey(b) ||
-        a.number - b.number,
-    )
+    .sort((a, b) => orderKey(a) - orderKey(b) || a.number - b.number)
     .slice(0, Math.max(0, max));
 }
 

test/unit/agent-sweep.test.ts

@@ -142,7 +142,12 @@ describe("selectRegateCandidates (#777 re-gate sweep selection)", () => {
       expect(picked.map((p) => p.number)).toEqual([2, 3]); // stalest re-gate (600m), then 300m; 120m dropped by cap
     });
 
-    it("REGRESSION (repair priority): missing public surfaces are selected before ordinary stale PRs", () => {
+    it("#selfhost-fifo-ordering: a repair-flagged PR does NOT jump ahead of staler ordinary PRs — same orderKey for everyone", () => {
+      // #2 has a missing public surface (surfaceRepairPriorityPullNumbers would flag it) but is also the LEAST
+      // stale of the three by lastRegatedAt. An earlier revision sorted repair candidates first regardless of
+      // staleness — this pinned #2 ahead of #1/#3 and was observed live as PRs dispatching out of their
+      // creation/staleness order ("spraying") whenever a repo had a mixed repair/ordinary backlog. Repair status
+      // must only affect eligibility (see the freshness-bypass + oldest-first-pool tests below), never order.
       const pulls = [
         pr({ number: 1, lastRegatedAt: minutesAgo(900) }),
         pr({ number: 2, lastRegatedAt: minutesAgo(10) }),
@@ -154,7 +159,7 @@ describe("selectRegateCandidates (#777 re-gate sweep selection)", () => {
         max: 2,
         priorityPullNumbers: new Set([2]),
       });
-      expect(picked.map((p) => p.number)).toEqual([2, 1]);
+      expect(picked.map((p) => p.number)).toEqual([1, 3]); // stalest-by-regate first, same as with no priority set at all
     });
 
     it("REGRESSION (repair priority): priority repairs can bypass webhook freshness when the current Gate check is missing", () => {
@@ -328,7 +333,11 @@ describe("selectRegateCandidates (#777 re-gate sweep selection)", () => {
       expect(picked.map((p) => p.number)).toEqual([2, 3]); // oldest-created (600m), then 300m; 120m dropped by cap
     });
 
-    it("REGRESSION (repair priority): priority repairs still sort before ordinary oldest-first candidates", () => {
+    it("#selfhost-fifo-ordering: a repair-flagged PR does NOT jump ahead of older oldest-first candidates", () => {
+      // #1 is flagged as a repair (e.g. opened during an extended agent pause, so it never published anything)
+      // but is by far the NEWEST-created of the three. It stays eligible (see the initial-drain test below) but
+      // must not cut ahead of #2/#3, which were created long before it — creation order is the same for every
+      // candidate regardless of repair status.
       const pulls = [
         pr({ number: 1, createdAt: minutesAgo(10) }),
         pr({ number: 2, createdAt: minutesAgo(900) }),
@@ -341,12 +350,14 @@ describe("selectRegateCandidates (#777 re-gate sweep selection)", () => {
         max: 2,
         priorityPullNumbers: new Set([1]),
       });
-      expect(picked.map((p) => p.number)).toEqual([1, 2]); // #1 (priority) wins despite being newest-created
+      expect(picked.map((p) => p.number)).toEqual([2, 3]); // oldest-created first, same as with no priority set at all; #1 (newest) dropped by the cap
     });
 
     it("REGRESSION (repair priority): a priority repair stays eligible during the oldest-first initial drain", () => {
       // #1 is a priority repair AND has already been regated, while #2 is still in the never-regated initial
-      // drain. Priority work remains eligible so a repair can preempt the ordinary creation-order backlog.
+      // drain. Priority work remains ELIGIBLE (not excluded by the initial-drain pool narrowing just because it
+      // already has a regate stamp) — but, per #selfhost-fifo-ordering, it no longer preempts the ordinary
+      // creation-order backlog: #2 is older-created than #1, so #2 still sorts first.
       const pulls = [
         pr({
           number: 1,
@@ -362,7 +373,7 @@ describe("selectRegateCandidates (#777 re-gate sweep selection)", () => {
         priorityPullNumbers: new Set([1]),
         priorityBypassesFreshness: true,
       });
-      expect(picked.map((p) => p.number)).toEqual([1, 2]); // #1 (priority) included despite already having a regate stamp
+      expect(picked.map((p) => p.number)).toEqual([2, 1]); // #1 (priority) included despite already having a regate stamp, but #2 (older-created) still sorts first
     });
 
     it("a just-regated PR is excluded while any never-regated PR remains, not re-selected forever by fixed createdAt", () => {

test/unit/queue.test.ts

@@ -7245,7 +7245,7 @@ describe("queue processors", () => {
     expect(typeof after?.last_regated_at).toBe("string"); // stamped via a D1 write at dispatch — convergence does not need a GitHub write
   });
 
-  it("agent re-gate sweep prioritizes PRs missing the current Gate check even when their surface marker is current", async () => {
+  it("agent re-gate sweep processes strict staleness order even when a PR is missing its current Gate check (#selfhost-fifo-ordering)", async () => {
     const sent: import("../../src/types").JobMessage[] = [];
     const env = createTestEnv({ JOBS: { async send(m: import("../../src/types").JobMessage) { sent.push(m); } } as unknown as Queue });
     await upsertInstallation(env, { action: "created", installation: { id: 9400, account: { login: "owner", id: 1, type: "Organization" }, target_type: "Organization", repository_selection: "selected", permissions: {}, events: [] } });
@@ -7286,7 +7286,15 @@ describe("queue processors", () => {
     await processJob(env, { type: "agent-regate-sweep", requestedBy: "test", repoFullName: "owner/agent-repo" });
 
     const fanned = sent.filter((job) => job.type === "agent-regate-pr");
-    expect(fanned.map((job) => (job as Extract<import("../../src/types").JobMessage, { type: "agent-regate-pr" }>).prNumber)).toEqual([2, 1, 3]);
+    // PR2 is missing its current Gate check (surfaceRepairPriorityPullNumbers would flag it as a repair
+    // candidate) but is also the LEAST stale by lastRegatedAt (10 min ago vs. 23-25h for the others). An earlier
+    // revision sorted repair candidates first regardless of staleness, jumping PR2 to the front of this batch --
+    // that let a PR needing repair cut ahead of older PRs that merely went stale, observed live as PRs
+    // dispatching out of order ("spraying") whenever a repo had a mixed repair/ordinary backlog. Repair status
+    // now only affects ELIGIBILITY (staying in the pool, bypassing the freshness guard), never final order, so
+    // PR2 takes its rightful (last, since it's the freshest-regated) place and is dropped by the max:3 cap this
+    // round -- same as it would be with no repair flag at all.
+    expect(fanned.map((job) => (job as Extract<import("../../src/types").JobMessage, { type: "agent-regate-pr" }>).prNumber)).toEqual([1, 3, 4]);
   });
 
   it("REGRESSION (#3815): regateSweepOrderMode 'oldest-first' fans out per-PR jobs in creation order with a monotonic delaySeconds stagger", async () => {