diff --git a/.release-please-manifest.json b/.release-please-manifest.json
index effd9a1389..1d401be83f 100644
--- a/.release-please-manifest.json
+++ b/.release-please-manifest.json
@@ -1,6 +1,6 @@
{
"packages/loopover-mcp": "3.14.1",
- "packages/loopover-engine": "3.15.0",
+ "packages/loopover-engine": "3.15.1",
"packages/loopover-miner": "3.14.1",
"packages/loopover-ui-kit": "1.2.0"
}
diff --git a/apps/loopover-ui/src/components/site/proof-of-power-stats-model.ts b/apps/loopover-ui/src/components/site/proof-of-power-stats-model.ts
index 14033fefaa..e383f428ee 100644
--- a/apps/loopover-ui/src/components/site/proof-of-power-stats-model.ts
+++ b/apps/loopover-ui/src/components/site/proof-of-power-stats-model.ts
@@ -39,7 +39,10 @@ export type PublicStats = {
closePrecisionCiPct?: { lo: number; hi: number } | null;
coveragePct?: number | null;
decidedCount?: number;
- guaranteed?: { close: { alpha: number; lambda: number; coveragePct: number; n: number } | null; merge: { alpha: number; lambda: number; coveragePct: number; n: number } | null };
+ guaranteed?: {
+ close: { alpha: number; lambda: number; coveragePct: number; n: number } | null;
+ merge: { alpha: number; lambda: number; coveragePct: number; n: number } | null;
+ };
instanceCount: number;
windowDays: number;
gamingFlagsCaught: number;
diff --git a/apps/loopover-ui/src/components/site/proof-of-power-stats.test.tsx b/apps/loopover-ui/src/components/site/proof-of-power-stats.test.tsx
index 45d760b709..c11187ddca 100644
--- a/apps/loopover-ui/src/components/site/proof-of-power-stats.test.tsx
+++ b/apps/loopover-ui/src/components/site/proof-of-power-stats.test.tsx
@@ -206,7 +206,13 @@ describe("ProofOfPowerStats", () => {
durationMs: 1,
data: {
...PAYLOAD,
- fleetAccuracy: { accuracyPct: 80, coveragePct: 64.3, instanceCount: 3, windowDays: 90, gamingFlagsCaught: 2 },
+ fleetAccuracy: {
+ accuracyPct: 80,
+ coveragePct: 64.3,
+ instanceCount: 3,
+ windowDays: 90,
+ gamingFlagsCaught: 2,
+ },
},
});
renderWithClient();
diff --git a/src/db/repositories.ts b/src/db/repositories.ts
index 3eed1e71d0..ad9543e0d7 100644
--- a/src/db/repositories.ts
+++ b/src/db/repositories.ts
@@ -5077,9 +5077,18 @@ export async function getLatestAdvisoryForPullRequest(env: Env, repoFullName: st
* sweep tick, while a stale non-cacheable row still correctly falls through to a fresh call.
*
* A PUBLISHED row (`published_at` set by markAiReviewPublished, once the review actually reached the PR) skips
- * the `maxAgeMs` staleness check entirely: the cooldown exists to bound reuse BEFORE the first publish (e.g. two
+ * the `maxAgeMs` staleness check: the cooldown exists to bound reuse BEFORE the first publish (e.g. two
* overlapping sweep passes racing the same head), not to force a periodic re-run of an already-surfaced verdict
- * for the SAME head+fingerprint — see the migration's doc comment for the production incident this closes. */
+ * for the SAME head+fingerprint — see the migration's doc comment for the production incident this closes.
+ *
+ * #9019: that publish exemption does NOT extend to a row whose own verdict was INCONCLUSIVE
+ * (`metadata.inconclusive` — a provider outage, a consensus-disputed roll). `cacheable=0` conflates two
+ * unrelated things: a dynamic review context (grounding/RAG), where the verdict is conclusive but simply not
+ * durable across time, and a genuinely inconclusive verdict. Only the latter must stay retryable: without this,
+ * a transient outage verdict was FINAL for that head once surfaced — the bot never retried, contradicting the
+ * finding's own "re-evaluates on the next update" text, while a green PR gave the contributor no reason to push
+ * the commit that would force one. The #2119 anti-churn incident the exemption exists for concerned
+ * dynamic-context rows, which keep it. */
export async function getCachedAiReview(
env: Env,
repoFullName: string,
@@ -5095,14 +5104,15 @@ export async function getCachedAiReview(
.bind(repoFullName, pullNumber, headSha)
.first<{ notes: string; reviewerCount: number; mode: string; findingsJson: string | null; metadataJson: string | null; cacheable: number; publishedAt: string | null; createdAt: string }>();
if (!row || row.mode !== mode) return null;
+ const metadata = parseJson>(row.metadataJson, {});
if (row.cacheable !== 1) {
if (!options?.allowNonCacheable) return null;
- if (row.publishedAt == null) {
+ // Published rows skip the cooldown UNLESS the verdict itself was inconclusive (#9019, see above).
+ if (row.publishedAt == null || metadata.inconclusive === true) {
const ageMs = Date.now() - Date.parse(row.createdAt);
if (!Number.isFinite(ageMs) || ageMs < 0 || ageMs > (options.maxAgeMs ?? 0)) return null;
}
}
- const metadata = parseJson>(row.metadataJson, {});
if (
expectedInputFingerprint !== undefined &&
metadata.inputFingerprint !== expectedInputFingerprint
@@ -5179,22 +5189,38 @@ export async function getLatestPublishedAiReview(
repoFullName: string,
pullNumber: number,
mode: string,
+ // #9019: `conclusiveOnly` skips rows whose own verdict was INCONCLUSIVE (`metadata.inconclusive` -- a
+ // provider outage, a consensus-disputed roll, a lock-contention placeholder). The one-shot cadence caller
+ // passes it because this lookup is head-AGNOSTIC by design: without it, one inconclusive verdict permanently
+ // pinned that PR across ALL future heads -- unlike every other cadence, a contributor pushing new code could
+ // not escape it, since the reused row was never keyed to their old commit. That PR never actually got its
+ // one real shot, so one-shot must not count it as spent. Deliberately NOT keyed on `cacheable`, which
+ // conflates this with a dynamic review context (a conclusive verdict that simply isn't durable across time,
+ // #2119) -- those stay reusable. The manual-review-freeze caller does not pass it: freezing intends to reuse
+ // whatever was last surfaced, conclusive or not, until a maintainer explicitly retriggers.
+ options?: { conclusiveOnly?: boolean } | undefined,
): Promise<{ notes: string; reviewerCount: number; findings: AdvisoryFinding[]; headSha?: string | undefined; metadata?: Record | undefined } | null> {
- const row = await env.DB
+ // `inconclusive` lives in the metadata JSON blob, so the filter can't be a portable SQL predicate across
+ // both backends -- scan the few most-recent published rows and pick in JS. Bounded by the same small limit
+ // the sibling fingerprint scan uses; a PR accumulates only a handful of review rows over its lifetime.
+ const { results } = await env.DB
.prepare(
- "SELECT notes, reviewer_count AS reviewerCount, head_sha AS headSha, findings_json AS findingsJson, metadata_json AS metadataJson FROM ai_review_cache WHERE repo_full_name = ? AND pull_number = ? AND ai_review_mode = ? AND published_at IS NOT NULL ORDER BY published_at DESC, rowid DESC LIMIT 1",
+ "SELECT notes, reviewer_count AS reviewerCount, head_sha AS headSha, findings_json AS findingsJson, metadata_json AS metadataJson FROM ai_review_cache WHERE repo_full_name = ? AND pull_number = ? AND ai_review_mode = ? AND published_at IS NOT NULL ORDER BY published_at DESC, rowid DESC LIMIT ?",
)
- .bind(repoFullName, pullNumber, mode)
- .first<{ notes: string; reviewerCount: number; headSha: string; findingsJson: string | null; metadataJson: string | null }>();
- if (!row) return null;
- const metadata = parseJson>(row.metadataJson, {});
- return {
- notes: row.notes,
- reviewerCount: row.reviewerCount,
- findings: parseJson(row.findingsJson, []),
- ...(row.headSha ? { headSha: row.headSha } : {}),
- ...(Object.keys(metadata).length > 0 ? { metadata } : {}),
- };
+ .bind(repoFullName, pullNumber, mode, options?.conclusiveOnly ? AI_REVIEW_FINGERPRINT_SCAN_LIMIT : 1)
+ .all<{ notes: string; reviewerCount: number; headSha: string; findingsJson: string | null; metadataJson: string | null }>();
+ for (const row of results ?? []) {
+ const metadata = parseJson>(row.metadataJson, {});
+ if (options?.conclusiveOnly && metadata.inconclusive === true) continue;
+ return {
+ notes: row.notes,
+ reviewerCount: row.reviewerCount,
+ findings: parseJson(row.findingsJson, []),
+ ...(row.headSha ? { headSha: row.headSha } : {}),
+ ...(Object.keys(metadata).length > 0 ? { metadata } : {}),
+ };
+ }
+ return null;
}
/** Count distinct PR head SHAs that already received a published AI review — used by
diff --git a/src/queue/processors.ts b/src/queue/processors.ts
index 0825d84b64..18d4c921a5 100644
--- a/src/queue/processors.ts
+++ b/src/queue/processors.ts
@@ -2266,6 +2266,16 @@ function closeWithheldReason(args: { protectedAuthor: boolean; closeOwnerAuthors
export function agentHoldAuditDetail(args: {
planned: PlannedAgentAction[];
breakerOnPlan: PlannedAgentAction[];
+ // #9040: each plan transform REPORTS whether it engaged, threaded in from the transform's own call site --
+ // this function must never re-derive the cause from a planned-vs-final set difference. The old inference
+ // ("a terminal action was planned but is not in the final plan ⇒ the precision breaker did it") was
+ // provably wrong on the live fleet: `breakerOnPlan` was passed the POST-HOLDOUT plan, so every ε-holdout
+ // adjudication hold (#8831) was attributed to a breaker that had never engaged -- 6 of 6 live
+ // "precision circuit breaker" rows paired 1:1 (within 20ms) with decision_audit_holdout events, while
+ // system_flags contained no engaged breaker at all. A wrong reason is worse than none: an operator chases
+ // a breaker that never fired while the real cause sits one line above in the ledger.
+ precisionBreakerEngaged: boolean;
+ closeAuditHoldoutEngaged: boolean;
gateConclusion: string;
gateBlockerCodes: string[];
ciState: string;
@@ -2279,10 +2289,20 @@ export function agentHoldAuditDetail(args: {
mergeAutonomy: string;
closeAutonomy: string;
}): string {
+ // Specific, transform-reported causes always beat the residual inference below (#9040). Holdout first:
+ // it consumes the post-breaker plan, so when both somehow engaged in one pass the holdout is the transform
+ // that actually removed the final close.
+ if (args.closeAuditHoldoutEngaged)
+ return "auto-action held for close-audit adjudication (ε-holdout)";
+ if (args.precisionBreakerEngaged)
+ return "auto-action held by precision circuit breaker";
+ // Residual (should be unreachable while breaker+holdout are the only terminal-action-removing transforms):
+ // an HONEST generic reason for any future transform that removes a planned terminal action without
+ // reporting itself here -- never a false specific attribution.
const plannedTerminalAction = args.planned.some((action) => action.actionClass === "merge" || action.actionClass === "close");
const finalTerminalAction = args.breakerOnPlan.some((action) => action.actionClass === "merge" || action.actionClass === "close");
if (plannedTerminalAction && !finalTerminalAction)
- return "auto-action held by precision circuit breaker";
+ return "auto-action held: a planned terminal action was removed before execution (unreported transform)";
if (args.ciHasPending || args.ciState === "pending")
return "auto-action held because CI is still pending";
const protectedAuthor = args.authorIsAutomationBot || args.authorIsOwner || args.authorIsAdmin;
@@ -2450,7 +2470,25 @@ async function maybeRunAgentMaintenance(
// block); a pass that loses the race defers cleanly — the next webhook/sweep tick is the backstop. Prefers
// the SubmissionLock Durable Object when bound; otherwise the transient-cache mutex in transient-locks.ts.
const actuationLock = await claimPrActuationLock(env, repoFullName, pr.number);
- if (!actuationLock.acquired) return;
+ if (!actuationLock.acquired) {
+ // #9025: this used to `return` silently -- the job completed "successfully", nothing re-queued the
+ // disposition, and no audit row recorded that a planned action was abandoned. That silently amplified
+ // every restart incident: the recovered job re-ran, republished a no-op surface, hit its own dead
+ // predecessor's orphaned actuation lock here, and lost the disposition a SECOND time with no trace.
+ // Throwing the retryable error instead (the exact pattern review-evasion.ts's withPrActuationLock
+ // established for this same condition) gets the queue's fast 5s-backoff retry, so the disposition lands
+ // once the contending pass (or the orphaned lock's boot-time flush, #9021) releases the PR. The audit row
+ // makes the contention itself visible even if every retry ultimately exhausts.
+ await recordAuditEvent(env, {
+ eventType: "github_app.agent_maintenance_lock_contended",
+ actor: null,
+ targetKey: `${repoFullName}#${pr.number}`,
+ outcome: "queued",
+ detail: "Another pass holds this PR's actuation lock; the disposition retries instead of being dropped.",
+ metadata: { deliveryId: args.deliveryId, repoFullName },
+ }).catch(() => undefined);
+ throw new PrActuationLockContendedError(repoFullName, pr.number, "agent-maintenance");
+ }
try {
await runAgentMaintenancePlanAndExecute(env, {
installationId,
@@ -3427,9 +3465,16 @@ async function runAgentMaintenancePlanAndExecute(
// by hand (#selfhost-holdplan-audit).
const isContributorAuthor = !authorIsOwner && !authorIsAdmin && !authorIsAutomationBot;
const closeEligible = isContributorAuthor || ((authorIsOwner || authorIsAdmin) && settings.closeOwnerAuthors === true);
+ // #9040: each transform's engagement is derived from ITS OWN before/after pair -- the breaker from
+ // (planned -> breakerOnPlan), the holdout from (breakerOnPlan -> holdoutOnPlan) -- and passed explicitly,
+ // so the audit function never guesses a cause from the combined end-to-end set difference again.
+ const closeAuditHoldoutEngaged =
+ breakerOnPlan.some((action) => action.actionClass === "close") && !holdoutOnPlan.some((action) => action.actionClass === "close");
const holdDetail = agentHoldAuditDetail({
planned,
breakerOnPlan: holdoutOnPlan,
+ precisionBreakerEngaged: precisionBreakerDirections.length > 0,
+ closeAuditHoldoutEngaged,
gateConclusion: gate.conclusion,
gateBlockerCodes,
ciState: ciAggregate.ciState,
@@ -3856,6 +3901,10 @@ export async function reReviewStoredPullRequest(
liveFacts,
}),
).catch((error) => {
+ // #9025: rate-limit / retryable errors (chiefly PrActuationLockContendedError from the maintenance
+ // lock claim) MUST reach the queue so the disposition retries instead of being logged-and-dropped --
+ // the same propagation contract the review pipeline's own catch sites already follow.
+ if (isGitHubRateLimitedError(error) || isRetryableJobError(error)) throw error;
console.error(
JSON.stringify({
level: "error",
@@ -6740,6 +6789,10 @@ async function handlePullRequestWebhookEvent(
liveFacts,
}),
).catch((error) => {
+ // #9025: same propagation contract as the sibling maintenance catch above -- a retryable error
+ // (chiefly the maintenance lock's own PrActuationLockContendedError) must reach the queue's retry
+ // path instead of being logged-and-dropped, or the disposition is silently lost.
+ if (isGitHubRateLimitedError(error) || isRetryableJobError(error)) throw error;
/* v8 ignore next -- best-effort: auto-maintain failures are logged, never surfaced to the gate. */
console.error(
JSON.stringify({
@@ -10005,9 +10058,13 @@ async function maybePublishPrPublicSurface(
// a PR that's blacklisted/frozen/already-skipped for another reason never shows AI content at all today,
// and one-shot mode must not change that. A non-null result here means this PR already had its one-shot
// main-review pass, so the fresh call below must be skipped and this reused instead.
+ // #9019: `conclusiveOnly` -- this lookup is head-AGNOSTIC, so without it a non-cacheable row (a provider
+ // outage, an inconclusive/disputed roll, a lock-contention placeholder) pinned the PR's verdict across
+ // every future head, and a contributor pushing new code could not escape it. That PR never got its one
+ // real shot, so it must not count as spent; the next pass/push retries instead of replaying the outage.
const oneShotPriorReview =
oneShotCadenceActive && !authorBlacklisted && !isFrozenForManualReview && !autoReviewSkipReason
- ? await getLatestPublishedAiReview(env, repoFullName, pr.number, settings.aiReviewMode).catch(() => null)
+ ? await getLatestPublishedAiReview(env, repoFullName, pr.number, settings.aiReviewMode, { conclusiveOnly: true }).catch(() => null)
: null;
const aiReviewWillRun =
!authorBlacklisted &&
@@ -10599,6 +10656,13 @@ async function maybePublishPrPublicSurface(
/* v8 ignore next -- runAiReviewForAdvisory (the sole path reaching here) always sets metadata on its "ok" returns; the nullish fallback is a type-level (optional field) safeguard, not a reachable runtime path. */
...(aiReview.metadata ?? {}),
inputFingerprint,
+ // #9019: `cacheable=0` conflates TWO independent, unrelated reasons -- (a) a dynamic review
+ // context (grounding/RAG), where the verdict itself is perfectly CONCLUSIVE but simply not
+ // durable across time, and (b) the review's own verdict being inconclusive/consensus-
+ // disputed. Only (b) should be retried; (a) is correctly reused once published (#2119).
+ // Recording the review's OWN verdict here keeps the two separable at read time without
+ // needing a schema migration, since `cacheable` alone can no longer tell them apart.
+ inconclusive: aiReview.cacheable === false,
// Persist line-anchored findings for post-submission MCP readback (#4519). Inline comments
// themselves are still only posted on a fresh review (see inlineFindings hoisting above);
// this metadata is read-only structured output, not a cache-replay trigger.
diff --git a/test/unit/ai-review-cache.test.ts b/test/unit/ai-review-cache.test.ts
index b27dd9458b..eaf10a7b91 100644
--- a/test/unit/ai-review-cache.test.ts
+++ b/test/unit/ai-review-cache.test.ts
@@ -338,6 +338,40 @@ describe("AI review cache (#1)", () => {
}
});
+ // #9019: `cacheable=0` conflates a DYNAMIC-CONTEXT verdict (conclusive, just not durable across time --
+ // the row above) with a genuinely INCONCLUSIVE one (a provider outage, a consensus-disputed roll). Only the
+ // latter must stay retryable: without this, a transient outage verdict became FINAL for that head the
+ // moment it published -- the bot never retried, contradicting the finding's own "re-evaluates on the next
+ // update" text, while a green PR gave the contributor no reason to push the commit that would force one.
+ it("#9019: a published INCONCLUSIVE row still expires with the cooldown, so the stuck verdict is retried", async () => {
+ const env = createTestEnv();
+ vi.useFakeTimers();
+ try {
+ vi.setSystemTime(new Date("2026-07-01T00:00:00.000Z"));
+ await putCachedAiReview(env, "o/r", 41, "sha1", "block", {
+ notes: "provider outage",
+ reviewerCount: 0,
+ cacheable: false,
+ metadata: { inconclusive: true },
+ });
+ await markAiReviewPublished(env, "o/r", 41, "sha1");
+
+ // Inside the cooldown the published row is still reused -- the retry stays bounded to one per window.
+ vi.setSystemTime(new Date("2026-07-01T00:10:00.000Z"));
+ expect(
+ await getCachedAiReview(env, "o/r", 41, "sha1", "block", undefined, { allowNonCacheable: true, maxAgeMs: 30 * 60 * 1000 }),
+ ).toMatchObject({ notes: "provider outage" });
+
+ // Past the cooldown it MISSES, so the next pass spends a fresh attempt instead of replaying the outage.
+ vi.setSystemTime(new Date("2026-07-01T05:00:00.000Z"));
+ expect(
+ await getCachedAiReview(env, "o/r", 41, "sha1", "block", undefined, { allowNonCacheable: true, maxAgeMs: 30 * 60 * 1000 }),
+ ).toBeNull();
+ } finally {
+ vi.useRealTimers();
+ }
+ });
+
it("still misses an UNPUBLISHED non-cacheable row past the cooldown (unchanged behavior)", async () => {
const env = createTestEnv();
vi.useFakeTimers();
@@ -505,6 +539,46 @@ describe("AI review cache (#1)", () => {
.run();
expect(await getLatestPublishedAiReview(env, "o/r", 55, "block")).toEqual({ notes: "empty head", reviewerCount: 1, findings: [] });
});
+
+ // #9019: this lookup is head-AGNOSTIC by design, so an INCONCLUSIVE row it returns pins the PR's verdict
+ // across ALL future heads -- a contributor pushing new code cannot escape it, because the reused row was
+ // never keyed to their old commit. The one-shot cadence caller passes conclusiveOnly so that PR (which
+ // never actually got its one real shot) is not counted as spent. The freeze caller does not pass it.
+ it("#9019: conclusiveOnly skips an inconclusive published row and falls through to the newest conclusive one", async () => {
+ const env = createTestEnv();
+ await putCachedAiReview(env, "o/r", 57, "sha1", "block", {
+ notes: "real verdict",
+ reviewerCount: 2,
+ metadata: { inconclusive: false },
+ });
+ await markAiReviewPublished(env, "o/r", 57, "sha1");
+ await putCachedAiReview(env, "o/r", 57, "sha2", "block", {
+ notes: "provider outage",
+ reviewerCount: 0,
+ cacheable: false,
+ metadata: { inconclusive: true },
+ });
+ await markAiReviewPublished(env, "o/r", 57, "sha2");
+
+ // Without the flag (the manual-review-freeze caller) the NEWEST published row wins, inconclusive or not.
+ expect(await getLatestPublishedAiReview(env, "o/r", 57, "block")).toMatchObject({ notes: "provider outage" });
+ // With it (the one-shot caller) the outage row is skipped in favor of the genuine prior verdict.
+ expect(await getLatestPublishedAiReview(env, "o/r", 57, "block", { conclusiveOnly: true })).toMatchObject({ notes: "real verdict" });
+ });
+
+ it("#9019: conclusiveOnly returns null when EVERY published row is inconclusive — the PR never got its one shot", async () => {
+ const env = createTestEnv();
+ await putCachedAiReview(env, "o/r", 58, "sha1", "block", {
+ notes: "provider outage",
+ reviewerCount: 0,
+ cacheable: false,
+ metadata: { inconclusive: true },
+ });
+ await markAiReviewPublished(env, "o/r", 58, "sha1");
+
+ expect(await getLatestPublishedAiReview(env, "o/r", 58, "block")).toMatchObject({ notes: "provider outage" });
+ expect(await getLatestPublishedAiReview(env, "o/r", 58, "block", { conclusiveOnly: true })).toBeNull();
+ });
});
describe("countPublishedAiReviewHeads — auto_pause_after_reviewed_commits (#2042)", () => {
diff --git a/test/unit/precision-breakers-chain.test.ts b/test/unit/precision-breakers-chain.test.ts
index a578a4c824..d15dfe5e6f 100644
--- a/test/unit/precision-breakers-chain.test.ts
+++ b/test/unit/precision-breakers-chain.test.ts
@@ -140,6 +140,9 @@ describe("agentHoldAuditDetail — durable why-no-action audit reason", () => {
const base = {
planned: [] as PlannedAgentAction[],
breakerOnPlan: [] as PlannedAgentAction[],
+ // #9040: both transforms now REPORT their own engagement rather than being inferred from a set difference.
+ precisionBreakerEngaged: false,
+ closeAuditHoldoutEngaged: false,
gateConclusion: "success",
gateBlockerCodes: [] as string[],
ciState: "passed",
@@ -155,16 +158,40 @@ describe("agentHoldAuditDetail — durable why-no-action audit reason", () => {
};
it("records that a precision breaker removed the planned terminal action", () => {
- expect(agentHoldAuditDetail({ ...base, planned: [mergeAction] })).toBe("auto-action held by precision circuit breaker");
+ expect(agentHoldAuditDetail({ ...base, precisionBreakerEngaged: true, planned: [mergeAction] })).toBe("auto-action held by precision circuit breaker");
expect(
agentHoldAuditDetail({
...base,
+ precisionBreakerEngaged: true,
planned: [readyLabel, mergeAction],
breakerOnPlan: [{ actionClass: "label", requiresApproval: false, reason: "held", label: AGENT_LABEL_NEEDS_REVIEW, labelOp: "add" }],
}),
).toBe("auto-action held by precision circuit breaker");
});
+ // #9040: the ε-holdout (#8831) and the precision breaker are DIFFERENT causes that both remove a planned
+ // terminal action. The old code inferred "breaker" purely from `planned` having a terminal action that
+ // `breakerOnPlan` lacks -- but the call site passes the POST-holdout plan, so every holdout hold was
+ // misattributed to a breaker that had never engaged (6/6 live rows paired 1:1 with decision_audit_holdout
+ // events while system_flags contained no engaged breaker at all).
+ it("#9040: attributes an ε-holdout hold to the holdout, never to the precision breaker", () => {
+ expect(
+ agentHoldAuditDetail({ ...base, closeAuditHoldoutEngaged: true, planned: [heuristicClose] }),
+ ).toBe("auto-action held for close-audit adjudication (ε-holdout)");
+ });
+
+ it("#9040: the holdout wins when both transforms engaged in one pass (it consumes the post-breaker plan)", () => {
+ expect(
+ agentHoldAuditDetail({ ...base, precisionBreakerEngaged: true, closeAuditHoldoutEngaged: true, planned: [heuristicClose] }),
+ ).toBe("auto-action held for close-audit adjudication (ε-holdout)");
+ });
+
+ it("#9040: a terminal action removed with NO transform reporting itself gets an honest generic reason, never a false breaker attribution", () => {
+ expect(agentHoldAuditDetail({ ...base, planned: [mergeAction] })).toBe(
+ "auto-action held: a planned terminal action was removed before execution (unreported transform)",
+ );
+ });
+
it("records pending CI ahead of mergeability or gate-policy guesses", () => {
expect(agentHoldAuditDetail({ ...base, ciState: "pending" })).toBe("auto-action held because CI is still pending");
expect(agentHoldAuditDetail({ ...base, ciHasPending: true })).toBe("auto-action held because CI is still pending");
diff --git a/test/unit/queue-2.test.ts b/test/unit/queue-2.test.ts
index 0633211e59..906fa84025 100644
--- a/test/unit/queue-2.test.ts
+++ b/test/unit/queue-2.test.ts
@@ -6,6 +6,7 @@ import { PR_PANEL_COMMENT_MARKER } from "../../src/github/comments";
import * as backfillModule from "../../src/github/backfill";
import * as rateLimitModule from "../../src/github/rate-limit";
import * as repositoriesModule from "../../src/db/repositories";
+import * as decisionRecordModule from "../../src/review/decision-record";
import * as reviewEffortModule from "../../src/review/review-effort";
import * as repositorySettingsModule from "../../src/settings/repository-settings";
import * as posthogModule from "../../src/selfhost/posthog";
@@ -3227,12 +3228,76 @@ describe("queue processors", () => {
// review round 4) — one shared namespace, not a maintenance-only lock.
await env.SELFHOST_TRANSIENT_CACHE?.set("pr-actuation-lock:owner/agent-repo#7", "1", 60);
- await processJob(env, { type: "agent-regate-pr", deliveryId: "race-sweep", repoFullName: "owner/agent-repo", prNumber: 7, installationId: 9001 });
+ // #9025: the contended pass RETRIES rather than returning silently. It used to `return`, which completed
+ // the job "successfully" while dropping the disposition entirely -- a silent amplifier for every restart
+ // incident (the recovered job re-ran, hit its own dead predecessor's orphaned lock, and lost the
+ // disposition a second time with no trace). Throwing the retryable error is the same contract
+ // review-evasion.ts's withPrActuationLock already used for this exact condition; the queue honors its
+ // retryAfterMs via consumingRetryDelayMs, so the disposition lands once the contending pass releases.
+ await expect(
+ processJob(env, { type: "agent-regate-pr", deliveryId: "race-sweep", repoFullName: "owner/agent-repo", prNumber: 7, installationId: 9001 }),
+ ).rejects.toMatchObject({ name: "PrActuationLockContendedError", retryKind: "pr_actuation_lock_contended" });
- // The held lock made this pass skip its plan-and-execute critical section entirely — no mutation attempted.
+ // The held lock still made this pass skip its plan-and-execute critical section entirely — the retry is
+ // about not LOSING the disposition, never about mutating while another pass owns the PR.
expect(mergeCalls).toBe(0);
const actionAudits = await env.DB.prepare("select count(*) as n from audit_events where event_type like 'agent.action.%'").first<{ n: number }>();
expect(actionAudits?.n).toBe(0);
+ // ...and the contention itself is now visible in the ledger instead of leaving no trace at all.
+ const contendedAudit = await env.DB.prepare("select outcome, detail from audit_events where event_type = ? and target_key = ?")
+ .bind("github_app.agent_maintenance_lock_contended", "owner/agent-repo#7")
+ .first<{ outcome: string; detail: string }>();
+ expect(contendedAudit?.outcome).toBe("queued");
+
+ // #9025: the contention audit write is best-effort -- a failing write must never swallow or replace the
+ // retry itself. The disposition's durability cannot depend on the ledger being writable.
+ const originalRecordAuditEvent = repositoriesModule.recordAuditEvent;
+ const auditSpy = vi.spyOn(repositoriesModule, "recordAuditEvent").mockImplementation(async (auditEnv, event) => {
+ if (event.eventType === "github_app.agent_maintenance_lock_contended") throw new Error("audit DB down");
+ await originalRecordAuditEvent(auditEnv, event);
+ });
+ await expect(
+ processJob(env, { type: "agent-regate-pr", deliveryId: "race-sweep-audit-down", repoFullName: "owner/agent-repo", prNumber: 7, installationId: 9001 }),
+ ).rejects.toMatchObject({ name: "PrActuationLockContendedError" });
+ auditSpy.mockRestore();
+ });
+
+ it("#9025: a NON-retryable maintenance failure is still swallowed and logged, never propagated to the queue", 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: {}, 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" }, gatePack: "oss-anti-slop" });
+ await upsertRepoFocusManifest(env, "owner/agent-repo", { settings: { checkRunMode: "off", commentMode: "off", publicSurface: "off", aiReviewMode: "off", reviewCheckMode: "required" } });
+ await upsertPullRequestFromGitHub(env, "owner/agent-repo", { number: 8, title: "Clean PR", state: "open", user: { login: "contributor" }, head: { sha: "a8" }, labels: [], body: "Closes #1" });
+ vi.stubGlobal("fetch", async (input: RequestInfo | URL, init?: RequestInit) => {
+ const url = input.toString();
+ if (url.includes("/access_tokens")) return Response.json({ token: "installation-token" });
+ if (url.includes("/pulls/8/files")) return Response.json([{ filename: "src/a.ts", status: "modified", additions: 1, deletions: 0, changes: 1, patch: "@@\n+export const ok = true;" }]);
+ if (url.includes("/pulls/8/reviews") && init?.method === "POST") return Response.json({ id: 1 });
+ if (url.includes("/pulls/8/reviews")) return Response.json([]);
+ if (/\/pulls\/8(\?|$)/.test(url)) return Response.json({ number: 8, title: "Clean PR", state: "open", user: { login: "contributor" }, head: { sha: "a8" }, labels: [], body: "Closes #1", mergeable_state: "clean" });
+ if (url.includes("/commits/a8/check-runs")) return Response.json({ total_count: 0, check_runs: [] });
+ if (url.includes("/commits/a8/status")) return Response.json({ state: "success", statuses: [] });
+ if (url.includes("/issues/1")) return Response.json({ number: 1, title: "Issue", state: "open", labels: [], user: { login: "reporter" } });
+ if (url.includes("/branches/")) return Response.json({ protected: false, protection: { required_status_checks: { contexts: [] } } });
+ if (url === "https://api.gittensor.io/miners") return Response.json([]);
+ return Response.json({});
+ });
+ vi.setSystemTime(new Date("2026-05-28T02:00:00.000Z"));
+ // A plain (non-retryable) failure from inside the maintenance pass: the guard's FALSE arm must still
+ // swallow-and-log exactly as before #9025, so an ordinary bug never dead-letters a whole webhook job.
+ const errorSpy = vi.spyOn(console, "error").mockImplementation(() => undefined);
+ // persistDecisionRecord is awaited UNCAUGHT and called ONLY inside runAgentMaintenancePlanAndExecute, so a
+ // rejection here surfaces exactly where a real bug in the maintenance critical section would.
+ const capSpy = vi.spyOn(decisionRecordModule, "persistDecisionRecord").mockRejectedValue(new Error("plain non-retryable failure"));
+
+ await expect(
+ processJob(env, { type: "agent-regate-pr", deliveryId: "maintenance-plain-failure", repoFullName: "owner/agent-repo", prNumber: 8, installationId: 9001 }),
+ ).resolves.toBeUndefined();
+
+ capSpy.mockRestore();
+ expect(errorSpy.mock.calls.some((call) => String(call[0]).includes("agent_maintenance_failed"))).toBe(true);
+ errorSpy.mockRestore();
});
it("the sweep stamps the marker INLINE when the repo has no installation (audit-only, still converges) (#audit-sweep-fanout)", async () => {
diff --git a/test/unit/queue-3.test.ts b/test/unit/queue-3.test.ts
index 456d6b1f33..19a1de8968 100644
--- a/test/unit/queue-3.test.ts
+++ b/test/unit/queue-3.test.ts
@@ -3082,17 +3082,23 @@ describe("queue processors", () => {
return Response.json({});
});
- await processJob(env, {
- type: "github-webhook",
- deliveryId: "contributor-cap-pr-lock-contended",
- eventName: "pull_request",
- payload: {
- action: "opened",
- installation: { id: 123, account: { login: "JSONbored", id: 1, type: "User" } },
- repository: { name: "gittensory", full_name: "JSONbored/gittensory", private: false, owner: { login: "JSONbored" } },
- pull_request: { number: 56, title: "Farmer's 3rd PR", state: "open", user: { login: "farmer99" }, head: { sha: "f56" }, labels: [], body: "x", mergeable_state: "clean", reviewDecision: "APPROVED" },
- },
- });
+ // #9025: the trailing maintenance pass hits the SAME still-held actuation lock and now throws the
+ // retryable contention error rather than returning silently, so the disposition retries instead of being
+ // dropped. The early-cap short-circuit under test here is a different call site and still defers cleanly
+ // (its own `return false`, unchanged) -- both assertions below are exactly as before.
+ await expect(
+ processJob(env, {
+ type: "github-webhook",
+ deliveryId: "contributor-cap-pr-lock-contended",
+ eventName: "pull_request",
+ payload: {
+ action: "opened",
+ installation: { id: 123, account: { login: "JSONbored", id: 1, type: "User" } },
+ repository: { name: "gittensory", full_name: "JSONbored/gittensory", private: false, owner: { login: "JSONbored" } },
+ pull_request: { number: 56, title: "Farmer's 3rd PR", state: "open", user: { login: "farmer99" }, head: { sha: "f56" }, labels: [], body: "x", mergeable_state: "clean", reviewDecision: "APPROVED" },
+ },
+ }),
+ ).rejects.toMatchObject({ name: "PrActuationLockContendedError" });
// The early close DEFERRED (no PATCH close fired from it) and the pipeline fell through, mirroring the
// author-lock contention semantics one namespace over.
diff --git a/test/unit/queue.test.ts b/test/unit/queue.test.ts
index 3e9372c936..1cbdabc9ca 100644
--- a/test/unit/queue.test.ts
+++ b/test/unit/queue.test.ts
@@ -6251,14 +6251,17 @@ describe("queue processors", () => {
expect(missAudit?.n).toBe(1); // only the genuine first-run miss — the forced pass is NOT double-counted here
});
- it("#9: a low-activity repo's old open PR NEVER generates a repeated AI review across many sweep ticks once published (#regate-churn)", async () => {
- // Superseded policy note: this used to assert a BOUNDED, periodic retry (one fresh attempt per tick once
- // AI_REVIEW_NON_CACHEABLE_RETRY_COOLDOWN_MS elapsed) for a never-durably-cacheable (still-inconclusive)
- // outcome. That was itself the incident-mitigation for #1462, but it was still an UNBOUNDED total spend
- // over a PR's lifetime (one fresh call every cooldown window, forever, for as long as the PR stayed open
- // and inconclusive). The `published_at` marker (this PR) makes ANY review — cacheable or not — immutable
- // for its exact head+fingerprint the moment it is actually published: a tick past the cooldown no longer
- // buys a fresh attempt at all; only a real content/config change or an explicit maintainer force-rerun does.
+ it("#9019: a published-but-INCONCLUSIVE review still retries once per cooldown window, but never within one", async () => {
+ // Policy history, twice revised. Originally: a bounded periodic retry (one fresh attempt per tick once
+ // AI_REVIEW_NON_CACHEABLE_RETRY_COOLDOWN_MS elapsed) for a never-durably-cacheable outcome (#1462).
+ // Then the `published_at` marker made ANY review — cacheable or not — immutable for its exact
+ // head+fingerprint the moment it published, to bound lifetime spend (#regate-churn).
+ // #9019 narrows that second change back: immutability is right for a CONCLUSIVE verdict (including a
+ // dynamic-context one, covered by the sibling grounding test), but wrong for an INCONCLUSIVE one. A
+ // provider outage or a consensus-disputed roll would otherwise be FINAL for that head forever — the bot
+ // never retried, directly contradicting the finding's own "re-evaluates on the next update" text, while a
+ // green PR gave the contributor no reason to push the commit that would force one. The cooldown still
+ // bounds the spend to at most one attempt per window; only genuinely inconclusive rows are eligible.
let aiCalls = 0;
const env = createTestEnv({
GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem(),
@@ -6291,23 +6294,27 @@ describe("queue processors", () => {
const callsPerAttempt = aiCalls;
expect(callsPerAttempt).toBeGreaterThan(0);
- // 5 more sweep ticks over a ~10-hour span (the production incident's 6h window had 97 sweep events for one
- // repo), each beyond what used to be the 30-minute cooldown — the unchanged PR's published review is now
- // reused indefinitely, so NONE of these buy a fresh attempt.
+ // 5 more sweep ticks over a ~10-hour span, each well BEYOND the 30-minute cooldown. Because the verdict
+ // is inconclusive, each buys exactly ONE fresh attempt (#9019) -- the stuck PR keeps getting a real
+ // chance to resolve instead of replaying the outage forever.
const tickTimes = ["03:50:00", "05:40:00", "07:30:00", "09:20:00", "11:10:00"];
for (const [index, time] of tickTimes.entries()) {
vi.setSystemTime(new Date(`2026-05-28T${time}.000Z`));
await processJob(env, { type: "agent-regate-pr", deliveryId: `low-activity-${index}`, repoFullName: "JSONbored/gittensory", prNumber: 65, installationId: 123 });
}
- expect(aiCalls).toBe(callsPerAttempt); // still just the one, original attempt
-
- // Tighten four more ticks to well INSIDE what used to be the cooldown window — still zero additional spend.
- const tightTicks = ["12:00:00", "12:05:00", "12:10:00", "12:15:00"];
+ expect(aiCalls).toBe(callsPerAttempt * (1 + tickTimes.length));
+
+ // Four more ticks well INSIDE one cooldown window buy NOTHING -- the retry stays strictly bounded to one
+ // attempt per window, which is what keeps lifetime spend finite. This is the half of #regate-churn's
+ // guarantee that #9019 deliberately preserves. Timed against the LAST attempt above (11:10), not the
+ // original baseline: every one of these lands inside its 30-minute window.
+ const callsBeforeTightTicks = aiCalls;
+ const tightTicks = ["11:20:00", "11:25:00", "11:30:00", "11:35:00"];
for (const [index, time] of tightTicks.entries()) {
vi.setSystemTime(new Date(`2026-05-28T${time}.000Z`));
await processJob(env, { type: "agent-regate-pr", deliveryId: `low-activity-tight-${index}`, repoFullName: "JSONbored/gittensory", prNumber: 65, installationId: 123 });
}
- expect(aiCalls).toBe(callsPerAttempt);
+ expect(aiCalls).toBe(callsBeforeTightTicks);
});
describe("#regate-churn: production reproductions (#3379, #3383) and the maintainer-gated freeze", () => {