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
30 changes: 30 additions & 0 deletions src/db/repositories.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
56 changes: 55 additions & 1 deletion src/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -890,9 +890,63 @@ async function main(): Promise<void> {
// #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",
Expand Down
42 changes: 42 additions & 0 deletions test/unit/db-persistence.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,9 @@ import {
persistRepoGithubTotalsSnapshot,
persistSignalSnapshot,
startActiveReviewTracking,
loadOrphanRequeueContext,
upsertPullRequestFromGitHub,
upsertRepositoryFromGitHub,
terminalizeActiveReviewsFromBeforeBoot,
terminalizeActiveReviewTracking,
updateUpstreamDriftReportIssue,
Expand Down Expand Up @@ -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
Expand Down