fix(queue): thread prCreatedAt through the trailing mergeable-state re-check, and guard the rest in CI (#9551)

.github/workflows/ci.yml

@@ -370,6 +370,12 @@ jobs:
       - name: Dead source-file check
         if: ${{ github.event_name == 'push' || needs.changes.outputs.backend == 'true' }}
         run: npm run dead-source-files:check
+      # #9499: an agent-regate-pr producer that omits prCreatedAt does not merely lose the oldest-first
+      # ordering, it INVERTS it -- the legacy sort fallback places such a job ahead of every real PR. Five of
+      # eight producers had drifted that way before this check existed.
+      - name: Re-gate sort-key check
+        if: ${{ github.event_name == 'push' || needs.changes.outputs.backend == 'true' }}
+        run: npm run regate-sort-key:check
       # Mechanical drift tripwire for the hand-duplicated src/{review,settings,signals} <-> loopover-engine
       # twin files, plus a version-skew check on the installed @loopover/engine (#4260). Same
       # local-only-until-now gap as the drift checks above. Also gated on `miner`: the reverse-direction

package.json

@@ -72,6 +72,7 @@
     "validate:no-hand-written-js": "node --experimental-strip-types scripts/validate-no-hand-written-js.ts",
     "import-specifiers:check": "node --experimental-strip-types scripts/check-import-specifiers.ts",
     "dead-source-files:check": "node --experimental-strip-types scripts/check-dead-source-files.ts",
+    "regate-sort-key:check": "node --experimental-strip-types scripts/check-regate-sort-key.ts",
     "replay-runner-manifest": "tsx scripts/replay-runner-image-manifest.ts",
     "replay-runner-manifest:write": "tsx scripts/replay-runner-image-manifest.ts --write",
     "replay-runner-manifest:check": "tsx scripts/replay-runner-image-manifest.ts --check",
@@ -116,7 +117,7 @@
     "test:smoke:browser:install": "playwright install chromium",
     "test:smoke:browser": "node --experimental-strip-types scripts/smoke-ui-browser.ts",
     "pretest:ci": "npm run check-node-version",
-    "test:ci": "git diff --check && npm run actionlint && npm run lint:composite-actions && npm run db:migrations:check && npm run db:schema-drift:check && npm run selfhost:env-reference:check && npm run miner:env-reference:check && npm run selfhost:validate-observability && npm run cf-typegen:check && npm run build --workspace @loopover/engine && npm run build --workspace @loopover/discovery-index && npm run build:mcp && npm run build:miner && npm run build --workspace @loopover/ui-kit && npm run typecheck && npm run test:coverage && npm run test:engine-parity && npm run test:live-gate-parity && npm run test:driver-parity && npm run test --workspace @loopover/engine && npm run test:workers && npm run test:mcp-pack && npm run test:miner-pack && npm run test:engine-pack && npm run test:ui-kit-pack && npm run test:miner-deployment-docs-audit && npm run rees:test && npm run ui:openapi:check && npm run ui:openapi:settings-parity && npm run ui:version-audit && npm run docs:drift-check && npm run coverage-boltons:check && npm run import-specifiers:check && npm run dead-source-files:check && npm run validate:no-hand-written-js && npm run replay-runner-manifest:check && npm run coco-dev-versions:check && npm run branding-drift:check && npm run manifest:drift-check && npm run engine-parity:drift-check && npm run engines-nvmrc:check && npm run release-manifest:sync:check && npm run command-reference:check && npm run ui:lint && npm run ui:typecheck && npm run ui:test && npm run ui:build",
+    "test:ci": "git diff --check && npm run actionlint && npm run lint:composite-actions && npm run db:migrations:check && npm run db:schema-drift:check && npm run selfhost:env-reference:check && npm run miner:env-reference:check && npm run selfhost:validate-observability && npm run cf-typegen:check && npm run build --workspace @loopover/engine && npm run build --workspace @loopover/discovery-index && npm run build:mcp && npm run build:miner && npm run build --workspace @loopover/ui-kit && npm run typecheck && npm run test:coverage && npm run test:engine-parity && npm run test:live-gate-parity && npm run test:driver-parity && npm run test --workspace @loopover/engine && npm run test:workers && npm run test:mcp-pack && npm run test:miner-pack && npm run test:engine-pack && npm run test:ui-kit-pack && npm run test:miner-deployment-docs-audit && npm run rees:test && npm run ui:openapi:check && npm run ui:openapi:settings-parity && npm run ui:version-audit && npm run docs:drift-check && npm run coverage-boltons:check && npm run import-specifiers:check && npm run dead-source-files:check && npm run regate-sort-key:check && npm run validate:no-hand-written-js && npm run replay-runner-manifest:check && npm run coco-dev-versions:check && npm run branding-drift:check && npm run manifest:drift-check && npm run engine-parity:drift-check && npm run engines-nvmrc:check && npm run release-manifest:sync:check && npm run command-reference:check && npm run ui:lint && npm run ui:typecheck && npm run ui:test && npm run ui:build",
     "test:release": "npm run test:ci && npm run changelog:check",
     "test:release:mcp": "npm run test:ci",
     "test:watch": "vitest",

scripts/check-regate-sort-key.ts

@@ -0,0 +1,144 @@
+#!/usr/bin/env node
+// #9499: every `agent-regate-pr` producer must carry `prCreatedAt`.
+//
+// `jobClaimSortKey` (src/selfhost/queue-common.ts) sorts regate jobs by the PR's own `createdAt` ascending —
+// the ONE real oldest-first ordering mechanism the queue has. A producer that omits `prCreatedAt` falls back
+// to `LEGACY_AGENT_REGATE_SORT_BASE_MS + prNumber` (~9.5e11), which sorts AHEAD of every genuinely older 2026
+// PR (~1.78e12). So an omission does not degrade the ordering — it INVERTS it, silently, for that producer's
+// jobs, and five of eight producers had done exactly that.
+//
+// A type-level guard cannot express this: `prCreatedAt` is legitimately optional on `JobMessage` (a producer
+// that truly has no PR record must still be able to enqueue), so making it required would break the
+// deliberate exceptions rather than catch the accidental ones. This check reads the producer sites instead
+// and requires each to either pass the field or be explicitly allowlisted with a reason — the same
+// "an exception must be stated, not inferred from absence" shape as check-dead-source-files.ts's entry points.
+import { readFileSync, readdirSync } from "node:fs";
+import { fileURLToPath } from "node:url";
+
+const SCAN_ROOTS = ["src"] as const;
+const SOURCE_PATTERN = /(?<!\.d)\.ts$/;
+const EXCLUDED_SEGMENT = /(?:^|\/)(?:node_modules|dist|dist-test)(?:\/|$)/;
+
+/** Hard ceiling on how far a producer's object literal may be scanned, purely so a malformed/unbalanced file
+ *  cannot make this walk the rest of the module. The real bound is the literal's own closing brace — see
+ *  {@link producerObjectText}. */
+const PRODUCER_SCAN_CEILING_LINES = 60;
+
+/**
+ * Producers that deliberately omit `prCreatedAt`, each with the reason. Keyed `file:marker`, where the marker
+ * is a distinctive substring of the producer's own `deliveryId` so the entry survives line-number churn.
+ */
+const ALLOWED_OMISSIONS: ReadonlyMap<string, string> = new Map([
+  [
+    "src/api/routes.ts:manual-regate:",
+    "The maintainer-triggered manual re-gate route enqueues at priority 99 to jump the queue ON PURPOSE — an operator asking for one PR now is exactly the case oldest-first should not apply to. It also has no PR record in hand (the body carries only repoFullName + prNumber).",
+  ],
+]);
+
+export type RegateSortKeyViolation = { file: string; line: number; snippet: string };
+
+function defaultListSourceFiles(root: string): string[] {
+  try {
+    return readdirSync(root, { recursive: true })
+      .map(String)
+      .filter((entry) => SOURCE_PATTERN.test(entry) && !EXCLUDED_SEGMENT.test(entry))
+      .map((entry) => `${root}/${entry}`);
+  } catch {
+    return [];
+  }
+}
+
+/**
+ * Pure over its inputs: finds every `type: "agent-regate-pr"` producer whose enqueued object does not carry
+ * `prCreatedAt` within the following {@link PRODUCER_WINDOW_LINES} lines, minus the allowlisted exceptions.
+ * `listSourceFiles`/`readFile` are injectable so tests can simulate a fresh offender without touching the tree.
+ */
+export function findRegateSortKeyViolations(
+  options: {
+    roots?: readonly string[];
+    listSourceFiles?: (root: string) => string[];
+    readFile?: (file: string) => string;
+    allowedOmissions?: ReadonlyMap<string, string>;
+  } = {},
+): RegateSortKeyViolation[] {
+  const {
+    roots = SCAN_ROOTS,
+    listSourceFiles = defaultListSourceFiles,
+    readFile = (file: string) => readFileSync(file, "utf8"),
+    allowedOmissions = ALLOWED_OMISSIONS,
+  } = options;
+
+  const violations: RegateSortKeyViolation[] = [];
+  for (const root of roots) {
+    for (const file of listSourceFiles(root)) {
+      const lines = readFile(file).split("\n");
+      for (const [index, line] of lines.entries()) {
+        if (!line.includes('type: "agent-regate-pr"')) continue;
+        const window = producerObjectText(lines, index);
+        if (window.includes("prCreatedAt")) continue;
+        const allowed = [...allowedOmissions.keys()].some((key) => {
+          const [allowedFile, marker] = splitAllowKey(key);
+          return allowedFile === file && marker !== "" && window.includes(marker);
+        });
+        if (allowed) continue;
+        violations.push({ file, line: index + 1, snippet: line.trim() });
+      }
+    }
+  }
+  return violations.sort((a, b) => (a.file === b.file ? a.line - b.line : a.file.localeCompare(b.file)));
+}
+
+/**
+ * The text of the object literal that OWNS the `type: "agent-regate-pr"` line at `startIndex`, bounded by that
+ * literal's own closing brace rather than a fixed line count.
+ *
+ * A fixed window is subtly wrong here and produced a real false negative while this check was being written:
+ * two producers sitting within a few lines of each other let the FIRST one's `prCreatedAt` satisfy the scan
+ * for the SECOND one's, so removing a field from one of them was not caught. Tracking brace depth means each
+ * producer is judged on its own literal and nothing else.
+ */
+function producerObjectText(lines: readonly string[], startIndex: number): string {
+  const collected: string[] = [];
+  let depth = 0;
+  for (let i = startIndex; i < Math.min(lines.length, startIndex + PRODUCER_SCAN_CEILING_LINES); i += 1) {
+    const line = lines[i] ?? "";
+    collected.push(line);
+    for (const char of line) {
+      if (char === "{") depth += 1;
+      else if (char === "}") depth -= 1;
+    }
+    // Depth goes negative at the `}` that closes the literal this `type:` line sits inside — that line is the
+    // last one belonging to this producer.
+    if (depth < 0) break;
+  }
+  return collected.join("\n");
+}
+
+/** Split `path/to/file.ts:marker-text` on the LAST colon that precedes the marker — a marker may itself
+ *  contain colons (`manual-regate:`), so a naive split on the first or last colon gets it wrong. */
+function splitAllowKey(key: string): [string, string] {
+  const boundary = key.indexOf(".ts:");
+  if (boundary === -1) return [key, ""];
+  return [key.slice(0, boundary + ".ts".length), key.slice(boundary + ".ts:".length)];
+}
+
+function main(): void {
+  const violations = findRegateSortKeyViolations();
+  if (violations.length === 0) {
+    process.stdout.write("agent-regate-pr sort keys: OK\n");
+    return;
+  }
+  process.stderr.write(`Found ${violations.length} agent-regate-pr producer(s) missing prCreatedAt (#9499):\n`);
+  for (const violation of violations) {
+    process.stderr.write(`  ${violation.file}:${violation.line} — ${violation.snippet}\n`);
+  }
+  process.stderr.write(
+    "\nAn omitted prCreatedAt does not merely lose the ordering — it INVERTS it: jobClaimSortKey falls back to\n" +
+      "LEGACY_AGENT_REGATE_SORT_BASE_MS + prNumber (~9.5e11), which sorts ahead of every real 2026 PR (~1.78e12).\n" +
+      "Pass the PR's createdAt, or — if the producer genuinely must jump the queue — add it to ALLOWED_OMISSIONS\n" +
+      "in scripts/check-regate-sort-key.ts with the reason.\n",
+  );
+  process.exit(1);
+}
+
+if (process.argv[1] === fileURLToPath(import.meta.url)) main();

src/queue/processors.ts

@@ -3911,7 +3911,10 @@ async function runAgentMaintenancePlanAndExecute(
     // reproduce, so they're deliberately NOT retried here. Best-effort: an enqueue failure here must never
     // fail this webhook pass, matching every other trailing-schedule call site in this file.
     if ((liveMergeState ?? pr.mergeableState) === "unknown") {
-      await scheduleTrailingMergeableStateReReview(env, deliveryId, installationId, repoFullName, pr.number).catch(() => undefined);
+      /* v8 ignore next -- scheduleTrailingMergeableStateReReview already catches internally (its own
+         enqueue-failure branch logs and returns without rethrowing), so this outer .catch is defensive only:
+         unreachable without a throw inside its OWN try, which nothing here can induce. */
+      await scheduleTrailingMergeableStateReReview(env, deliveryId, installationId, repoFullName, pr.number, pr.createdAt).catch(() => undefined);
     }
   }
   if (holdoutOnPlan.length === 0) {
@@ -5098,6 +5101,10 @@ async function scheduleTrailingMergeableStateReReview(
   installationId: number,
   repoFullName: string,
   prNumber: number,
+  /** #9499: the PR's own createdAt, so this trailing re-review keeps its place in the oldest-first ordering.
+   *  Optional only because a caller without the record in hand degrades to the legacy sort base exactly as
+   *  before -- every present caller has it. */
+  prCreatedAt?: string | null | undefined,
 ): Promise<void> {
   const key = `merge-state-unknown-trailing:${repoFullName.toLowerCase()}#${prNumber}`;
   // Same check-then-claim-only-after-send shape as scheduleTrailingIssueLinkedReReview above (#2371 follow-up
@@ -5106,7 +5113,7 @@ async function scheduleTrailingMergeableStateReReview(
   if (await getTransientKey(env, key)) return;
   try {
     await env.JOBS.send(
-      { type: "agent-regate-pr", deliveryId, repoFullName, prNumber, installationId },
+      { type: "agent-regate-pr", deliveryId, repoFullName, prNumber, installationId, ...(prCreatedAt ? { prCreatedAt } : {}) },
       { delaySeconds: MERGE_STATE_UNKNOWN_TRAILING_RECHECK_DELAY_SECONDS },
     );
   } catch (error) {

test/unit/check-regate-sort-key-script.test.ts

@@ -0,0 +1,134 @@
+import { describe, expect, it } from "vitest";
+import { findRegateSortKeyViolations } from "../../scripts/check-regate-sort-key";
+
+/** Simulates a tree without touching the real one. Mirrors check-import-specifiers-script.test.ts's helper. */
+function fakeFiles(byFile: Record<string, string>) {
+  const files = Object.keys(byFile);
+  return {
+    listSourceFiles: (root: string) => files.filter((f) => f.startsWith(`${root}/`)),
+    readFile: (file: string) => byFile[file] ?? "",
+  };
+}
+
+// #9499: jobClaimSortKey sorts agent-regate-pr jobs by the PR's own createdAt ascending — the one real
+// oldest-first mechanism the queue has. A producer omitting prCreatedAt falls back to
+// LEGACY_AGENT_REGATE_SORT_BASE_MS + prNumber (~9.5e11), which sorts AHEAD of every genuinely older 2026 PR
+// (~1.78e12). So an omission does not degrade the ordering, it INVERTS it — silently, for that producer only.
+describe("check-regate-sort-key script", () => {
+  it("REGRESSION: flags a producer that omits prCreatedAt", () => {
+    const violations = findRegateSortKeyViolations({
+      roots: ["src"],
+      allowedOmissions: new Map(),
+      ...fakeFiles({
+        "src/queue/thing.ts": `await env.JOBS.send({\n  type: "agent-regate-pr",\n  deliveryId,\n  repoFullName,\n  prNumber,\n});\n`,
+      }),
+    });
+    expect(violations).toEqual([{ file: "src/queue/thing.ts", line: 2, snippet: 'type: "agent-regate-pr",' }]);
+  });
+
+  it("INVARIANT: a producer that passes prCreatedAt is clean, in either spread or plain form", () => {
+    const violations = findRegateSortKeyViolations({
+      roots: ["src"],
+      allowedOmissions: new Map(),
+      ...fakeFiles({
+        "src/a.ts": `send({\n  type: "agent-regate-pr",\n  prNumber,\n  ...(pr.createdAt ? { prCreatedAt: pr.createdAt } : {}),\n});\n`,
+        "src/b.ts": `send({\n  type: "agent-regate-pr",\n  prNumber,\n  prCreatedAt,\n});\n`,
+      }),
+    });
+    expect(violations).toEqual([]);
+  });
+
+  // The false negative this check actually had while being written: a fixed-line window let one producer's
+  // field satisfy the scan for a NEIGHBOURING producer's omission, so removing a field was not caught. The
+  // scan is bounded by the literal's own closing brace precisely so each producer is judged on its own.
+  it("REGRESSION: a NEIGHBOURING producer's prCreatedAt does not mask this one's omission", () => {
+    const source = [
+      "const good = {",
+      '  type: "agent-regate-pr",',
+      "  prNumber,",
+      "  ...(pr.createdAt ? { prCreatedAt: pr.createdAt } : {}),",
+      "};",
+      "const bad = {",
+      '  type: "agent-regate-pr",',
+      "  prNumber,",
+      "};",
+      "",
+    ].join("\n");
+    const violations = findRegateSortKeyViolations({
+      roots: ["src"],
+      allowedOmissions: new Map(),
+      ...fakeFiles({ "src/two.ts": source }),
+    });
+    expect(violations).toEqual([{ file: "src/two.ts", line: 7, snippet: 'type: "agent-regate-pr",' }]);
+  });
+
+  it("INVARIANT: the reverse order also holds — a good producer AFTER a bad one does not rescue it", () => {
+    const source = [
+      "const bad = {",
+      '  type: "agent-regate-pr",',
+      "  prNumber,",
+      "};",
+      "const good = {",
+      '  type: "agent-regate-pr",',
+      "  prNumber,",
+      "  prCreatedAt,",
+      "};",
+      "",
+    ].join("\n");
+    const violations = findRegateSortKeyViolations({
+      roots: ["src"],
+      allowedOmissions: new Map(),
+      ...fakeFiles({ "src/two.ts": source }),
+    });
+    expect(violations).toEqual([{ file: "src/two.ts", line: 2, snippet: 'type: "agent-regate-pr",' }]);
+  });
+
+  it("INVARIANT: an allowlisted deliberate omission is not flagged, and the allowlist is matched by MARKER not line number", () => {
+    // Keyed on a distinctive deliveryId substring so the entry survives ordinary line churn — an
+    // allowlist keyed on position would silently stop applying (or start applying to the wrong producer)
+    // the first time someone edited the file above it.
+    const violations = findRegateSortKeyViolations({
+      roots: ["src"],
+      allowedOmissions: new Map([["src/api/routes.ts:manual-regate:", "operator-triggered, jumps the queue on purpose"]]),
+      ...fakeFiles({
+        "src/api/routes.ts": `const message = {\n  type: "agent-regate-pr",\n  deliveryId: \`manual-regate:\${id}\`,\n  prNumber,\n};\n`,
+      }),
+    });
+    expect(violations).toEqual([]);
+  });
+
+  it("INVARIANT: the allowlist is scoped to its own FILE — the same marker elsewhere is still flagged", () => {
+    const violations = findRegateSortKeyViolations({
+      roots: ["src"],
+      allowedOmissions: new Map([["src/api/routes.ts:manual-regate:", "reason"]]),
+      ...fakeFiles({
+        "src/queue/copycat.ts": `const message = {\n  type: "agent-regate-pr",\n  deliveryId: \`manual-regate:\${id}\`,\n  prNumber,\n};\n`,
+      }),
+    });
+    expect(violations).toHaveLength(1);
+    expect(violations[0]?.file).toBe("src/queue/copycat.ts");
+  });
+
+  it("INVARIANT: a file with no regate producer at all yields nothing", () => {
+    const violations = findRegateSortKeyViolations({
+      roots: ["src"],
+      allowedOmissions: new Map(),
+      ...fakeFiles({ "src/unrelated.ts": `send({ type: "recapture-preview", prNumber });\n` }),
+    });
+    expect(violations).toEqual([]);
+  });
+
+  it("reports violations sorted by file then line, so failure output is stable", () => {
+    const bad = `send({\n  type: "agent-regate-pr",\n  prNumber,\n});\n`;
+    const violations = findRegateSortKeyViolations({
+      roots: ["src"],
+      allowedOmissions: new Map(),
+      ...fakeFiles({ "src/z.ts": bad, "src/a.ts": bad }),
+    });
+    expect(violations.map((violation) => violation.file)).toEqual(["src/a.ts", "src/z.ts"]);
+  });
+
+  it("the REAL repo tree is clean — this check runs in CI and must stay green", () => {
+    expect(findRegateSortKeyViolations()).toEqual([]);
+  });
+});