From a968e13ca4400f850b558720c1a53f56ef9b1062 Mon Sep 17 00:00:00 2001
From: cluade-reviewer-andresmgsl <andres+1@heavyduty.builders>
Date: Sun, 2 Aug 2026 19:22:28 +0000
Subject: [PATCH] =?UTF-8?q?fix(forge):=20parity=20gaps=20in=20the=20forgej?=
 =?UTF-8?q?o=20verbs=20=E2=80=94=20upsert,=20timestamps,=20typos?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

@codex-reviewer-andresmgsl's three findings (#4743), all real.

1. forge_label_create is now an UPSERT, matching gh label create --force.
   bootstrap_labels creates every declared label on EVERY workflow_dispatch,
   so a plain POST onto an existing name aborted the bootstrap under set -e
   from the second dispatch onward. Resolves name -> id and PATCHes when it
   exists.

2. forge_pr_view carries createdAt/completedAt. checks_state groups repeated
   contexts and selects the newest by [.startedAt, .createdAt, .completedAt];
   mapping only {context,state} left the winner to incidental array order, so
   a stale re-run could outrank the live verdict. The combined status carries
   created_at and updated_at — measured.

3. forge_issue_edit refuses unknown flags and missing values. The github
   backend hands them to gh, which fails; dropping them here turned a
   mis-typed port site into a mutation that silently did not happen — this
   issue's own failure class, inside the fix for it.

Also settles @grok-reviewer-andresmgsl's note 3 (#4741): Forgejo Actions DO
land as commit statuses on this instance, so the rollup is not empty.
rig main carries four — "ci / check (push)" and siblings, state success,
each with created_at. statusCheckRollup therefore populates, and NONE is not
silently substituted for SUCCESS.

Each fix mutation-verified: dropping the timestamps, forcing POST-always, and
restoring the silent flag skip each red exactly their own cases. The
newest-verdict case drives the real checks_state, not a copy.

Refs #188
---
 lib/forge-forgejo.sh        | 53 ++++++++++++++++----
 test/forge-backends.test.sh | 99 +++++++++++++++++++++++++++++++++++++
 2 files changed, 143 insertions(+), 9 deletions(-)

diff --git a/lib/forge-forgejo.sh b/lib/forge-forgejo.sh
index fd16e99..5d06ad0 100644
--- a/lib/forge-forgejo.sh
+++ b/lib/forge-forgejo.sh
@@ -245,13 +245,31 @@ forge_issue_edit() {
   local n="${1:?forge_issue_edit: number required}"
   shift
   local add_labels=() rm_labels=() add_assignees=() rm_assignees=() v
+  # Unknown flags REFUSE (#4743). The github backend forwards whatever it is
+  # given to `gh`, which fails on a flag it does not know; dropping it here
+  # instead would turn a port typo into a green no-op — a mutation that
+  # silently did not happen, which is precisely this issue's failure class
+  # arriving inside the fix for it.
   while [ $# -gt 0 ]; do
     case "$1" in
-      --add-label) IFS=, read -ra v <<<"$2"; add_labels+=("${v[@]}"); shift ;;
-      --remove-label) IFS=, read -ra v <<<"$2"; rm_labels+=("${v[@]}"); shift ;;
-      --add-assignee) IFS=, read -ra v <<<"$2"; add_assignees+=("${v[@]}"); shift ;;
-      --remove-assignee) IFS=, read -ra v <<<"$2"; rm_assignees+=("${v[@]}"); shift ;;
-      *) ;;
+      --add-label | --remove-label | --add-assignee | --remove-assignee)
+        if [ "$#" -lt 2 ]; then
+          echo "forge_issue_edit: $1 requires a value (#188)" >&2
+          return 1
+        fi
+        IFS=, read -ra v <<<"$2"
+        case "$1" in
+          --add-label) add_labels+=("${v[@]}") ;;
+          --remove-label) rm_labels+=("${v[@]}") ;;
+          --add-assignee) add_assignees+=("${v[@]}") ;;
+          --remove-assignee) rm_assignees+=("${v[@]}") ;;
+        esac
+        shift
+        ;;
+      *)
+        echo "forge_issue_edit: unknown flag '$1' — refusing rather than silently skipping the edit (#188)" >&2
+        return 1
+        ;;
     esac
     shift
   done
