Merge pull request 'fix(labels): grade Forgejo review states' (#244) from codex-bot-andresmgsl/ceremony:build/235-forgejo-review-vocabulary into main

Reviewed-on: heavy-duty/ceremony#244
Reviewed-by: glm-bot-andresmgsl <andres+5@heavyduty.builders>
Reviewed-by: claude-bot-andresmgsl <andres+1@heavyduty.builders>
Reviewed-by: kimi-bot-andresmgsl <andres+4@heavyduty.builders>

actions/labels-reconcile/labels-reconcile.sh

@@ -272,7 +272,7 @@ set_required_bots() { # the PR author is recused by construction
 #   HEAD_SHA     the PR's current head commit
 #   BASE_SHA     the PR's base branch head (the release-shape guard's ref)
 #   REQUESTED    newline-separated logins with a review currently requested
-#   REVIEWS_JSON JSON array of submitted (non-PENDING) reviews
+#   REVIEWS_JSON JSON array of submitted, gradeable reviews
 #   MERGEABLE    MERGEABLE | CONFLICTING | UNKNOWN  (GitHub's own verdict)
 #   CHECKS       SUCCESS | FAILURE | PENDING | NONE (the check rollup)
 #   LABELS       newline-separated labels currently on the PR
@@ -444,18 +444,25 @@ bot_verdict() { # $1 = login → MISSING | BLOCK | APPROVE | STALE | FEEDBACK
   if [ -z "$review" ]; then echo MISSING; return; fi
   state="$(jq -r '.state' <<<"$review")"
   commit="$(jq -r '.commit_id' <<<"$review")"
+  # This case grades a submitted verdict. The ingestion allow-list answers the
+  # separate question of whether a row is a submitted review at all (#235).
   case "$state" in
-    CHANGES_REQUESTED)
+    CHANGES_REQUESTED | REQUEST_CHANGES)
-      # blocks at ANY head — GitHub's own semantic: only a newer review
+      # blocks at ANY head — both forges' semantic: only a newer review from
-      # from the same reviewer clears it
+      # the same reviewer clears it
       echo BLOCK ;;
     APPROVED)
       if [ "$commit" = "$HEAD_SHA" ]; then echo APPROVE; else echo STALE; fi ;;
-    *)
+    COMMENTED | COMMENT)
-      # COMMENTED and anything else: a non-verdict. The machine does not
+      # A comment is a non-verdict. The machine does not read bodies — if the
-      # read bodies — if the comment is really an agreement, the AUTHOR
+      # comment is really an agreement, the AUTHOR says so by requesting the
-      # says so by requesting the human's review.
+      # human's review.
       echo FEEDBACK ;;
+    *)
+      # An unknown state is not evidence that a reviewer answered. Keep the
+      # round open and make the next forge vocabulary surprise visible (#235).
+      log "$1: unrecognised review state $state" >&2
+      echo MISSING ;;
   esac
 }
 
@@ -1054,9 +1061,15 @@ main() {
       HEAD_SHA="$(jq -r '.head.sha' <<<"$PR_JSON")"
       BASE_SHA="$(jq -r '.base.sha' <<<"$PR_JSON")"
       LABELS="$(jq -r '.labels[].name' <<<"$PR_JSON")"
-      # PENDING reviews are unsubmitted drafts in someone's browser — not a verdict
+      # This allow-list answers whether a row is a submitted, gradeable review;
+      # bot_verdict separately answers what that submitted verdict says (#235).
+      # PENDING drafts and Forgejo REQUEST_REVIEW request rows are not reviews.
       REVIEWS_JSON="$(forge_api --paginate "repos/$REPO/pulls/$n/reviews" --jq '.[]' \
-        | jq -s '[.[] | select(.state != "PENDING")]')"
+        | jq -s '[.[] | select(.state == "APPROVED"
+                               or .state == "CHANGES_REQUESTED"
+                               or .state == "REQUEST_CHANGES"
+                               or .state == "COMMENTED"
+                               or .state == "COMMENT")]')"
       # Read AFTER the reviews, because the raw field is not portable: Forgejo
       # never clears it, so it is intersected with who still owes a verdict on
       # this head (#188 term 4). A no-op on GitHub, which clears it itself.

changelog.d/235.md

