From 81d0fbe02ab70c20a2c394d8fe6521490a193684 Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Tue, 28 Jul 2026 01:04:18 -0700 Subject: [PATCH] fix(queue): thread prCreatedAt through the trailing mergeable-state re-check, and guard the rest in CI (#9551) --- .github/workflows/ci.yml | 6 + package.json | 3 +- scripts/check-regate-sort-key.ts | 144 ++++++++++++++++++ src/queue/processors.ts | 11 +- .../unit/check-regate-sort-key-script.test.ts | 134 ++++++++++++++++ test/unit/queue-lifecycle-guards.test.ts | 100 ++++++++++++ test/unit/selfhost-queue-common.test.ts | 14 ++ 7 files changed, 409 insertions(+), 3 deletions(-) create mode 100644 scripts/check-regate-sort-key.ts create mode 100644 test/unit/check-regate-sort-key-script.test.ts diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index f31ee23de4..efa5584b33 100644 --- a/.github/workflows/ci.yml +++ b/.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 diff --git a/package.json b/package.json index 5a985977e0..b086f825b5 100644 --- a/package.json +++ b/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", diff --git a/scripts/check-regate-sort-key.ts b/scripts/check-regate-sort-key.ts new file mode 100644 index 0000000000..67587b1967 --- /dev/null +++ b/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 = /(? = 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; + } = {}, +): 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(); diff --git a/src/queue/processors.ts b/src/queue/processors.ts index 74ae0c8567..5b829396f3 100644 --- a/src/queue/processors.ts +++ b/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 { 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) { diff --git a/test/unit/check-regate-sort-key-script.test.ts b/test/unit/check-regate-sort-key-script.test.ts new file mode 100644 index 0000000000..aec7f8d672 --- /dev/null +++ b/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) { + 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([]); + }); +}); diff --git a/test/unit/queue-lifecycle-guards.test.ts b/test/unit/queue-lifecycle-guards.test.ts index 51b61643f3..c531d78aae 100644 --- a/test/unit/queue-lifecycle-guards.test.ts +++ b/test/unit/queue-lifecycle-guards.test.ts @@ -1,6 +1,7 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { generateKeyPairSync } from "node:crypto"; import { clearInstallationTokenCacheForTest } from "../../src/github/app"; +import { jobClaimSortKey } from "../../src/selfhost/queue-common"; import { clearReviewSuppressionCacheForTest } from "../../src/review/review-memory-wire"; import { PR_PANEL_COMMENT_MARKER } from "../../src/github/comments"; import * as backfillModule from "../../src/github/backfill"; @@ -5316,6 +5317,105 @@ describe("auto-action convergence: end-to-end plan+execute for the general heuri const trailing = scheduled.find((s) => s.message.type === "agent-regate-pr" && "prNumber" in s.message && s.message.prNumber === 63); expect(trailing).toBeDefined(); expect(trailing?.options).toEqual({ delaySeconds: 60 }); + // #9499 INVARIANT: this fixture's payload carries no `created_at`, so the stored row has none either and + // the producer degrades to the legacy sort base — exactly as before. The field is threaded, not forced: + // a caller without a timestamp must still enqueue rather than throw or send an empty value. + const trailingMessage = trailing?.message as { prCreatedAt?: string } | undefined; + expect(trailingMessage?.prCreatedAt).toBeUndefined(); + }); + + // #9499: the same producer WITH a real creation time — the case that was actually broken. Without the + // field, jobClaimSortKey falls back to LEGACY_AGENT_REGATE_SORT_BASE_MS + prNumber, which sorts this + // re-check AHEAD of every genuinely older contributor PR instead of behind them. That does not merely lose + // the oldest-first ordering, it inverts it for this producer's jobs. + it("REGRESSION (#9499): the trailing mergeable-state re-check carries the PR's createdAt, keeping its place in the oldest-first ordering", async () => { + const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem() }); + await setupAutoActionRepo(env, { autonomy: { merge: "auto", approve: "auto" }, linkedIssueGateMode: "off" }); + await upsertOfficialMinerDetection(env, "contributor", { status: "confirmed", snapshot: queueMinerSnapshot("contributor") }, 60_000); + const scheduled: Array<{ message: import("../../src/types").JobMessage; options?: { delaySeconds?: number } }> = []; + const originalSend = env.JOBS.send.bind(env.JOBS); + env.JOBS.send = (async (message: import("../../src/types").JobMessage, options?: { delaySeconds?: number }) => { + scheduled.push(options ? { message, options } : { message }); + return originalSend(message, options); + }) as typeof env.JOBS.send; + vi.stubGlobal("fetch", async (input: RequestInfo | URL, init?: RequestInit) => { + const url = input.toString(); + const method = init?.method ?? "GET"; + if (url === "https://api.gittensor.io/miners") return Response.json([]); + if (url === "https://api.github.com/graphql") return Response.json({ data: { repository: { pullRequest: { reviewDecision: "APPROVED" } } } }); + if (url.includes("/access_tokens")) return Response.json({ token: "installation-token" }); + if (url.includes("/pulls/65/files")) return Response.json([{ filename: "src/a.ts", status: "modified", additions: 1, deletions: 0, changes: 1, patch: "@@\n+export const ok = true;" }]); + if (url.includes("/pulls/65/reviews")) return Response.json([]); + if (url.includes("/pulls/65/commits")) return Response.json([]); + if (url.endsWith("/pulls/65/merge") && method === "PUT") return Response.json({ merged: true }); + if (url.endsWith("/pulls/65")) return Response.json({ number: 65, state: "open", user: { login: "contributor" }, head: { sha: "conv65" }, mergeable_state: "unknown" }); + if (url.includes("/commits/conv65/check-runs")) { + return Response.json({ total_count: 1, check_runs: [{ name: "CI", status: "completed", conclusion: "success", app: { slug: "github-actions" } }] }); + } + if (url.includes("/commits/conv65/status")) return Response.json({ state: "success", statuses: [] }); + if (url.includes("/issues/65/labels")) return Response.json([]); + if (url.includes("/issues/65/comments")) return Response.json([]); + return Response.json({}); + }); + + const createdAt = "2026-03-04T05:06:07Z"; + await processJob(env, { + type: "github-webhook", + deliveryId: "merge-race-created-at", + eventName: "pull_request", + payload: prPayload({ number: 65, head: { sha: "conv65" }, body: "Closes #1", action: "synchronize", created_at: createdAt }), + }); + + const trailing = scheduled.find((s) => s.message.type === "agent-regate-pr" && "prNumber" in s.message && s.message.prNumber === 65); + expect(trailing).toBeDefined(); + const message = trailing?.message as { prCreatedAt?: string }; + expect(message.prCreatedAt).toBe(createdAt); + // The property that actually matters: the job now sorts by the PR's real age, not the legacy base. + expect(jobClaimSortKey(JSON.stringify(trailing?.message), 0)).toBe(Date.parse(createdAt)); + }); + + it("INVARIANT (#9499): a failing enqueue of the trailing re-check never fails the webhook pass", async () => { + // Fail-open, matching every other trailing-schedule call site in this file: the re-check is a best-effort + // guarantee on top of the periodic sweep, so losing one must degrade to "the sweep gets it" rather than + // failing the pass that had already done its real work. + const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem() }); + await setupAutoActionRepo(env, { autonomy: { merge: "auto", approve: "auto" }, linkedIssueGateMode: "off" }); + await upsertOfficialMinerDetection(env, "contributor", { status: "confirmed", snapshot: queueMinerSnapshot("contributor") }, 60_000); + env.JOBS.send = (async (message: import("../../src/types").JobMessage) => { + if (message.type === "agent-regate-pr") throw new Error("queue unavailable"); + }) as unknown as typeof env.JOBS.send; + vi.stubGlobal("fetch", async (input: RequestInfo | URL, init?: RequestInit) => { + const url = input.toString(); + const method = init?.method ?? "GET"; + if (url === "https://api.gittensor.io/miners") return Response.json([]); + if (url === "https://api.github.com/graphql") return Response.json({ data: { repository: { pullRequest: { reviewDecision: "APPROVED" } } } }); + if (url.includes("/access_tokens")) return Response.json({ token: "installation-token" }); + if (url.includes("/pulls/66/files")) return Response.json([{ filename: "src/a.ts", status: "modified", additions: 1, deletions: 0, changes: 1, patch: "@@\n+export const ok = true;" }]); + if (url.includes("/pulls/66/reviews")) return Response.json([]); + if (url.includes("/pulls/66/commits")) return Response.json([]); + if (url.endsWith("/pulls/66/merge") && method === "PUT") return Response.json({ merged: true }); + if (url.endsWith("/pulls/66")) return Response.json({ number: 66, state: "open", user: { login: "contributor" }, head: { sha: "conv66" }, mergeable_state: "unknown" }); + if (url.includes("/commits/conv66/check-runs")) { + return Response.json({ total_count: 1, check_runs: [{ name: "CI", status: "completed", conclusion: "success", app: { slug: "github-actions" } }] }); + } + if (url.includes("/commits/conv66/status")) return Response.json({ state: "success", statuses: [] }); + if (url.includes("/issues/66/labels")) return Response.json([]); + if (url.includes("/issues/66/comments")) return Response.json([]); + return Response.json({}); + }); + + await expect( + processJob(env, { + type: "github-webhook", + deliveryId: "merge-race-enqueue-fails", + eventName: "pull_request", + payload: prPayload({ number: 66, head: { sha: "conv66" }, body: "Closes #1", action: "synchronize", created_at: "2026-03-04T05:06:07Z" }), + }), + ).resolves.not.toThrow(); + + // And the pass still recorded its real decision -- the enqueue failure changed nothing upstream of it. + const holdAudit = await env.DB.prepare("select detail from audit_events where event_type = 'agent.action.hold' order by created_at desc limit 1").first<{ detail: string }>(); + expect(holdAudit?.detail).toContain("mergeable_state is unknown"); }); it("REGRESSION (#merge-race): a real, stable non-clean mergeable_state (dirty) does NOT schedule a trailing re-check — only the transient \"unknown\" value does", async () => { diff --git a/test/unit/selfhost-queue-common.test.ts b/test/unit/selfhost-queue-common.test.ts index 9c4592087e..dfa454fc72 100644 --- a/test/unit/selfhost-queue-common.test.ts +++ b/test/unit/selfhost-queue-common.test.ts @@ -1071,6 +1071,20 @@ describe("self-host queue common helpers", () => { expect(jobCoalesceKey(payload({ type: "selftune", requestedBy: "schedule" }))).toBe("selftune"); }); + // #9499: the legacy fallback is not a neutral default — it actively INVERTS the ordering. This pins the + // magnitude relationship that makes an omitted prCreatedAt a bug rather than a lost optimization, and is + // the property scripts/check-regate-sort-key.ts exists to protect. + it("REGRESSION (#9499): the legacy fallback sorts AHEAD of every real PR, so an omitted prCreatedAt inverts the ordering", () => { + const withCreatedAt = jobClaimSortKey( + payload({ type: "agent-regate-pr", repoFullName: "owner/repo", prNumber: 1, prCreatedAt: "2026-01-01T00:00:00.000Z" }), + 999, + ); + // A very HIGH pr number on the fallback path — the most favourable case for the fallback — still sorts + // ahead of the OLDEST plausible real 2026 PR. + const withoutCreatedAt = jobClaimSortKey(payload({ type: "agent-regate-pr", repoFullName: "owner/repo", prNumber: 999_999 }), 999); + expect(withoutCreatedAt).toBeLessThan(withCreatedAt); + }); + it("orders per-PR re-gate jobs by GitHub PR creation time, with a deterministic legacy fallback", () => { expect( jobClaimSortKey(