diff --git a/src/db/repositories.ts b/src/db/repositories.ts index 075b52118..a3bc1a986 100644 --- a/src/db/repositories.ts +++ b/src/db/repositories.ts @@ -6613,6 +6613,36 @@ export async function listStaleActiveReviewTracking( * * Returns the terminalized (repo, PR) pairs so the caller can log one line per healed row. */ +/** + * Everything an orphan re-queue needs that the active_review_tracking row does not carry (#9870): the + * installation the repo is registered under, and the PR's own GitHub creation time. + * + * `prCreatedAt` is not optional politeness -- omitting it INVERTS the queue order. jobClaimSortKey falls back + * to LEGACY_AGENT_REGATE_SORT_BASE_MS + prNumber (~9.5e11), which sorts ahead of every real 2026 PR (~1.78e12), + * so a re-queued job would jump the entire contributor backlog. A restart-orphaned review deserves to be + * re-driven, not prioritised over work that has been waiting longer. + * + * Returns null when the repo is not registered -- the caller logs a skip rather than enqueueing a job that + * cannot act. + */ +export async function loadOrphanRequeueContext( + env: Env, + repoFullName: string, + pullNumber: number, +): Promise<{ installationId: number; prCreatedAt: string | null } | null> { + const db = getDb(env.DB); + const bounded = boundedString(repoFullName, 200); + const repoRow = await db.select({ installationId: repositories.installationId }).from(repositories).where(eq(repositories.fullName, bounded)).limit(1); + const installationId = repoRow[0]?.installationId ?? null; + if (typeof installationId !== "number" || !Number.isFinite(installationId)) return null; + const prRow = await db + .select({ createdAt: pullRequests.createdAt }) + .from(pullRequests) + .where(and(eq(pullRequests.repoFullName, bounded), eq(pullRequests.number, pullNumber))) + .limit(1); + return { installationId, prCreatedAt: prRow[0]?.createdAt ?? null }; +} + export async function terminalizeActiveReviewsFromBeforeBoot( env: Env, bootIso: string, diff --git a/src/server.ts b/src/server.ts index 8a88d8b72..335606bc9 100644 --- a/src/server.ts +++ b/src/server.ts @@ -890,9 +890,63 @@ async function main(): Promise { // #deploy-orphaned-reviews: heal rows this very restart orphaned, BEFORE any webhook can bounce off them. // Fail-safe: a failure here must never block boot -- the 10-minute reconciliation sweep remains the backstop. try { - const { terminalizeActiveReviewsFromBeforeBoot } = await import("./db/repositories"); + const { terminalizeActiveReviewsFromBeforeBoot, loadOrphanRequeueContext } = await import("./db/repositories"); const healed = await terminalizeActiveReviewsFromBeforeBoot(env, new Date().toISOString()); for (const row of healed) { + // #9870: healing the ROW is only half of it. The interrupted pass had already published the + // "LoopOver is reviewing..." placeholder comment, and terminalizing its tracking row does not replace + // that comment -- it only stops the next pass bouncing off a stale lock. Nothing else re-drives the PR + // either: the head has not changed, so no webhook fires, and the published review cache is empty + // because the pass never finished. The PR therefore sits claiming a review is in progress FOREVER. + // + // Observed on JSONbored/metagraphed#8693: three container recreates in one afternoon (two of them + // routine config reloads) each killed a mid-flight review, and the PR showed "reviewing..." with zero + // published reviews and zero queued work until a human noticed. + // + // So re-drive it. `force` bypasses the AI-review cache and the reuse cooldown, which is correct here: + // the interrupted pass produced no usable result to reuse, and the one-shot cadence would otherwise + // reuse a stale published review (or nothing at all) rather than actually re-reviewing. + try { + const context = await loadOrphanRequeueContext(env, row.repoFullName, row.pullNumber); + if (context !== null) { + await backend.queue.binding.send({ + type: "agent-regate-pr", + deliveryId: `boot-orphan-requeue:${row.repoFullName}#${row.pullNumber}`, + repoFullName: row.repoFullName, + prNumber: row.pullNumber, + installationId: context.installationId, + // #9499: carries the PR's real creation time so this takes its NATURAL place in the oldest-first + // drain. Omitting it would sort this job ahead of every real PR -- a restart-orphaned review + // deserves to be re-driven, not to jump the contributor backlog. + prCreatedAt: context.prCreatedAt, + force: true, + } as never); + } else { + // A tracking row whose repo is no longer registered cannot be re-driven; say so rather than + // silently skipping, since the placeholder on that PR will stay until someone acts. + console.warn( + JSON.stringify({ + level: "warn", + event: "active_review_boot_orphan_requeue_skipped", + repo: row.repoFullName, + pr: row.pullNumber, + reason: "no installation id for repo", + }), + ); + } + } catch (error) { + // Never let a re-queue failure abort the sweep: the remaining rows still need healing, and a healed + // row with no re-queue is strictly better than a wedged one. + console.error( + JSON.stringify({ + level: "error", + event: "active_review_boot_orphan_requeue_failed", + repo: row.repoFullName, + pr: row.pullNumber, + message: String(error).slice(0, 200), + }), + ); + } console.warn( JSON.stringify({ level: "warn", diff --git a/test/unit/db-persistence.test.ts b/test/unit/db-persistence.test.ts index bc9c37051..cccee9718 100644 --- a/test/unit/db-persistence.test.ts +++ b/test/unit/db-persistence.test.ts @@ -16,6 +16,9 @@ import { persistRepoGithubTotalsSnapshot, persistSignalSnapshot, startActiveReviewTracking, + loadOrphanRequeueContext, + upsertPullRequestFromGitHub, + upsertRepositoryFromGitHub, terminalizeActiveReviewsFromBeforeBoot, terminalizeActiveReviewTracking, updateUpstreamDriftReportIssue, @@ -602,6 +605,45 @@ describe("active-review tracking (#review-evasion-protection)", () => { }); }); +describe("loadOrphanRequeueContext (#9870)", () => { + it("returns the installation id AND the PR's real creation time", async () => { + // The boot sweep heals the tracking row, but re-queueing the pass needs both — and the tracking row + // carries neither. Without this the placeholder comment stays published forever. + const env = createTestEnv(); + await upsertRepositoryFromGitHub(env, { full_name: "owner/repo", name: "repo", id: 1, private: false } as never, 4242); + await upsertPullRequestFromGitHub(env, "owner/repo", { + number: 7, title: "t", state: "open", user: { login: "c" }, head: { sha: "s7" }, labels: [], created_at: "2026-07-05T10:00:00.000Z", + } as never); + expect(await loadOrphanRequeueContext(env, "owner/repo", 7)).toEqual({ installationId: 4242, prCreatedAt: "2026-07-05T10:00:00.000Z" }); + }); + + it("INVARIANT (#9499): prCreatedAt is carried so the re-queue does NOT jump the backlog", async () => { + // Omitting it inverts the order: jobClaimSortKey falls back to a ~9.5e11 base that sorts ahead of every + // real 2026 PR (~1.78e12). A restart-orphaned review should be re-driven, not prioritised over work + // that has waited longer. The repo-wide regate-sort-key check enforces the same rule at the call site. + const env = createTestEnv(); + await upsertRepositoryFromGitHub(env, { full_name: "owner/repo", name: "repo", id: 1, private: false } as never, 4242); + await upsertPullRequestFromGitHub(env, "owner/repo", { + number: 8, title: "t", state: "open", user: { login: "c" }, head: { sha: "s8" }, labels: [], created_at: "2026-07-06T09:00:00.000Z", + } as never); + const context = await loadOrphanRequeueContext(env, "owner/repo", 8); + expect(context?.prCreatedAt).toBe("2026-07-06T09:00:00.000Z"); + }); + + it("INVARIANT: an unregistered repo yields null rather than a bogus id", async () => { + // A wrong id would enqueue a job that cannot act; null lets the caller log a skip an operator can see. + const env = createTestEnv(); + expect(await loadOrphanRequeueContext(env, "owner/never-registered", 1)).toBeNull(); + }); + + it("a registered repo with no stored PR row still resolves, with a null creation time", async () => { + // Better to re-drive with an unknown position than not at all; the sort key falls back and the pass runs. + const env = createTestEnv(); + await upsertRepositoryFromGitHub(env, { full_name: "owner/repo", name: "repo", id: 1, private: false } as never, 4242); + expect(await loadOrphanRequeueContext(env, "owner/repo", 999)).toEqual({ installationId: 4242, prCreatedAt: null }); + }); +}); + describe("terminalizeActiveReviewsFromBeforeBoot (#deploy-orphaned-reviews)", () => { it("REGRESSION: terminalizes a row orphaned by a restart, so the head is not wedged for 15-25 minutes", async () => { // Observed live on the ORB (2026-07-29): a container recreation 3 seconds after a review pass started