diff --git a/actions/labels-reconcile/labels-reconcile.sh b/actions/labels-reconcile/labels-reconcile.sh index 72e8375..e154f13 100755 --- a/actions/labels-reconcile/labels-reconcile.sh +++ b/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) - # blocks at ANY head — GitHub's own semantic: only a newer review - # from the same reviewer clears it + CHANGES_REQUESTED | REQUEST_CHANGES) + # blocks at ANY head — both forges' semantic: only a newer review from + # the same reviewer clears it echo BLOCK ;; APPROVED) if [ "$commit" = "$HEAD_SHA" ]; then echo APPROVE; else echo STALE; fi ;; - *) - # COMMENTED and anything else: a non-verdict. The machine does not - # read bodies — if the comment is really an agreement, the AUTHOR - # says so by requesting the human's review. + COMMENTED | COMMENT) + # A comment is a non-verdict. The machine does not read bodies — if the + # comment is really an agreement, the AUTHOR says so by requesting the + # 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. diff --git a/changelog.d/235.md b/changelog.d/235.md new file mode 100644 index 0000000..49f9123 --- /dev/null +++ b/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). diff --git a/test/labels-reconcile.test.sh b/test/labels-reconcile.test.sh index 3f009b0..86869f4 100755 --- a/test/labels-reconcile.test.sh +++ b/test/labels-reconcile.test.sh @@ -182,7 +182,7 @@ REVIEWS_JSON="$(reviews \ "$(rev "$BOT3" REQUEST_REVIEW "" "" t3)")" REQUESTED="$(outstanding_requests "$BOT1 $BOT2 -$BOT3")" +$BOT3" 2>"$RTMP/request-round-log")" expect "three Forgejo request rows keep the opening round with the panel" \ state:bots-reviewing "$(round_state)" @@ -1709,7 +1709,7 @@ expect "a reviewer who never reviewed still owes one" "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")" + "$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)" \