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
24 changes: 16 additions & 8 deletions src/queue/processors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3440,14 +3440,10 @@ async function prReadyForReview(
// Staleness cap: inferred or unreadable pending CI can otherwise defer FOREVER (orphaned required context,
// transiently unreadable pages, fork check that never reports). Past the cap we stop deferring and let the
// gate FINALIZE so the PR surfaces. A trusted required/base-repo visibly queued/in_progress CI signal is
// active CI, though, so never cut in front of it. A required context that never appeared in any page this
// fetch read to completion (hasMissingRequiredContext) has NO webhook to ever wait for — nothing fires
// check_run/check_suite "completed" for a context name that structurally never runs (a path-filtered
// workflow, a mistyped branch-protection context) — so it gets a much shorter cap than genuinely active or
// merely unreadable/non-required pending CI (#selfhost-ci-deferral-staleness). first-seen is tracked in the
// self-host Redis transient cache per PR+headSha (a new push = a fresh window, and the SAME key anchors
// both cap classes so a pending reason that changes class mid-window doesn't reset the clock); a cache
// miss degrades to the old defer. (#ci-stuck-finalize)
// active CI, though, so never cut in front of it. first-seen is tracked in the self-host Redis transient
// cache per PR+headSha (a new push = a fresh window, and the SAME key anchors both cap classes so a pending
// reason that changes class mid-window doesn't reset the clock); a cache miss degrades to the old defer.
// (#ci-stuck-finalize)
const deferCapMs = ci.hasMissingRequiredContext ? MISSING_REQUIRED_CONTEXT_DEFER_MS : STUCK_CI_DEFER_MS;
if (
ci.hasVisiblePending ||
Expand All @@ -3463,6 +3459,18 @@ async function prReadyForReview(
}).catch(() => undefined);
return false;
}
if (ci.hasMissingRequiredContext) {
await recordAuditEvent(env, {
eventType: "github_app.review_deferred_ci_pending",
actor: "gittensory",
targetKey: `${repoFullName}#${pr.number}`,
outcome: "queued",
detail:
"Required CI context is still missing — review deferred instead of publishing a passing gate before expected CI reports",
metadata: { deliveryId, repoFullName },
}).catch(() => undefined);
return false;
}
// #orb-ci-stuck-repeat: finalizing here runs a full paid AI review -- but a permanently-stuck CI context
// (a fork check that will never report, an orphaned required context) never resolves, so every later
// evaluation of the SAME head SHA hits this exact branch again and would re-spend another review for a
Expand Down
16 changes: 10 additions & 6 deletions test/unit/queue.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1859,16 +1859,16 @@ describe("queue processors", () => {
}
});

it("finalizes a missing-required-context PR within minutes, well before the old 30-minute stale-CI cap (#selfhost-ci-deferral-staleness)", async () => {
it("keeps deferring a missing-required-context PR after the short surfacing cap (#selfhost-ci-deferral-staleness)", async () => {
const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem() });
await upsertInstallation(env, { action: "created", installation: { id: 9001, account: { login: "owner", id: 1, type: "Organization" }, target_type: "Organization", repository_selection: "selected", permissions: { pull_requests: "write", checks: "write" }, events: [] } });
await upsertRepositoryFromGitHub(env, { name: "agent-repo", full_name: "owner/agent-repo", private: false, owner: { login: "owner" } }, 9001);
await upsertRepositorySettings(env, { repoFullName: "owner/agent-repo", autonomy: { merge: "auto", update_branch: "auto" }, aiReviewMode: "off", gatePack: "oss-anti-slop", gateCheckMode: "enabled", checkRunMode: "off", commentMode: "off", publicSurface: "off" });
await upsertPullRequestFromGitHub(env, "owner/agent-repo", { number: 7, title: "Missing required context, past short cap", state: "open", user: { login: "contributor" }, head: { sha: "a7" }, base: { ref: "main" }, labels: [], body: "Closes #1" });
vi.setSystemTime(new Date("2026-05-28T02:00:00.000Z"));
// 3 minutes elapsed: past the new 2-minute missing-required-context cap, but nowhere near the old 30-minute
// cap — this is the key regression proving #selfhost-ci-deferral-staleness actually shortens the wait
// instead of the PR sitting deferred for up to half an hour on a context that will never post.
// 3 minutes elapsed: past the 2-minute missing-required-context surfacing cap, but nowhere near the old
// 30-minute stale-CI cap. Missing required contexts must still not publish a passing gate before expected CI
// reports, because the review check may itself be branch-protection-required.
await env.SELFHOST_TRANSIENT_CACHE?.set(
"ci-pending-first-seen:owner/agent-repo#7:a7",
String(Date.now() - 3 * 60 * 1000),
Expand Down Expand Up @@ -1902,11 +1902,15 @@ describe("queue processors", () => {
try {
await processJob(env, { type: "agent-regate-pr", deliveryId: "missing-context-past-short-cap", repoFullName: "owner/agent-repo", prNumber: 7, installationId: 9001 });

expect(gateChecks).toBeGreaterThan(0);
expect(gateChecks).toBe(0);
const deferred = await env.DB.prepare("select count(*) as n from audit_events where event_type = ?")
.bind("github_app.review_deferred_ci_pending")
.first<{ n: number }>();
const finalized = await env.DB.prepare("select count(*) as n from audit_events where event_type = ?")
.bind("github_app.review_finalized_ci_stuck")
.first<{ n: number }>();
expect(finalized?.n).toBe(1);
expect(deferred?.n).toBe(1);
expect(finalized?.n).toBe(0);
} finally {
liveCiSpy.mockRestore();
requiredContextsSpy.mockRestore();
Expand Down