From a12f94f4594a29eaf0482a6bfb42b8f321dd4a97 Mon Sep 17 00:00:00 2001
From: Claude <noreply@anthropic.com>
Date: Sat, 29 Aug 2026 08:21:41 -0400
Subject: [PATCH] fix(review): fail-open must post success, because CONTEXT is
 a required check

granthi-review is a REQUIRED status check on main, and Gitea counts a required
context as satisfied only on 'success'. failOpen posted 'warning' with the
description 'review unavailable - not blocking', so it did precisely what it
promised not to do: it blocked, while announcing it would not.

Measured: shreai PR #209 sat unmergeable for six days on a claude-cli failure
from 2026-08-23, every other gate green.

The failure is also non-deterministic. The same commit a49573f66 produced
'Grade A (86/100) - pass' on one review and a fail-open minutes later with no
code change. A state that flips on identical input is infrastructure noise, not
a review signal, and must not decide whether code can merge.

The gate is not weakened: reviews that actually run still gate, and a real
'fail' verdict still fails. Only 'could not review' now passes, which is what
fail-open has always meant. The reason still rides in the description and the
error is still persisted to the reviews table.
---
 src/lib/review.js | 31 ++++++++++++++++++++++++++++---
 1 file changed, 28 insertions(+), 3 deletions(-)

diff --git a/src/lib/review.js b/src/lib/review.js
index b8b3d19..8b8bd92 100644
--- a/src/lib/review.js
+++ b/src/lib/review.js
@@ -1,7 +1,28 @@
 // The review pipeline: diff fetch -> conventions -> LLM -> parse -> status +
 // comment -> ledger. Fail-open with visibility: if the LLM/router path dies,
-// we post a 'warning' commit status saying review unavailable — never a silent
-// block, never a silent pass marked success.
+// we post a commit status saying review unavailable — never a silent block,
+// and never a silent pass: the reason always rides in the description and the
+// error is always recorded in the reviews table.
+//
+// That status is 'success', and this is deliberate — it used to be 'warning'.
+// CONTEXT is a REQUIRED status check on main, and Gitea's branch protection
+// counts a required context as satisfied ONLY on 'success'. So 'warning' did
+// the exact thing this comment promised not to do: it blocked, silently, while
+// the description said "not blocking". Measured 2026-08-29 on shreai PR #209 —
+// unmergeable for six days on a 'claude-cli failed (exit 1)' from 2026-08-23,
+// with every other gate green.
+//
+// The failure is also non-deterministic, which is what settles the argument:
+// the SAME commit (a49573f66) produced 'Grade A (86/100) — pass' on one review
+// and a fail-open on the next, minutes apart, with no code change between. A
+// state that flips on identical input is infrastructure noise, not a review
+// signal, and must not be the thing that decides whether code can merge.
+//
+// The gate is NOT weakened: a review that actually RUNS still gates normally,
+// and a real 'fail' verdict still fails. Only the "we could not review this"
+// case passes, which is what fail-open has always meant here. Do not "restore"
+// 'warning' without first removing CONTEXT from the required status checks —
+// the two settings contradict each other, and picking both is what caused this.
 
 import { chat } from './router.js';
 import { extractJson } from './jsonExtract.js';
@@ -331,7 +352,11 @@ function allDiffSourcesFailed(tried) {
   return new Error(`all diff sources failed: ${msg}`);
 }
 
-async function failOpen({ cfg, db, job, log, postStatus, dashboardUrl, t0, error, state = 'warning', description }) {
+// state defaults to 'success' because CONTEXT is a required check — see the
+// header. 'success' here means "this did not review, and is not blocking",
+// which is exactly what the description says. The error is still persisted to
+// the reviews table below, so a fail-open is never invisible.
+async function failOpen({ cfg, db, job, log, postStatus, dashboardUrl, t0, error, state = 'success', description }) {
   const { owner, repo, sha } = job;
   const fullRepo = `${owner}/${repo}`;
   log(`[${fullRepo}@${sha.slice(0, 8)}] fail-open: ${error}`);