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
51 changes: 51 additions & 0 deletions src/db/repositories.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2942,6 +2942,57 @@ export async function countRecentAuditEventsForActorInRepoWithTargetSuffix(
return row.count;
}

// #orb-stale-recheck-priority: the THREE self-resolving "denied" detail strings the executor's live staleness
// rechecks can produce (agent-action-executor.ts's "8) Live ... re-verification" block -- duplicateStaleReason /
// mergeableStaleReason / threadStaleReason, each suffixed with " — action not executed" by that block's shared
// `audit("denied", ...)` call). Deliberately NOT imported from agent-action-executor.ts: that module imports
// FROM db/repositories.ts (installation tokens, PR records, ...), so importing back would create a real
// module-load cycle -- same hazard agent-actions.ts's CONCRETE_EVIDENCE_BLOCKER_CODES comment documents for the
// identical reason. A source-text parity test guards these three literals against producer-side drift instead.
// Deliberately EXCLUDES a CI-staleness denial (ciStaleReason): CI flipping already re-triggers a fresh
// evaluation via the check-run/status webhook that changed it, so it doesn't share the other three's "no
// webhook ever reaches this PR" gap.
const STALE_RECHECK_DENIAL_DETAIL_PATTERN =
/^(duplicate-cluster winner #\d+ is no longer open|the base-branch conflict that justified this close has since cleared|the review thread\(s\) that justified this close are now all resolved) — action not executed$/;

/**
* PR numbers within `repoFullName` whose most recent `agent.action.close`/`agent.action.merge` attempt was
* DENIED by one of the executor's live staleness rechecks (duplicate-cluster winner / base-conflict / review-
* thread) within `sinceIso`, rather than by a durable, externally-actioned reason (a manual-review label, a
* merge-train wait, contributor-cap contention, ...). Those rechecks exist precisely because the fact that
* justified the close/merge can flip WITHOUT a webhook ever notifying THIS pr -- a duplicate-cluster sibling
* merging fires a webhook about the SIBLING, not this PR, so nothing naturally re-triggers a look here. Callers
* (surfaceRepairPriorityPullNumbers) fold this into the SAME priority set the outage-repair path already uses,
* so a matching PR gets a fast, prioritized re-look instead of waiting out the ordinary sweep cadence -- and
* inherits that path's existing per-head-SHA attempt cap for free, so a genuinely stuck PR still falls back to
* ordinary cadence rather than being re-selected forever. Reuses the same literal-prefix `LIKE ... ESCAPE`
* scoping as {@link countRecentAuditEventsForActorInRepo} so a repo name containing a SQL wildcard is matched
* literally.
*/
export async function recentStaleRecheckDeniedPullNumbers(env: Env, repoFullName: string, sinceIso: string): Promise<number[]> {
const db = getDb(env.DB);
const targetPrefixPattern = `${escapeSqlLikePattern(repoFullName)}#%`;
const rows = await db
.select({ targetKey: auditEvents.targetKey, detail: auditEvents.detail })
.from(auditEvents)
.where(
and(
eq(auditEvents.actor, "loopover"),
inArray(auditEvents.eventType, ["agent.action.close", "agent.action.merge"]),
eq(auditEvents.outcome, "denied"),
sql`${auditEvents.targetKey} LIKE ${targetPrefixPattern} ESCAPE '\\'`,
gte(auditEvents.createdAt, sinceIso),
),
);
const pullNumbers = new Set<number>();
for (const row of rows) {
if (!row.detail || !STALE_RECHECK_DENIAL_DETAIL_PATTERN.test(row.detail)) continue;
const target = parsePullRequestTargetKey(row.targetKey);
if (target) pullNumbers.add(target.pullNumber);
}
return [...pullNumbers];
}

/** #orb-ci-stuck-repeat / #orb-retry-storm ops-alerts signal: the single PR within `repoFullName` that published
* the most review surfaces in the last `sinceIso`-bounded window, and how many. `github_app.pr_public_surface_
* published` is a genuine INSERT-only event (never upserted) recorded once per successful publish pass
Expand Down
21 changes: 19 additions & 2 deletions src/queue/processors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ import {
countRecentAuditEventsForActorAndTarget,
countRecentAuditEventsForActorInRepo,
countRecentAuditEventsForActorInRepoWithTargetSuffix,
recentStaleRecheckDeniedPullNumbers,
hasAuditEventForDelivery,
hasAuditEventForHeadSha,
recordGateBlockOutcome,
Expand Down Expand Up @@ -1123,7 +1124,7 @@ async function isRegateRepairExhausted(env: Env, repoFullName: string, pr: Pick<
return true;
}

async function surfaceRepairPriorityPullNumbers(
export async function surfaceRepairPriorityPullNumbers(
env: Env,
repoFullName: string,
pulls: readonly PullRequestRecord[],
Expand All @@ -1133,7 +1134,23 @@ async function surfaceRepairPriorityPullNumbers(
for (const pr of pulls) {
if (pr.headSha && pr.lastPublishedSurfaceSha !== pr.headSha)
priorityPullNumbers.add(pr.number);
// #orb-stale-recheck-priority: the bot already approved THIS exact commit (approvedHeadSha === headSha,
// set only once agentHoldAuditDetail's own gate-passed/CI-green/approvals-satisfied checks all cleared),
// yet the PR is still open with mergeable_state neither "clean" (would already have merged) nor "dirty"
// (a real conflict, which needs a human, not a fast recheck) -- i.e. GitHub is still computing
// mergeability. GitHub sends NO webhook when that computation finishes, so without this the PR waits out
// whatever the ordinary sweep cadence happens to be instead of the few seconds this actually takes
// (observed: green+approved PRs stuck OPEN, see fetchLivePullRequestMergeState's own doc comment).
if (pr.headSha && pr.approvedHeadSha === pr.headSha && pr.mergeableState && pr.mergeableState !== "clean" && pr.mergeableState !== "dirty")
priorityPullNumbers.add(pr.number);
}
// Scoped to `pulls` (not added unconditionally): a denial recorded against a PR that has SINCE closed/merged
// (so no longer appears in this open-PR list) must not resurrect a priority entry for it here — and doing so
// would violate the "every priorityPullNumbers entry has a pulls match with a truthy headSha" invariant the
// exhaustion-check loop below relies on.
const openPullNumbers = new Set(pulls.filter((pr) => pr.headSha).map((pr) => pr.number));
for (const prNumber of await recentStaleRecheckDeniedPullNumbers(env, repoFullName, new Date(Date.now() - REGATE_REPAIR_ATTEMPT_LOOKBACK_MS).toISOString()))
if (openPullNumbers.has(prNumber)) priorityPullNumbers.add(prNumber);
if (gateCheckEnabled) {
await Promise.all(
pulls.map(async (pr) => {
Expand All @@ -1154,7 +1171,7 @@ async function surfaceRepairPriorityPullNumbers(
await Promise.all(
[...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. */
/* v8 ignore next -- priorityPullNumbers is only ever populated (every loop above, including the openPullNumbers-scoped stale-recheck one) 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;
if (await isRegateRepairExhausted(env, repoFullName, pr)) priorityPullNumbers.delete(prNumber);
}),
Expand Down
120 changes: 120 additions & 0 deletions test/unit/db-parsers.test.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { readFileSync } from "node:fs";
import { afterEach, describe, expect, it, vi } from "vitest";
import {
claimMaintainerRecapPeriod,
Expand All @@ -8,6 +9,7 @@ import {
countRecentAuditEventsForActorAndTarget,
countRecentAuditEventsForActorInRepo,
countRecentAuditEventsForActorInRepoWithTargetSuffix,
recentStaleRecheckDeniedPullNumbers,
findHottestInconclusiveReviewTargetForRepo,
findHottestReviewTargetForRepo,
hasAuditEventForDelivery,
Expand Down Expand Up @@ -946,6 +948,124 @@ describe("database row parser hardening", () => {
expect(await countRecentAuditEventsForActorInRepoWithTargetSuffix(env, "chatty", "github_app.monitored_mention_ping", "owner/foo_bar", "mention:some_login", "2026-06-24T09:00:00.000Z")).toBe(1);
});

describe("recentStaleRecheckDeniedPullNumbers (#orb-stale-recheck-priority)", () => {
it("returns the PR number for a duplicate-cluster-winner staleness denial", async () => {
const env = createTestEnv();
await recordAuditEvent(env, {
eventType: "agent.action.close",
actor: "loopover",
targetKey: "owner/repo#5",
outcome: "denied",
detail: "duplicate-cluster winner #7437 is no longer open — action not executed",
createdAt: "2026-06-24T10:00:00.000Z",
});

expect(await recentStaleRecheckDeniedPullNumbers(env, "owner/repo", "2026-06-24T09:00:00.000Z")).toEqual([5]);
});

it("returns the PR number for a base-conflict or a review-thread staleness denial, and dedupes repeats for the same PR", async () => {
const env = createTestEnv();
await recordAuditEvent(env, {
eventType: "agent.action.close",
actor: "loopover",
targetKey: "owner/repo#6",
outcome: "denied",
detail: "the base-branch conflict that justified this close has since cleared — action not executed",
createdAt: "2026-06-24T10:00:00.000Z",
});
// A second denial for the SAME pr later in the window must not produce a duplicate entry.
await recordAuditEvent(env, {
eventType: "agent.action.merge",
actor: "loopover",
targetKey: "owner/repo#6",
outcome: "denied",
detail: "the review thread(s) that justified this close are now all resolved — action not executed",
createdAt: "2026-06-24T10:05:00.000Z",
});

expect(await recentStaleRecheckDeniedPullNumbers(env, "owner/repo", "2026-06-24T09:00:00.000Z")).toEqual([6]);
});

it("ignores a denial reason that is NOT one of the three self-resolving staleness reasons", async () => {
const env = createTestEnv();
// A manual-review-label denial is durable (needs a human to remove the label) -- retrying fast wastes a
// cycle for zero output, so it must not be surfaced here.
await recordAuditEvent(env, {
eventType: "agent.action.merge",
actor: "loopover",
targetKey: "owner/repo#7",
outcome: "denied",
detail: 'manual-review label "manual-review" is present on the live PR — merge not executed',
createdAt: "2026-06-24T10:00:00.000Z",
});
// A live-CI staleness denial already gets a fresh look from the check-run/status webhook that changed
// CI in the first place, so it's deliberately excluded too.
await recordAuditEvent(env, {
eventType: "agent.action.merge",
actor: "loopover",
targetKey: "owner/repo#8",
outcome: "denied",
detail: "live CI is no longer passing (now: pending) — action not executed",
createdAt: "2026-06-24T10:00:00.000Z",
});

expect(await recentStaleRecheckDeniedPullNumbers(env, "owner/repo", "2026-06-24T09:00:00.000Z")).toEqual([]);
});

it("ignores a matching denial outside the actor/eventType/outcome/repo/time scope", async () => {
const env = createTestEnv();
const matchingDetail = "duplicate-cluster winner #1 is no longer open — action not executed";
// Wrong actor.
await recordAuditEvent(env, { eventType: "agent.action.close", actor: "someone-else", targetKey: "owner/repo#10", outcome: "denied", detail: matchingDetail, createdAt: "2026-06-24T10:00:00.000Z" });
// Wrong event type (a "completed" close whose OWN detail happens to reuse the phrase, e.g. a comment quoting it).
await recordAuditEvent(env, { eventType: "agent.action.hold", actor: "loopover", targetKey: "owner/repo#11", outcome: "denied", detail: matchingDetail, createdAt: "2026-06-24T10:00:00.000Z" });
// Wrong outcome (the SAME reason text, but the action actually went through).
await recordAuditEvent(env, { eventType: "agent.action.close", actor: "loopover", targetKey: "owner/repo#12", outcome: "completed", detail: matchingDetail, createdAt: "2026-06-24T10:00:00.000Z" });
// Wrong repo.
await recordAuditEvent(env, { eventType: "agent.action.close", actor: "loopover", targetKey: "owner/other-repo#13", outcome: "denied", detail: matchingDetail, createdAt: "2026-06-24T10:00:00.000Z" });
// Before the cutoff.
await recordAuditEvent(env, { eventType: "agent.action.close", actor: "loopover", targetKey: "owner/repo#14", outcome: "denied", detail: matchingDetail, createdAt: "2026-06-24T07:00:00.000Z" });

expect(await recentStaleRecheckDeniedPullNumbers(env, "owner/repo", "2026-06-24T09:00:00.000Z")).toEqual([]);
});

it("treats the repo name as a literal LIKE prefix (regression mirroring countRecentAuditEventsForActorInRepo's escaping fix)", async () => {
const env = createTestEnv();
const matchingDetail = "duplicate-cluster winner #1 is no longer open — action not executed";
await recordAuditEvent(env, { eventType: "agent.action.close", actor: "loopover", targetKey: "owner/foo_bar#20", outcome: "denied", detail: matchingDetail, createdAt: "2026-06-24T10:00:00.000Z" });
// owner/fooXbar is a DIFFERENT repo that would spuriously match "owner/foo_bar#%" if `_` were left as a
// SQL wildcard instead of being escaped.
await recordAuditEvent(env, { eventType: "agent.action.close", actor: "loopover", targetKey: "owner/fooXbar#21", outcome: "denied", detail: matchingDetail, createdAt: "2026-06-24T10:01:00.000Z" });

expect(await recentStaleRecheckDeniedPullNumbers(env, "owner/foo_bar", "2026-06-24T09:00:00.000Z")).toEqual([20]);
});

it("skips a matching row whose targetKey fails to parse into a pull number (matches the LIKE prefix but has a non-numeric suffix)", async () => {
const env = createTestEnv();
const matchingDetail = "duplicate-cluster winner #1 is no longer open — action not executed";
// Satisfies the "owner/repo#%" SQL prefix but parsePullRequestTargetKey rejects the non-integer suffix.
await recordAuditEvent(env, { eventType: "agent.action.close", actor: "loopover", targetKey: "owner/repo#abc", outcome: "denied", detail: matchingDetail, createdAt: "2026-06-24T10:00:00.000Z" });
await recordAuditEvent(env, { eventType: "agent.action.close", actor: "loopover", targetKey: "owner/repo#22", outcome: "denied", detail: matchingDetail, createdAt: "2026-06-24T10:01:00.000Z" });

expect(await recentStaleRecheckDeniedPullNumbers(env, "owner/repo", "2026-06-24T09:00:00.000Z")).toEqual([22]);
});

// #orb-stale-recheck-priority parity guard: STALE_RECHECK_DENIAL_DETAIL_PATTERN hand-types the three
// "denied" reason strings the executor's live staleness rechecks can produce, rather than importing them
// from agent-action-executor.ts -- that module imports FROM db/repositories.ts, so the reverse import would
// create a real module-load cycle (same hazard CONCRETE_EVIDENCE_BLOCKER_CODES's own parity guard,
// agent-actions.test.ts, documents). This reads the real producer source text instead, so a future rewording
// at the producer fails this test immediately rather than silently making the hand-typed pattern permanently
// unmatchable.
it.each(["duplicate-cluster winner #${action.duplicateWinnerPrNumber} is no longer open", "the base-branch conflict that justified this close has since cleared", "the review thread(s) that justified this close are now all resolved"])(
"%s is still produced (not merely mentioned) in its producer (src/services/agent-action-executor.ts)",
(reason) => {
const source = readFileSync("src/services/agent-action-executor.ts", "utf8");
expect(source).toContain(reason);
},
);
});

it("findHottestReviewTargetForRepo returns the PR with the most published surfaces in the window, scoped to ONE repo (#orb-ci-stuck-repeat)", async () => {
const env = createTestEnv();
const publish = (targetKey: string, createdAt: string) =>
Expand Down
Loading