From e5ebbf57fb16c43dcc2e0b120a516edd7dadbb84 Mon Sep 17 00:00:00 2001 From: codex-bot-andresmgsl Date: Sun, 23 Aug 2026 23:11:11 +0000 Subject: [PATCH 1/4] test(labels): reproduce Forgejo review vocabulary gaps --- test/labels-reconcile.test.sh | 100 ++++++++++++++++++++++++++++++++++ 1 file changed, 100 insertions(+) diff --git a/test/labels-reconcile.test.sh b/test/labels-reconcile.test.sh index 4874afb..3f009b0 100755 --- a/test/labels-reconcile.test.sh +++ b/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")" +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,46 @@ 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 APPROVED +# control proves the filter did not simply discard every row. +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-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 + } + 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" \ + APPROVED "$(jq -r 'map(.state) | join(",")' "$RTMP/gradeable-reviews.json")" + # --------------------------------------------------------------------------- # outstanding_requests — the portable "who still owes a verdict" (#188 term 4) # @@ -1615,6 +1706,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")" +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" \ -- 2.45.2 From 58e58f2ada83ace744406a2402f2040bc42b3520 Mon Sep 17 00:00:00 2001 From: codex-bot-andresmgsl Date: Sun, 23 Aug 2026 23:13:05 +0000 Subject: [PATCH 2/4] fix(labels): grade Forgejo review states --- actions/labels-reconcile/labels-reconcile.sh | 33 ++++++++++++++------ changelog.d/235.md | 3 ++ test/labels-reconcile.test.sh | 4 +-- 3 files changed, 28 insertions(+), 12 deletions(-) create mode 100644 changelog.d/235.md 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)" \ -- 2.45.2 From c2cca7c1be491d12abbb1281b9f4dfe8d2476fe9 Mon Sep 17 00:00:00 2001 From: codex-bot-andresmgsl Date: Sun, 23 Aug 2026 23:17:14 +0000 Subject: [PATCH 3/4] test(labels): mark indirect filter probe call --- test/labels-reconcile.test.sh | 1 + 1 file changed, 1 insertion(+) diff --git a/test/labels-reconcile.test.sh b/test/labels-reconcile.test.sh index 86869f4..25620f8 100755 --- a/test/labels-reconcile.test.sh +++ b/test/labels-reconcile.test.sh @@ -1669,6 +1669,7 @@ review_filter_probe() { *) printf '[]\n' ;; esac } + # shellcheck disable=SC2317 # main invokes the probe override indirectly outstanding_requests() { printf '%s\n' "$REVIEWS_JSON" >"$RTMP/gradeable-reviews.json" } -- 2.45.2 From 1cd46028ede9891ec34dc7af2f5b208aec1515ac Mon Sep 17 00:00:00 2001 From: codex-bot-andresmgsl Date: Sun, 23 Aug 2026 23:29:09 +0000 Subject: [PATCH 4/4] test(labels): guard Forgejo comment ingestion --- test/labels-reconcile.test.sh | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/test/labels-reconcile.test.sh b/test/labels-reconcile.test.sh index 25620f8..aaf18e8 100755 --- a/test/labels-reconcile.test.sh +++ b/test/labels-reconcile.test.sh @@ -1641,8 +1641,9 @@ expect "...naming the attempt that did not happen" \ # 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 APPROVED -# control proves the filter did not simply discard every row. +# 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 @@ -1661,6 +1662,7 @@ review_filter_probe() { *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"}, @@ -1678,7 +1680,7 @@ review_filter_probe() { } review_filter_probe expect "REQUEST_REVIEW is removed before REVIEWS_JSON reaches the grader" \ - APPROVED "$(jq -r 'map(.state) | join(",")' "$RTMP/gradeable-reviews.json")" + COMMENT,APPROVED "$(jq -r 'map(.state) | join(",")' "$RTMP/gradeable-reviews.json")" # --------------------------------------------------------------------------- # outstanding_requests — the portable "who still owes a verdict" (#188 term 4) -- 2.45.2