diff --git a/src/queue/processors.ts b/src/queue/processors.ts index 9f50092797..51e7386caf 100644 --- a/src/queue/processors.ts +++ b/src/queue/processors.ts @@ -1356,6 +1356,25 @@ async function refreshOpenPullRequestsForScheduledSweep( }); } +// #orb-retry-storm: outage-repair priority (below) deliberately bypasses the normal staleness throttle +// (priorityBypassesFreshness) so a PR missing its current-head gate check gets re-repaired on every ~2-minute +// sweep tick instead of waiting out the ordinary cadence. That is correct for a transient blip, but +// surfaceRepairPriorityPullNumbers has no memory of prior attempts -- if the repair keeps failing for the SAME +// head SHA (e.g. every AI-provider attempt times out), it would otherwise re-select that PR forever, burning a +// fresh review attempt every cycle for zero output. These two constants cap that: once a SHA has already had +// REGATE_REPAIR_MAX_ATTEMPTS_PER_SHA dispatches recorded, it drops back to ordinary staleness-gated candidacy +// (still eventually re-checked, just not on every tick) and a single REGATE_REPAIR_EXHAUSTED_EVENT_TYPE audit +// event is recorded so the stuck PR is visible instead of silently retried forever. A new commit changes the +// head SHA, which resets the count naturally (the target key is scoped to repo+PR+SHA). +const REGATE_REPAIR_ATTEMPT_EVENT_TYPE = "agent.sweep.regate.repair_attempt"; +const REGATE_REPAIR_EXHAUSTED_EVENT_TYPE = "agent.sweep.regate.repair_exhausted"; +const REGATE_REPAIR_MAX_ATTEMPTS_PER_SHA = 3; +const REGATE_REPAIR_ATTEMPT_LOOKBACK_MS = 24 * 60 * 60 * 1000; + +function regateRepairTargetKey(repoFullName: string, prNumber: number, headSha: string): string { + return `${repoFullName}#${prNumber}#${headSha}`; +} + async function surfaceRepairPriorityPullNumbers( env: Env, repoFullName: string, @@ -1367,20 +1386,55 @@ async function surfaceRepairPriorityPullNumbers( if (pr.headSha && pr.lastPublishedSurfaceSha !== pr.headSha) priorityPullNumbers.add(pr.number); } - if (!gateCheckEnabled) return [...priorityPullNumbers]; + if (gateCheckEnabled) { + await Promise.all( + pulls.map(async (pr) => { + if (!pr.headSha) return; + const checks = await listCheckSummaries(env, repoFullName, pr.number).catch( + () => [], + ); + const currentGateCheck = checks.find( + (check) => + check.name === GITTENSORY_GATE_CHECK_NAME && + check.headSha === pr.headSha && + check.status === "completed", + ); + if (!currentGateCheck) priorityPullNumbers.add(pr.number); + }), + ); + } + const sinceIso = new Date(Date.now() - REGATE_REPAIR_ATTEMPT_LOOKBACK_MS).toISOString(); await Promise.all( - pulls.map(async (pr) => { - if (!pr.headSha) return; - const checks = await listCheckSummaries(env, repoFullName, pr.number).catch( - () => [], + [...priorityPullNumbers].map(async (prNumber) => { + const pr = pulls.find((candidate) => candidate.number === prNumber); + /* v8 ignore next -- priorityPullNumbers is only ever populated (both loops above) from a `pr` in `pulls` that already had a truthy headSha, so this lookup always succeeds with one; the guard only satisfies Array#find's `| undefined` return type. */ + if (!pr?.headSha) return; + const targetKey = regateRepairTargetKey(repoFullName, pr.number, pr.headSha); + const attempts = await countRecentAuditEventsForActorAndTarget( + env, + "gittensory", + REGATE_REPAIR_ATTEMPT_EVENT_TYPE, + targetKey, + sinceIso, ); - const currentGateCheck = checks.find( - (check) => - check.name === GITTENSORY_GATE_CHECK_NAME && - check.headSha === pr.headSha && - check.status === "completed", + if (attempts < REGATE_REPAIR_MAX_ATTEMPTS_PER_SHA) return; + priorityPullNumbers.delete(prNumber); + const alreadyFlagged = await countRecentAuditEventsForActorAndTarget( + env, + "gittensory", + REGATE_REPAIR_EXHAUSTED_EVENT_TYPE, + targetKey, + sinceIso, ); - if (!currentGateCheck) priorityPullNumbers.add(pr.number); + if (alreadyFlagged > 0) return; + await recordAuditEvent(env, { + eventType: REGATE_REPAIR_EXHAUSTED_EVENT_TYPE, + actor: "gittensory", + targetKey, + outcome: "denied", + detail: `re-gate repair exhausted after ${attempts} attempt(s) for the same head SHA; falling back to ordinary staleness cadence`, + metadata: { repoFullName, prNumber: pr.number, headSha: pr.headSha, attempts }, + }); }), ); return [...priorityPullNumbers]; @@ -1687,10 +1741,11 @@ async function sweepRepoRegate( // candidates at once (#audit-sweep-fanout). The cheap verdict summary above is computed inline and recorded // below, preserving the advisory audit. The convergence marker was already stamped for every candidate at // dispatch (above); with no installation to act with there is simply no re-review to fan out (audit-only). + const isPriorityRepair = priorityPullNumberSet.has(pr.number); if (sweepInstallationId != null) { const job: JobMessage = { type: "agent-regate-pr", - deliveryId: priorityPullNumberSet.has(pr.number) + deliveryId: isPriorityRepair ? `regate-repair:${repoFullName}#${pr.number}` : `regate-sweep:${repoFullName}#${pr.number}`, repoFullName, @@ -1702,6 +1757,19 @@ async function sweepRepoRegate( await (delaySeconds > 0 ? env.JOBS.send(job, { delaySeconds }) : env.JOBS.send(job)); + // #orb-retry-storm: record every priority-repair dispatch so surfaceRepairPriorityPullNumbers can cap + // how many times the SAME head SHA gets bounced back through this bypass (see its own comment above). + /* v8 ignore next -- isPriorityRepair is only true for a pr.number surfaceRepairPriorityPullNumbers added to priorityPullNumbers, which requires that same pr to have a truthy headSha; `&& pr.headSha` only satisfies its optional TS type. */ + if (isPriorityRepair && pr.headSha) { + await recordAuditEvent(env, { + eventType: REGATE_REPAIR_ATTEMPT_EVENT_TYPE, + actor: "gittensory", + targetKey: regateRepairTargetKey(repoFullName, pr.number, pr.headSha), + outcome: "queued", + detail: `outage-repair re-review dispatched for ${repoFullName}#${pr.number}`, + metadata: { repoFullName, prNumber: pr.number, headSha: pr.headSha }, + }); + } } } await recordAuditEvent(env, { diff --git a/src/selfhost/ai.ts b/src/selfhost/ai.ts index 4475dd49db..403f93fabd 100644 --- a/src/selfhost/ai.ts +++ b/src/selfhost/ai.ts @@ -148,7 +148,16 @@ export function resolveCodexEffort(configured: string | undefined): string { // old fixed 120s cap silently SIGKILLed a large max-effort review mid-generation (the review then degrades to // nothing). These scale the ceiling with the provider-specific effort dial; provider-specific timeout vars override // them outright. -const EFFORT_TIMEOUT_MS: Record = { low: 120_000, medium: 120_000, high: 240_000, xhigh: 360_000, max: 600_000 }; +// +// `medium` was left pinned to `low`'s 120s when this ladder was introduced (#3612-era), on the assumption that +// capping it tightly would conserve subscription tokens. In production it did the opposite: `medium` is the +// DEFAULT effort (see resolveEffort/resolveCodexEffort above), so a real medium-effort review that runs past 120s +// gets SIGKILLed mid-generation — the tokens already spent are wasted, and because that PR's head SHA never gets +// a completed gate check, the regate-repair sweep (queue/processors.ts's surfaceRepairPriorityPullNumbers) treats +// it as an outage and bypasses its own staleness throttle to retry it every ~2 minutes, indefinitely. Giving +// `medium` its own tier (rather than reusing `low`'s) lets a normal medium-effort review actually finish instead +// of feeding that loop. +const EFFORT_TIMEOUT_MS: Record = { low: 120_000, medium: 180_000, high: 240_000, xhigh: 360_000, max: 600_000 }; function resolveCliTimeoutFrom(configured: string | undefined, effort: string): number { const raw = Number(configured); diff --git a/test/unit/queue.test.ts b/test/unit/queue.test.ts index 055d8e66fa..e3583d7b58 100644 --- a/test/unit/queue.test.ts +++ b/test/unit/queue.test.ts @@ -6146,6 +6146,88 @@ describe("queue processors", () => { }); }); + it("REGRESSION (#orb-retry-storm): after MAX_ATTEMPTS repair dispatches for the SAME head SHA, the sweep stops bypassing freshness and records exactly one repair_exhausted audit event", async () => { + const sent: import("../../src/types").JobMessage[] = []; + const env = createTestEnv({ JOBS: { async send(m: import("../../src/types").JobMessage) { sent.push(m); } } as unknown as Queue }); + await upsertInstallation(env, { action: "created", installation: { id: 9407, account: { login: "owner", id: 1, type: "Organization" }, target_type: "Organization", repository_selection: "selected", permissions: {}, events: [] } }); + await upsertRepositoryFromGitHub(env, { name: "agent-repo", full_name: "owner/agent-repo", private: false, owner: { login: "owner" } }, 9407); + await upsertRepositorySettings(env, { repoFullName: "owner/agent-repo", autonomy: { merge: "auto" }, gateCheckMode: "enabled", checkRunMode: "off", commentMode: "off", publicSurface: "off" }); + // PR 1: missing its current Gate check for its current head -- would ordinarily be flagged outage-repair + // priority on every tick. Pre-seed 3 prior repair-attempt audit events for this EXACT head SHA to simulate a + // review that keeps failing (e.g. a timeout) and never publishes a completed gate check. + await upsertPullRequestFromGitHub(env, "owner/agent-repo", { number: 1, title: "Stuck repair", state: "open", user: { login: "c" }, head: { sha: "stuck-sha" }, labels: [], body: "" }); + await repositoriesModule.markPullRequestSurfacePublished(env, "owner/agent-repo", 1, "stuck-sha"); + const targetKey = "owner/agent-repo#1#stuck-sha"; + for (let i = 0; i < 3; i += 1) { + await repositoriesModule.recordAuditEvent(env, { + eventType: "agent.sweep.regate.repair_attempt", + actor: "gittensory", + targetKey, + outcome: "queued", + detail: "prior attempt", + metadata: {}, + }); + } + vi.setSystemTime(new Date("2026-05-28T02:00:00.000Z")); + + await processJob(env, { type: "agent-regate-sweep", requestedBy: "test", repoFullName: "owner/agent-repo" }); + + // No longer treated as priority repair -- either not fanned at all, or fanned as an ordinary "regate-sweep:" + // candidate, but never re-dispatched as "regate-repair:" once the same SHA has exhausted its attempt budget. + const fanned = sent.filter((job): job is Extract => job.type === "agent-regate-pr"); + expect(fanned.every((job) => job.deliveryId !== "regate-repair:owner/agent-repo#1")).toBe(true); + const exhausted = await env.DB.prepare("select count(*) as n from audit_events where event_type = ? and target_key = ?") + .bind("agent.sweep.regate.repair_exhausted", targetKey) + .first<{ n: number }>(); + expect(exhausted?.n).toBe(1); + // No further repair-attempt event was recorded for the exhausted SHA this tick. + const attempts = await env.DB.prepare("select count(*) as n from audit_events where event_type = ? and target_key = ?") + .bind("agent.sweep.regate.repair_attempt", targetKey) + .first<{ n: number }>(); + expect(attempts?.n).toBe(3); + }); + + it("REGRESSION (#orb-retry-storm): a repair dispatch under the attempt cap records a repair_attempt audit event, and a second sweep tick does not duplicate the repair_exhausted event once already flagged", async () => { + const sent: import("../../src/types").JobMessage[] = []; + const env = createTestEnv({ JOBS: { async send(m: import("../../src/types").JobMessage) { sent.push(m); } } as unknown as Queue }); + await upsertInstallation(env, { action: "created", installation: { id: 9408, account: { login: "owner", id: 1, type: "Organization" }, target_type: "Organization", repository_selection: "selected", permissions: {}, events: [] } }); + await upsertRepositoryFromGitHub(env, { name: "agent-repo", full_name: "owner/agent-repo", private: false, owner: { login: "owner" } }, 9408); + await upsertRepositorySettings(env, { repoFullName: "owner/agent-repo", autonomy: { merge: "auto" }, gateCheckMode: "enabled", checkRunMode: "off", commentMode: "off", publicSurface: "off" }); + await upsertPullRequestFromGitHub(env, "owner/agent-repo", { number: 2, title: "Fresh repair", state: "open", user: { login: "c" }, head: { sha: "fresh-sha" }, labels: [], body: "" }); + await repositoriesModule.markPullRequestSurfacePublished(env, "owner/agent-repo", 2, "fresh-sha"); + const targetKey = "owner/agent-repo#2#fresh-sha"; + vi.setSystemTime(new Date("2026-05-28T02:00:00.000Z")); + + await processJob(env, { type: "agent-regate-sweep", requestedBy: "test", repoFullName: "owner/agent-repo" }); + + const fanned = sent.filter((job): job is Extract => job.type === "agent-regate-pr"); + expect(fanned.map((job) => job.deliveryId)).toContain("regate-repair:owner/agent-repo#2"); + const attemptsAfterFirst = await env.DB.prepare("select count(*) as n from audit_events where event_type = ? and target_key = ?") + .bind("agent.sweep.regate.repair_attempt", targetKey) + .first<{ n: number }>(); + expect(attemptsAfterFirst?.n).toBe(1); + + // Manually push this SHA over the cap, then run the sweep twice more -- the exhausted event must be recorded + // only once even though the PR is (re-)evaluated on every tick. + for (let i = 0; i < 2; i += 1) { + await repositoriesModule.recordAuditEvent(env, { + eventType: "agent.sweep.regate.repair_attempt", + actor: "gittensory", + targetKey, + outcome: "queued", + detail: "prior attempt", + metadata: {}, + }); + } + await processJob(env, { type: "agent-regate-sweep", requestedBy: "test", repoFullName: "owner/agent-repo" }); + await processJob(env, { type: "agent-regate-sweep", requestedBy: "test", repoFullName: "owner/agent-repo" }); + + const exhausted = await env.DB.prepare("select count(*) as n from audit_events where event_type = ? and target_key = ?") + .bind("agent.sweep.regate.repair_exhausted", targetKey) + .first<{ n: number }>(); + expect(exhausted?.n).toBe(1); + }); + it("agent re-gate sweep fail-opens when current Gate check reads fail during repair priority selection", async () => { const sent: import("../../src/types").JobMessage[] = []; const env = createTestEnv({ JOBS: { async send(m: import("../../src/types").JobMessage) { sent.push(m); } } as unknown as Queue }); diff --git a/test/unit/selfhost-ai.test.ts b/test/unit/selfhost-ai.test.ts index bc5ee26c3f..c1486d261e 100644 --- a/test/unit/selfhost-ai.test.ts +++ b/test/unit/selfhost-ai.test.ts @@ -45,20 +45,20 @@ describe("resolveCodexEffort (#selfhost-effort — Codex reasoning effort, expli describe("provider-specific CLI timeouts (#selfhost — no shared timeout ambiguity)", () => { it("scales Claude timeout from CLAUDE_AI_EFFORT and honors CLAUDE_AI_TIMEOUT_MS", () => { expect(resolveClaudeCliTimeoutMs({ CLAUDE_AI_EFFORT: "low" })).toBe(120_000); - expect(resolveClaudeCliTimeoutMs({ CLAUDE_AI_EFFORT: "medium" })).toBe(120_000); + expect(resolveClaudeCliTimeoutMs({ CLAUDE_AI_EFFORT: "medium" })).toBe(180_000); expect(resolveClaudeCliTimeoutMs({ CLAUDE_AI_EFFORT: "high" })).toBe(240_000); expect(resolveClaudeCliTimeoutMs({ CLAUDE_AI_EFFORT: "xhigh" })).toBe(360_000); expect(resolveClaudeCliTimeoutMs({ CLAUDE_AI_EFFORT: "max" })).toBe(600_000); - expect(resolveClaudeCliTimeoutMs({})).toBe(120_000); + expect(resolveClaudeCliTimeoutMs({})).toBe(180_000); expect(resolveClaudeCliTimeoutMs({ CLAUDE_AI_TIMEOUT_MS: "300000", CLAUDE_AI_EFFORT: "low" })).toBe(300_000); }); it("scales Codex timeout from CODEX_AI_EFFORT and honors CODEX_AI_TIMEOUT_MS", () => { expect(resolveCodexCliTimeoutMs({ CODEX_AI_EFFORT: "low" })).toBe(120_000); - expect(resolveCodexCliTimeoutMs({ CODEX_AI_EFFORT: "medium" })).toBe(120_000); + expect(resolveCodexCliTimeoutMs({ CODEX_AI_EFFORT: "medium" })).toBe(180_000); expect(resolveCodexCliTimeoutMs({ CODEX_AI_EFFORT: "high" })).toBe(240_000); expect(resolveCodexCliTimeoutMs({ CODEX_AI_EFFORT: "xhigh" })).toBe(360_000); expect(resolveCodexCliTimeoutMs({ CODEX_AI_EFFORT: "max" })).toBe(360_000); - expect(resolveCodexCliTimeoutMs({})).toBe(120_000); + expect(resolveCodexCliTimeoutMs({})).toBe(180_000); expect(resolveCodexCliTimeoutMs({ CODEX_AI_TIMEOUT_MS: "1000" })).toBe(30_000); expect(resolveCodexCliTimeoutMs({ CODEX_AI_TIMEOUT_MS: "9999999" })).toBe(1_800_000); }); @@ -947,7 +947,7 @@ describe("subscription CLI helpers + fail-safe", () => { expect(seen[seen.indexOf("--model") + 1]).toBe("claude-sonnet-5"); expect(seen[seen.indexOf("--effort") + 1]).toBe("medium"); expect(seen).not.toContain("--append-system-prompt"); - expect(timeout).toBe(120_000); // medium → 120s by default to conserve fallback subscription tokens + expect(timeout).toBe(180_000); // medium → 180s: its own tier, distinct from low's 120s (#orb-retry-storm) // Provider-specific overrides flow through to the argv + timeout scale. await createClaudeCodeAi({ CLAUDE_CODE_OAUTH_TOKEN: "t", CLAUDE_AI_MODEL: "claude-opus-4-8", CLAUDE_AI_EFFORT: "max" }, cap).run("", { prompt: "x" }); expect(seen[seen.indexOf("--model") + 1]).toBe("claude-opus-4-8"); @@ -1334,9 +1334,9 @@ describe("subscription CLI helpers + fail-safe", () => { await expect( createCodexAi({ GITTENSORY_ENABLE_UNSAFE_CODEX_REVIEWER: "1" }, stalled, noAuthCheck).run("m", { prompt: "x" }), ).rejects.not.toThrow(/^codex_timeout/); - // The fast-fail deadline defaults to 30s and is strictly less than the (120s-default) full timeout. + // The fast-fail deadline defaults to 30s and is strictly less than the (180s-default) full timeout. expect(capturedOpts?.firstOutputTimeoutMs).toBe(30_000); - expect(capturedOpts?.timeoutMs).toBe(120_000); + expect(capturedOpts?.timeoutMs).toBe(180_000); expect(capturedOpts?.firstOutputTimeoutMs).toBeLessThan(capturedOpts!.timeoutMs); });