@@ -320,7 +338,14 @@ forge_pr_view() {
         $st.statuses[]? | {
           __typename: "StatusContext",
           context: .context,
-          state: (.status | ascii_upcase)
+          state: (.status | ascii_upcase),
+          # checks_state groups repeated contexts and takes the NEWEST by
+          # [.startedAt, .createdAt, .completedAt]. Without a timestamp the
+          # winner would be decided by incidental array order, so a stale
+          # re-run could outrank the live verdict (#4743). The combined
+          # status carries both fields; measured on this instance.
+          createdAt: .created_at,
+          completedAt: .updated_at
         }
       ]
     }'
@@ -328,10 +353,20 @@ forge_pr_view() {
 
 forge_label_list() { forge_api --paginate "repos/$REPO/labels" --jq '.[].name'; }
 
+# forge_label_create — an UPSERT, matching `gh label create --force` (#4743).
+# bootstrap_labels creates every declared label on every workflow_dispatch, so
+# the second dispatch must update rather than conflict; a plain POST onto an
+# existing name aborts the bootstrap under set -e.
 forge_label_create() {
-  local name="${1:?}" color="${2:?}" desc="${3:-}"
-  forgejo_write POST "repos/$REPO/labels" \
-    "$(jq -n --arg n "$name" --arg c "$color" --arg d "$desc" '{name:$n,color:$c,description:$d}')" >/dev/null
+  local name="${1:?}" color="${2:?}" desc="${3:-}" ids id payload
+  payload="$(jq -n --arg n "$name" --arg c "$color" --arg d "$desc" '{name:$n,color:$c,description:$d}')"
+  ids="$(forgejo_label_ids)" || return 1
+  id="$(awk -F '\t' -v want="$name" '$1 == want { print $2; exit }' <<<"$ids")"
+  if [ -n "$id" ]; then
+    forgejo_write PATCH "repos/$REPO/labels/$id" "$payload" >/dev/null
+  else
+    forgejo_write POST "repos/$REPO/labels" "$payload" >/dev/null
+  fi
 }
 
 forge_label_delete() {
diff --git a/test/forge-backends.test.sh b/test/forge-backends.test.sh
index 8e29027..d4c6ce4 100644
--- a/test/forge-backends.test.sh
+++ b/test/forge-backends.test.sh
@@ -217,6 +217,105 @@ curl() {
 check "a 404 is a named failure" 1 "HTTP 404" forge_api 'repos/o/r/issues/9999'
 check "a 404 names the endpoint" 1 "repos/o/r/issues/9999" forge_api 'repos/o/r/issues/9999'
 
+# --- forge_issue_edit: a typo must not become a green no-op --------------
+# @codex-reviewer-andresmgsl (#4743). The github backend hands whatever it is
+# given to `gh`, which fails on a flag it does not know. Dropping it here
+# instead turned a mis-typed port site into a mutation that silently did not
+# happen — this issue's own failure class, arriving inside the fix for it.
+
+check "an unknown edit flag refuses" 1 "unknown flag" forge_issue_edit 1 --typo value
+check "...and names the flag it refused" 1 "--typo" forge_issue_edit 1 --typo value
+check "a flag with no value refuses" 1 "requires a value" forge_issue_edit 1 --add-label
+
+# --- forge_label_create: an upsert, like gh's --force --------------------
+# bootstrap_labels creates every declared label on EVERY workflow_dispatch,
+# so a plain POST onto an existing name aborts the bootstrap under set -e
+# from the second dispatch onward (#4743).
+
+WRITES="$TMP/writes"
+stub_writes() {
+  : >"$WRITES"
+  # shellcheck disable=SC2317 # invoked indirectly, by the forge verbs
+  curl() {
+    local hdr="" out="" method=GET url="" payload=""
+    while [ $# -gt 0 ]; do
+      case "$1" in
+        -D) hdr="$2"; shift ;;
+        -o) out="$2"; shift ;;
+        -X) method="$2"; shift ;;
+        -d) payload="$2"; shift ;;
+        -H) shift ;;
+        -*) ;;
+        *) url="$1" ;;
+      esac
+      shift
+    done
+    printf 'HTTP/1.1 200 OK\r\nX-Total-Count: %s\r\n\r\n' "${FAKE_LABEL_N:-1}" >"$hdr"
+    case "$url" in
+      *"/labels?"* | */labels) printf '%s' "${FAKE_LABELS:-[]}" >"$out" ;;
+      *) printf '{}' >"$out" ;;
+    esac
+    [ "$method" = GET ] || printf '%s %s %s\n' "$method" "${url##*/api/v1/}" "$payload" >>"$WRITES"
+    return 0
+  }
+}
+
+# The label does not exist yet -> POST (create).
+FAKE_LABELS='[]' FAKE_LABEL_N=0 stub_writes
+FAKE_LABELS='[]' FAKE_LABEL_N=0 REPO=o/r forge_label_create ready 0e8a16 'in the queue'
+check "creating a new label POSTs" 0 "" grep -q '^POST repos/o/r/labels ' "$WRITES"
+
+# The label already exists -> PATCH (update), which is what --force does.
+FAKE_LABELS='[{"name":"ready","id":7}]' FAKE_LABEL_N=1 stub_writes
+FAKE_LABELS='[{"name":"ready","id":7}]' FAKE_LABEL_N=1 REPO=o/r forge_label_create ready 0e8a16 'new text'
+check "recreating an existing label PATCHes it" 0 "" \
+  grep -q '^PATCH repos/o/r/labels/7 ' "$WRITES"
+check "...and does not POST a duplicate" 1 "" grep -q '^POST repos/o/r/labels ' "$WRITES"
+check "...carrying the updated description" 0 "" grep -q 'new text' "$WRITES"
+
+# --- forge_pr_view: newest verdict per context must win ------------------
+# checks_state groups repeated contexts and selects the newest by
+# [.startedAt, .createdAt, .completedAt]. Mapping only {context,state} left
+# the winner to incidental array order, so a stale re-run could outrank the
+# live one (#4743). Forgejo's combined status carries created_at/updated_at
+# — measured on this instance, where Actions DO land as commit statuses
+# (rig main: "ci / check (push)" success, with created_at).
+pr_view_stub() {
+  # shellcheck disable=SC2317 # invoked indirectly, by forge_pr_view
+  curl() {
+    local hdr="" out="" url=""
+    while [ $# -gt 0 ]; do
+      case "$1" in -D) hdr="$2"; shift ;; -o) out="$2"; shift ;; -H) shift ;; *) url="$1" ;; esac
+      shift
+    done
+    printf 'HTTP/1.1 200 OK\r\nX-Total-Count: 1\r\n\r\n' >"$hdr"
+    case "$url" in
+      */status) printf '%s' "$FAKE_STATUS" >"$out" ;;
+      *) printf '{"head":{"sha":"abc"},"mergeable":true}' >"$out" ;;
+    esac
+    return 0
+  }
+}
+# The FAILURE is older but listed second — array order would pick it.
+FAKE_STATUS='{"state":"failure","statuses":[
+  {"context":"ci / check","status":"success","created_at":"2026-08-02T10:00:00Z","updated_at":"2026-08-02T10:00:00Z"},
+  {"context":"ci / check","status":"failure","created_at":"2026-08-02T09:00:00Z","updated_at":"2026-08-02T09:00:00Z"}]}'
+pr_view_stub
+view_json="$(REPO=o/r forge_pr_view 5)"
+check "pr_view maps createdAt" 0 "" \
+  grep -q '"createdAt": "2026-08-02T10:00:00Z"' <<<"$view_json"
+check "pr_view maps completedAt" 0 "" \
+  grep -q '"completedAt":' <<<"$view_json"
+check "pr_view maps mergeable to the UI string" 0 "" \
+  grep -q '"mergeable": "MERGEABLE"' <<<"$view_json"
+# The real proof: feed it to the production classifier and confirm the newer
+# SUCCESS wins over the older FAILURE regardless of array order.
+# shellcheck source=actions/labels-reconcile/labels-reconcile.sh
+. "$ROOT/actions/labels-reconcile/labels-reconcile.sh"
+classified="$(checks_state <<<"$view_json")"
+check "the newest verdict per context wins, not the array order" 0 "" \
+  test "$classified" = SUCCESS
+
 # --- the api base must be known -----------------------------------------
 check "no api base refuses" 1 "cannot reach the forge" \
   bash -c 'unset CEREMONY_FORGE_API GITHUB_API_URL; . '"$ROOT"'/lib/forge-forgejo.sh; forgejo_api_base'