@@ -0,0 +1,3 @@
+### Fixed
+
+- Forgejo review requests no longer count as verdicts, while its blocking and comment states now grade like their GitHub equivalents (#235).

test/labels-reconcile.test.sh

@@ -172,6 +172,20 @@ $BOT2
 $BOT3" REVIEWS_JSON='[]'
 expect "requested bots mean bots-reviewing" state:bots-reviewing "$(decide_state)"
 
+# Forgejo materializes each live request as a REQUEST_REVIEW row. Those rows
+# are not submitted verdicts (#235): they must leave all three logins
+# outstanding, so an opening round stays with the panel rather than falling
+# through to the builder as three comment-only answers.
+REVIEWS_JSON="$(reviews \
+  "$(rev "$BOT1" REQUEST_REVIEW "" "" t1)" \
+  "$(rev "$BOT2" REQUEST_REVIEW "" "" t2)" \
+  "$(rev "$BOT3" REQUEST_REVIEW "" "" t3)")"
+REQUESTED="$(outstanding_requests "$BOT1
+$BOT2
+$BOT3" 2>"$RTMP/request-round-log")"
+expect "three Forgejo request rows keep the opening round with the panel" \
+  state:bots-reviewing "$(round_state)"
+
 # -- a bot that never reviewed keeps the round open ---------------------------
 #    With a live request that is the bots' ball; with NO request outstanding it
 #    is the agent's, because nothing is coming until somebody asks.
@@ -215,6 +229,25 @@ REVIEWS_JSON="$(reviews \
   "$(rev "$BOT3" APPROVED head1 "" t3)")"
 expect "changes-requested blocks even from an old head" state:addressing "$(decide_state)"
 
+# bot_verdict grades submitted states from both forges (#235). Each direct
+# assertion names one arm so a later vocabulary regression cannot hide behind
+# round_state's shared BLOCK/FEEDBACK handling.
+expect "GitHub CHANGES_REQUESTED grades as a block" BLOCK \
+  "$(bot_verdict "$BOT1")"
+REVIEWS_JSON="$(reviews "$(rev "$BOT1" REQUEST_CHANGES old1 "blockers below" t1)")"
+expect "Forgejo REQUEST_CHANGES grades as a block" BLOCK \
+  "$(bot_verdict "$BOT1")"
+REVIEWS_JSON="$(reviews "$(rev "$BOT1" COMMENT head1 "non-blocking note" t1)")"
+expect "Forgejo COMMENT grades as feedback" FEEDBACK \
+  "$(bot_verdict "$BOT1")"
+REVIEWS_JSON="$(reviews "$(rev "$BOT1" FUTURE_FORGE_STATE head1 "" t1)")"
+bot_verdict "$BOT1" >"$RTMP/unknown-verdict" 2>"$RTMP/unknown-verdict-log"
+expect "an unrecognised review state is conservatively missing" MISSING \
+  "$(cat "$RTMP/unknown-verdict")"
+expect "an unrecognised review state logs the login and spelling" yes \
+  "$(grep -qF "$BOT1: unrecognised review state FUTURE_FORGE_STATE" \
+      "$RTMP/unknown-verdict-log" && echo yes || echo no)"
+
 # -- a stale approval must not promote unreviewed code ------------------------
 REVIEWS_JSON="$(reviews \
   "$(rev "$BOT1" APPROVED old1 "" t1)" \
@@ -253,6 +286,24 @@ REQUESTED="$HUMAN"
 expect "re-requested human is needs-human again" state:needs-human "$(decide_state)"
 REQUESTED=""
 
+# Forgejo's human-block spelling carries the same meaning (#235). This is
+# independently observable because only BLOCK prevents state:needs-human once
+# every bot approves.
+REVIEWS_JSON="$(reviews \
+  "$(rev "$BOT1" APPROVED head1 "" t1)" \
+  "$(rev "$BOT2" APPROVED head1 "" t2)" \
+  "$(rev "$BOT3" APPROVED head1 "" t3)" \
+  "$(rev "$HUMAN" REQUEST_CHANGES head1 "not yet" t4)")"
+expect "Forgejo human request-changes with bots approving is addressing" \
+  state:addressing "$(decide_state)"
+REVIEWS_JSON="$(reviews \
+  "$(rev "$BOT1" APPROVED head1 "" t1)" \
+  "$(rev "$BOT2" APPROVED head1 "" t2)" \
+  "$(rev "$BOT3" APPROVED head1 "" t3)" \
+  "$(rev "$HUMAN" APPROVED head1 "" t4)")"
+expect "the same Forgejo-shaped fixture with human approval reaches needs-human" \
+  state:needs-human "$(decide_state)"
+
 # -- an old human comment must not wedge the handoff (codex, #85 round 3) -----
 REVIEWS_JSON="$(reviews \
   "$(rev "$HUMAN" COMMENTED old1 "early thoughts" t0)" \
@@ -1588,6 +1639,49 @@ expect "...with no 'reconciled.' token in the output" \
 expect "...naming the attempt that did not happen" \
   yes "$(grep -q 'label edit FAILED' <<<"$sf_out" && echo yes || echo no)"
 
+# Drive the ingestion expression through main(), independently of
+# bot_verdict (#235). Capturing REVIEWS_JSON at the outstanding_requests
+# boundary proves REQUEST_REVIEW never reaches the grader; the COMMENT and
+# APPROVED controls prove both gradeable Forgejo states and rows generally
+# survive the filter.
+review_filter_probe() {
+  (
+    REPO=owner/repo
+    LABELS_CONF="$FIXTURE_CONF"
+    CEREMONY_FORGE=github
+    # shellcheck disable=SC2317 # reached through the forge backend, not called directly (#188)
+    gh() {
+      if [ "$1" = label ] && [ "$2" = list ]; then core_label_rows | cut -d'|' -f1; return 0; fi
+      if [ "$1" = pr ] && [ "$2" = list ]; then printf '%s\n' 601; return 0; fi
+      if [ "$1" = pr ] && [ "$2" = view ]; then
+        jq -n '{mergeable:"MERGEABLE",statusCheckRollup:[]}'
+        return 0
+      fi
+      if [ "$1" = issue ] && [ "$2" = edit ]; then return 0; fi
+      case "$(forge_stub_path "$*")" in
+        *repos/owner/repo/pulls/601/reviews*)
+          jq -nc \
+            '{user:{login:"fixture-bot-one"},state:"REQUEST_REVIEW",commit_id:"",submitted_at:"2026-08-22T00:46:05Z"},
+             {user:{login:"fixture-bot-three"},state:"COMMENT",commit_id:"head1",submitted_at:"2026-08-22T00:46:35Z"},
+             {user:{login:"fixture-bot-two"},state:"APPROVED",commit_id:"head1",submitted_at:"2026-08-22T00:47:05Z"}' ;;
+        */pulls/601)
+          jq -n '{draft:true,user:{login:"fixture-builder"},head:{sha:"head1"},base:{sha:"base1"},
+                  labels:[{name:"state:building"}],requested_reviewers:[],
+                  created_at:"2026-08-22T00:45:00Z"}' ;;
+        *) printf '[]\n' ;;
+      esac
+    }
+    # shellcheck disable=SC2317 # main invokes the probe override indirectly
+    outstanding_requests() {
+      printf '%s\n' "$REVIEWS_JSON" >"$RTMP/gradeable-reviews.json"
+    }
+    main >/dev/null
+  )
+}
+review_filter_probe
+expect "REQUEST_REVIEW is removed before REVIEWS_JSON reaches the grader" \
+  COMMENT,APPROVED "$(jq -r 'map(.state) | join(",")' "$RTMP/gradeable-reviews.json")"
+
 # ---------------------------------------------------------------------------
 # outstanding_requests — the portable "who still owes a verdict" (#188 term 4)
 #
@@ -1615,6 +1709,15 @@ expect "a stale approval still owes a verdict" "$BOT3" \
 expect "a reviewer who never reviewed still owes one" "nobody" \
   "$(outstanding_requests "nobody")"
 
+REVIEWS_JSON="$(reviews \
+  "$(rev "$BOT1" REQUEST_REVIEW "" "" 2026-08-22T00:46:05Z)")"
+expect "a Forgejo request row is not an answer and leaves the login outstanding" \
+  "$BOT1" "$(outstanding_requests "$BOT1" 2>"$RTMP/request-outstanding-log")"
+REVIEWS_JSON="$(reviews \
+  "$(rev "$BOT1" APPROVED head1 "" 2026-08-01T00:00:00Z)" \
+  "$(rev "$BOT2" CHANGES_REQUESTED head1 "" 2026-08-01T00:00:00Z)" \
+  "$(rev "$BOT3" APPROVED head0 "" 2026-07-01T00:00:00Z)")"
+
 # The Forgejo shape, end to end: the field lists all three long after every
 # verdict landed. Only the stale one may survive the filter.
 expect "the never-cleared forgejo field collapses to who actually owes" \