Commit Graph
8 Commits
Author SHA1 Message Date
Claude 2099a16a1f test(review): prove fail-open status contract 2026-08-30 14:11:03 -04:00
Claude a12f94f459 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.
2026-08-29 08:21:41 -04:00
Claude 0db44a4d73 fix(review): run claude-cli models directly 2026-08-23 00:23:02 -04:00
Claude 0dec11b5ed Disable router fallbacks for local reviews 2026-08-21 19:55:50 -04:00
Claude 6478ce2bfa Reject local review model drift 2026-08-21 19:49:45 -04:00
Claude c648a5a6da Track running Granthi review queue jobs 2026-08-21 18:04:35 -04:00
Claude ad8c1256d8 fix: dual-review round-2 — partial diffs cap the verdict, bounded fallback chain, URL-safe paths
Codex adversarial review of the fallback chain found 1 P1 + 3 P2:
- P1: head-only and incomplete per-commit fallbacks could present a SUBSET
  diff as a full passing review. fetchDiff now returns {diff, partial, note};
  review.js caps partial reviews at 'warn' (same contract as truncation) and
  labels the status 'partial review (<reason>)'.
- getRepo default-branch lookup swallows only 404; auth/5xx propagate.
- per-commit reconstruction dedupes + validates shas (40-hex) and hard-caps
  at 20 requests; capped/short coverage marks the result partial; the
  all-sources-failed message is bounded to 1500 chars.
- gitea.js URL-encodes every webhook-supplied path segment (owner/repo/ref/
  sha) — branch names with '/' or '#' can no longer distort request paths.

Tests: 61 pass (2 new: sha cap/dedupe, getRepo 401 propagation).

Co-Authored-By: Claude Fable 5 <[email protected]>
2026-08-19 11:58:59 -04:00
Claude a531323865 fix: diff-fetch fallback chain — stop failing open on compare 404
~23% of reviews were failing open with `diff fetch failed: gitea GET
/{owner}/{repo}/compare/{before}...{after}.diff -> 404`. Probing the live
forge (Gitea 1.27.1) showed the web compare route answers 404 for ALL refs
on private repos — it does not honor `Authorization: token` and resolves as
anonymous — while the API compare (JSON) and per-commit
/git/commits/{sha}.diff endpoints succeed for the exact same sha pairs.
It also 404s legitimately when `before` is unknown (force-push, rebase).

fetchDiff now walks a fallback chain instead of failing open on the first
404 (any non-404 error still propagates immediately):

  PR job:     pulls/{n}.diff -> git/commits/{head}.diff
  range push: web compare {before}...{after}.diff
              -> web compare {defaultBranch}...{after}.diff
              -> API compare commit list (falls back to the webhook's
                 commitShas) -> concatenated per-commit git/commits/{sha}.diff,
                 stopping past maxDiffBytes so the existing truncation cap
                 (verdict capped at 'warn') kicks in
              -> git/commits/{after}.diff alone
  new branch: git/commits/{after}.diff (unchanged)

Only when every source is exhausted does the review fail open, and the log
line now lists every URL tried. Webhook jobs carry the repo default_branch
so the fallback needs no extra API call; getRepo() covers recovered jobs
persisted before this change.

Verified against the live forge for the exact failing pair from today's
service.log (Nirlabinc/shreai 481d8663...46696b50): the chain recovers a
full-range 8,993-byte diff via 2/2 per-commit diffs.

Co-Authored-By: Claude Fable 5 <[email protected]>
2026-08-19 11:48:06 -04:00