fix(forge): parity gaps in the forgejo verbs — upsert, timestamps, typos

@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
This commit is contained in:
cluade-reviewer-andresmgsl 2026-08-02 19:22:28 +00:00
parent adf3299192
commit a968e13ca4
2 changed files with 143 additions and 9 deletions

View file

@ -245,13 +245,31 @@ forge_issue_edit() {
local n="${1:?forge_issue_edit: number required}" local n="${1:?forge_issue_edit: number required}"
shift shift
local add_labels=() rm_labels=() add_assignees=() rm_assignees=() v 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 while [ $# -gt 0 ]; do
case "$1" in case "$1" in
--add-label) IFS=, read -ra v <<<"$2"; add_labels+=("${v[@]}"); shift ;; --add-label | --remove-label | --add-assignee | --remove-assignee)
--remove-label) IFS=, read -ra v <<<"$2"; rm_labels+=("${v[@]}"); shift ;; if [ "$#" -lt 2 ]; then
--add-assignee) IFS=, read -ra v <<<"$2"; add_assignees+=("${v[@]}"); shift ;; echo "forge_issue_edit: $1 requires a value (#188)" >&2
--remove-assignee) IFS=, read -ra v <<<"$2"; rm_assignees+=("${v[@]}"); shift ;; 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 esac
shift shift
done done
@ -320,7 +338,14 @@ forge_pr_view() {
$st.statuses[]? | { $st.statuses[]? | {
__typename: "StatusContext", __typename: "StatusContext",
context: .context, 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_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() { forge_label_create() {
local name="${1:?}" color="${2:?}" desc="${3:-}" local name="${1:?}" color="${2:?}" desc="${3:-}" ids id payload
forgejo_write POST "repos/$REPO/labels" \ payload="$(jq -n --arg n "$name" --arg c "$color" --arg d "$desc" '{name:$n,color:$c,description:$d}')"
"$(jq -n --arg n "$name" --arg c "$color" --arg d "$desc" '{name:$n,color:$c,description:$d}')" >/dev/null 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() { forge_label_delete() {

View file

@ -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 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' 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 ----------------------------------------- # --- the api base must be known -----------------------------------------
check "no api base refuses" 1 "cannot reach the forge" \ 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' bash -c 'unset CEREMONY_FORGE_API GITHUB_API_URL; . '"$ROOT"'/lib/forge-forgejo.sh; forgejo_api_base'