Merge pull request 'lib/forge-forgejo.sh + labels-reconcile — label removal is a full-set PUT, and a write that did not happen fails the sweep (#192)' (#206) from build/192-label-write into main

Reviewed-on: #206
Reviewed-by: codex-reviewer-andresmgsl <andres+2@heavyduty.builders>
Reviewed-by: kimi-reviewer-andresmgsl <andres+4@heavyduty.builders>

actions/labels-reconcile/labels-reconcile.sh

@@ -820,6 +820,27 @@ tree_version() { # $1 = ref → that tree's version via the API, or nothing
   return 0
 }
 
+# label_write <n> <args…> — every label mutation on this surface goes through
+# here (#192). A write that did not happen must reach main's exit code, and the
+# first version of this fix marked only the primary state edit: clearing
+# `merge-next` and the two `stale` edits could still fail into the generic
+# per-PR branch and finish with `reconciled.` and exit 0
+# (@codex-reviewer-andresmgsl). One helper means a future call site cannot
+# reopen that by forgetting to mark itself.
+#
+# The marker is a log line rather than a return code because reconcile_pr runs
+# in a subshell whose STDOUT main reads — the same channel the degraded-read
+# warning already travels on.
+label_write() {
+  local n="$1"
+  shift
+  if run forge_issue_edit "$n" "$@" >/dev/null; then
+    return 0
+  fi
+  log "#$n: label edit FAILED — attempted: forge_issue_edit $n $*; the write did not happen (reason on stderr above)"
+  return 1
+}
+
 reconcile_pr() { # $1 = PR number; relies on the globals set from its fetch
   local n="$1" desired remove s args last_activity last_activity_epoch age
 
@@ -896,11 +917,21 @@ reconcile_pr() { # $1 = PR number; relies on the globals set from its fetch
   if [ "$skip_edit" = false ] && { ! has_label "$desired" || [ -n "$remove" ] || [ -n "$add" ]; }; then
     args=(--add-label "$desired${add:+,$add}")
     [ -n "$remove" ] && args+=(--remove-label "$remove")
-    if run forge_issue_edit "$n" "${args[@]}" >/dev/null; then
+    if label_write "$n" "${args[@]}"; then
       log "#$n: state -> $desired${add:+ +$add}${remove:+ (cleared $remove)}"
     else
-      # a deleted label must not wedge the sweep — dispatch heals the taxonomy
+      # A WRITE THAT DID NOT HAPPEN IS FATAL, not a warning (#192). This was
-      log "#$n: WARNING: label edit failed (missing label? run the workflow manually to bootstrap)"
+      # `log WARNING` and fell through, so the sweep printed `reconciled.` and
+      # exited green over an edit the forge had refused — the
+      # degraded-write-reports-success class #188 exists to eliminate,
+      # surviving inside the reconciler that reports it.
+      #
+      # The old text also diagnosed a cause it had not established: it named a
+      # missing label and told the operator to bootstrap, when the label was
+      # present and the call had returned 500. #101's rule is report, do not
+      # diagnose — so this says what was attempted and that it did not happen,
+      # and leaves the backend's own stderr to say why.
+      return 1
     fi
   fi
 
@@ -919,7 +950,7 @@ reconcile_pr() { # $1 = PR number; relies on the globals set from its fetch
   # the moment the PR is no longer the thing a human should merge next, the
   # claim is removed. Setting it stays with whoever owns the queue.
   if has_label merge-next && [ "$desired" != state:needs-human ]; then
-    run forge_issue_edit "$n" --remove-label merge-next >/dev/null
+    label_write "$n" --remove-label merge-next || return 1
     log "#$n: cleared merge-next (state is $desired, not mergeable-by-a-human)"
   fi
 
@@ -945,11 +976,11 @@ reconcile_pr() { # $1 = PR number; relies on the globals set from its fetch
   # (#50 D10). The 7-day nudge is #52's, once for both surfaces.
   if has_label blocked || has_label needs-ruling || [ "$age" -le "$STALE_AFTER" ]; then
     if has_label stale; then
-      run forge_issue_edit "$n" --remove-label stale >/dev/null
+      label_write "$n" --remove-label stale || return 1
       log "#$n: unstale"
     fi
   elif ! has_label stale; then
-    run forge_issue_edit "$n" --add-label stale >/dev/null
+    label_write "$n" --add-label stale || return 1
     log "#$n: stale ($((age / 3600))h quiet)"
   fi
 
@@ -1001,7 +1032,7 @@ main() {
   [ -z "$REPO_LABELS" ] && log "WARNING: could not read the label set — applying labels unfiltered"
   missing_core_labels_warning "$(core_label_rows)" "$REPO_LABELS"
 
-  local n output status total=0 unreadable=0 sampled_reason=""
+  local n output status total=0 unreadable=0 write_failures=0 sampled_reason=""
   while IFS= read -r n; do
     [ -n "$n" ] || continue
     total=$((total + 1))
@@ -1087,10 +1118,33 @@ main() {
         sampled_reason="$(sed -n "s/^labels: #$n: read failed: //p" <<<"$output" | head -n1)"
       fi
     elif [ "$status" -ne 0 ]; then
+      # The per-PR tolerance is right and stays: one bad PR must not blind the
+      # sweep over the rest of the board. What was missing is the sweep-level
+      # accounting — a failed WRITE has to reach main's exit code, or a builder
+      # satisfies every task and the sweep still prints `reconciled.` over an
+      # edit that never happened (#192, @kimi-reviewer-andresmgsl #5189).
+      #
+      # Reads stay tolerated: an unreadable fact is already reported by the
+      # blind-sweep warning and leaves the board untouched. A write is
+      # different — the board and the tree now disagree.
+      if grep -q "^labels: #$n: label edit FAILED" <<<"$output"; then
+        write_failures=$((write_failures + 1))
+        log "#$n: reconcile failed on a WRITE — continuing the sweep, but it will not report success"
+      else
         log "#$n: reconcile failed — continuing with the remaining PRs"
       fi
+    fi
   done < <(forge_pr_list)
   blind_sweep_warning "$unreadable" "$total" "$sampled_reason"
+  if [ "$write_failures" -gt 0 ]; then
+    # The line must not contain the literal "reconciled." ANYWHERE — "NOT
+    # reconciled." still does, and a consumer grepping a job-log tail for that
+    # token would find it after a write that did not happen
+    # (@codex-reviewer-andresmgsl). The test asserts the whole output is free
+    # of it, not merely that the success prefix is absent.
+    log "$write_failures label write(s) attempted did not happen — sweep incomplete"
+    return 1
+  fi
   log "reconciled."
 }
 

changelog.d/192.md

@@ -0,0 +1,47 @@
+### Fixed
+
+- Label removal on Forgejo is a full-set `PUT`, not a per-label `DELETE`. The
+  workflow token gets HTTP 500 on every `DELETE .../labels/{id}` on this
+  instance, so the state machine could only ever ADD labels (#192).
+
+- Every `state:*` transition that needs the previous state cleared, and every
+  `blocker:*` that should lift, can now actually clear. They were inert (#192).
+
+- A label edit that fails is fatal to `labels-reconcile`, matching
+  `issueflow-reconcile`. One cause had two contradictory policies (#192).
+
+- A failed write reaches the sweep's exit code: per-PR tolerance is kept for
+  READS, but a sweep that could not write exits non-zero and its output carries
+  no `reconciled.` token at all (#192).
+
+- Every label mutation goes through one checked helper, so clearing
+  `merge-next` or either `stale` edit fails the sweep too — not only the
+  primary state edit (#192).
+
+- A preserved label keeps the id the issue payload already carried, so
+  preservation does not depend on a repository-wide list that has nothing to do
+  with the issue (#192).
+
+- A removal that changes nothing writes nothing, rather than replacing the set
+  with itself and opening a race for no state change (#192).
+
+- Every failure diagnostic on the forgejo backend names the verb as well as the
+  path and the status. A read used to say `HTTP 500 from 'repos/…'`, which
+  cannot be told from a failed write of the same path (#192).
+
+- The diagnostic names what was attempted and that it did not happen, instead
+  of blaming a missing label and telling the operator to bootstrap — a cause it
+  had not established (#192, #101).
+
+- An add-label the repo does not carry refuses before any write, so a
+  replacement `PUT` can never drop a label nobody asked to remove (#192).
+
+### Added
+
+- `test/forge-backends.test.sh` pins the replacement contract: preserve
+  unrelated labels across a combined add+remove, an absent removal as a
+  successful no-op, the empty set as a full clear, and `forge_labels_add`
+  still `POST`-only, per ceremony#128 (#192).
+
+- `test/labels-reconcile.test.sh` drives a failing write through `main()` — the
+  swallow was in the loop, where a fixture-level probe cannot reach (#192).

lib/forge-forgejo.sh

@@ -123,7 +123,7 @@ forge_api() {
       echo "forge_api: request failed: $endpoint" >&2
       return 1
     fi
-    forgejo_http_ok "$hdr" "$endpoint" || return 1
+    forgejo_http_ok "$hdr" "GET $endpoint" || return 1
     if [ "$have_jq" = true ]; then jq -r "$jqexpr" <"$body"; else cat "$body"; fi
     return 0
   fi
@@ -140,7 +140,7 @@ forge_api() {
       echo "forge_api: request failed: $endpoint (page $page)" >&2
       return 1
     fi
-    forgejo_http_ok "$hdr" "$endpoint" || return 1
+    forgejo_http_ok "$hdr" "GET $endpoint" || return 1
 
     # Re-read on EVERY page, not once (#4712). A board that changes size
     # under the walk was invisible: page 1 declaring 4 and page 2 declaring
@@ -223,9 +223,14 @@ EOF
   printf '%s\n' "$total"
 }
 
-# forgejo_http_ok <header-file> <endpoint> — a non-2xx is named, not
+# forgejo_http_ok <header-file> <verb-and-endpoint> — a non-2xx is named, not
 # swallowed. gh exits non-zero on HTTP failure; curl does not without -f,
 # and -f would throw away the body that says why.
+# The second argument carries the VERB as well as the path — "GET repos/…",
+# "PUT repos/…". #192's acceptance criterion is that a failure names the verb,
+# the path and the status, and reads used to omit the verb: a caller reading
+# `HTTP 500 from 'repos/o/r/issues/5'` could not tell a failed read from a
+# failed write of the same path (@codex-reviewer-andresmgsl).
 forgejo_http_ok() {
   local hdr="$1" endpoint="$2" code
   code="$(tr -d '\r' <"$hdr" | awk '/^HTTP\// { c = $2 } END { print c }')"
@@ -298,25 +303,94 @@ forge_issue_edit() {
     shift
   done
 
+  # THE LABEL DELTA (#192). Removal used to be a per-label
+  # `DELETE .../labels/{id}` loop. On this instance that call returns HTTP 500
+  # for EVERY removal under the token the sweep actually holds — measured
+  # under a real Actions token inside a workflow, probe run 701:
+  #
+  #   POST   /issues/{n}/labels  ["probe-a","probe-b"]  -> 200
+  #   DELETE /issues/{n}/labels/{id}                    -> 500   labels unchanged
+  #   PUT    /issues/{n}/labels  {"labels":[<id>]}      -> 200
+  #   PUT    /issues/{n}/labels  {"labels":[]}          -> 200   (full clear)
+  #
+  # A PAT gets 204 on the same DELETE, which is why this survived a week
+  # unseen: it fails only for `${{ github.token }}`, and only inside Actions.
+  # Net effect before this fix: on Forgejo the state machine could only ever
+  # ADD labels — every `state:*` transition needing the previous state cleared,
+  # and every `blocker:*` that should lift, was inert.
+  #
+  # So a removal is expressed as a full-set PUT, exactly as the assignee branch
+  # below expresses its own delta as one PATCH — read current, compute wanted,
+  # write once.
+  #
+  # AN ADD-ONLY CALL KEEPS ITS ADDITIVE POST, deliberately. ceremony#128 lost
+  # its `release` label — the merge door's declared-intent read — to a
+  # read-modify-write that clobbered a label set two seconds after a builder
+  # wrote it, and `forge_labels_add` is pinned against ever doing that
+  # (test/forge-backends.test.sh). The read-modify-write window is real and is
+  # accepted HERE and only here, where the caller has asked to REMOVE something
+  # and no additive verb can express that.
+  if [ "${#rm_labels[@]}" -gt 0 ]; then
+    local current want_pairs ids id name payload
+    local want_ids=() missing=()
+    # name<TAB>id straight from the ISSUE payload. Preserved labels carry
+    # their authoritative id here already, so they need no second lookup —
+    # re-resolving them through the repository-wide list would make
+    # preservation depend on a paginated read that has nothing to do with
+    # this issue, and an incomplete one would drop a bystander
+    # (@codex-reviewer-andresmgsl). Only ADDED names need forgejo_label_ids.
+    current="$(forge_api "repos/$REPO/issues/$n" \
+      --jq '[.labels[]? | "\(.name)\t\(.id)"] | join("\n")')" || return 1
+    # The rows are name<TAB>id, so removals filter on the NAME field — a
+    # whole-line match would never fire against a pair.
+    want_pairs="$(
+      awk -F '\t' 'NR==FNR { drop[$0]=1; next } !($1 in drop)' \
+        <(printf '%s\n' "${rm_labels[@]}") \
+        <(printf '%s\n' "$current" | grep -v '^$')
+    )"
+    while IFS=$'\t' read -r name id; do
+      [ -n "$name" ] || continue
+      want_ids+=("$id")
+    done <<<"$want_pairs"
     if [ "${#add_labels[@]}" -gt 0 ]; then
+      ids="$(forgejo_label_ids)" || return 1
+      for name in "${add_labels[@]}"; do
+        # already on the issue? its id is in want_ids already
+        awk -F '\t' -v want="$name" '$1 == want { found=1 } END { exit !found }' \
+          <<<"$want_pairs" && continue
+        id="$(awk -F '\t' -v want="$name" '$1 == want { print $2; exit }' <<<"$ids")"
+        if [ -z "$id" ]; then missing+=("$name"); continue; fi
+        want_ids+=("$id")
+      done
+    fi
+    # An add-label the repo does not carry refuses BEFORE the write. A PUT
+    # that silently dropped an unresolvable name would remove a label nobody
+    # asked to remove — a destructive write dressed as a partial success.
+    if [ "${#missing[@]}" -gt 0 ]; then
+      echo "forge_issue_edit: #$n: no label id on $REPO for: ${missing[*]} — refusing to PUT a set that would drop it" >&2
+      return 1
+    fi
+    # NOTHING TO CHANGE, NOTHING TO WRITE. The reconcilers call
+    # --remove-label unconditionally to converge state, so most calls here ask
+    # to remove a label the issue does not carry. Writing the unchanged set
+    # back would open the read-modify-write window of ceremony#128 for no
+    # state change at all; the GET above is already the proof the sweep
+    # reached the forge (@codex-reviewer-andresmgsl). gh's own behaviour on an
+    # absent --remove-label is likewise a no-op.
+    local current_ids
+    current_ids="$(printf '%s\n' "$current" | grep -v '^$' | cut -f2 | sort -n | tr '\n' ' ')"
+    if [ "$(printf '%s\n' ${want_ids[@]+"${want_ids[@]}"} | grep -v '^$' | sort -n | tr '\n' ' ')" = "$current_ids" ]; then
+      return 0
+    fi
+    payload="$(printf '%s\n' ${want_ids[@]+"${want_ids[@]}"} \
+      | jq -R 'select(. != "") | tonumber' | jq -sc '{labels: .}')"
+    forgejo_write PUT "repos/$REPO/issues/$n/labels" "$payload" >/dev/null || return 1
+  elif [ "${#add_labels[@]}" -gt 0 ]; then
     local payload
     payload="$(printf '%s\n' "${add_labels[@]}" | jq -R . | jq -sc '{labels: .}')"
     forgejo_write POST "repos/$REPO/issues/$n/labels" "$payload" >/dev/null || return 1
   fi
 
-  if [ "${#rm_labels[@]}" -gt 0 ]; then
-    local ids id name
-    ids="$(forgejo_label_ids)" || return 1
-    for name in "${rm_labels[@]}"; do
-      id="$(awk -F '\t' -v want="$name" '$1 == want { print $2; exit }' <<<"$ids")"
-      # A label the repo does not have is not an error: the reconcilers call
-      # --remove-label unconditionally to converge state, and gh's own
-      # behaviour there is a no-op.
-      [ -n "$id" ] || continue
-      forgejo_write DELETE "repos/$REPO/issues/$n/labels/$id" '' >/dev/null || return 1
-    done
-  fi
-
   if [ "${#add_assignees[@]}" -gt 0 ] || [ "${#rm_assignees[@]}" -gt 0 ]; then
     local current want payload
     current="$(forge_api "repos/$REPO/issues/$n" --jq '[.assignees[]?.login] | join("\n")')" || return 1

test/forge-backends.test.sh

@@ -257,9 +257,20 @@ stub_writes() {
       esac
       shift
     done
+    # FAKE_FAIL_URL + FAKE_HTTP fault-inject one endpoint, so the refusal
+    # boundaries are driven rather than assumed (#192 review).
+    if [ -n "${FAKE_FAIL_URL:-}" ] && [ "${url##*"$FAKE_FAIL_URL"}" != "$url" ]; then
+      printf 'HTTP/1.1 %s Server Error\r\n\r\n' "${FAKE_HTTP:-500}" >"$hdr"
+      printf '{}' >"$out"
+      [ "$method" = GET ] || printf '%s %s %s\n' "$method" "${url##*/api/v1/}" "$payload" >>"$WRITES"
+      return 0
+    fi
     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" ;;
+      # The issue itself: the removal path reads its CURRENT label set before
+      # computing the set to PUT (#192).
+      */issues/[0-9]*) printf '{"labels": %s}' "${FAKE_ISSUE_LABELS:-[]}" >"$out" ;;
       *) printf '{}' >"$out" ;;
     esac
     [ "$method" = GET ] || printf '%s %s %s\n' "$method" "${url##*/api/v1/}" "$payload" >>"$WRITES"
@@ -285,19 +296,111 @@ check "...carrying the updated description" 0 "" grep -q 'new text' "$WRITES"
 # #4751 item 2). Live scratch-repo evidence proved these work; these prove
 # they keep working, and pin the SHAPE of the requests.
 
-# Removal resolves name -> id, because Forgejo takes names on add and only a
+# Removal is a FULL-SET PUT, not a per-label DELETE (#192). Measured under a
-# numeric id on remove. Measured: DELETE .../labels/probe:one -> 422,
+# real Actions token, probe run 701: DELETE .../labels/{id} -> 500 for every
-# DELETE .../labels/149 -> 204.
+# removal, PUT .../labels -> 200 including the empty set. A PAT gets 204 on the
-FAKE_LABELS='[{"name":"stale","id":11},{"name":"ready","id":12}]' FAKE_LABEL_N=2 stub_writes
+# same DELETE, which is why it went unseen — it fails only for the identity the
-FAKE_LABELS='[{"name":"stale","id":11},{"name":"ready","id":12}]' FAKE_LABEL_N=2   REPO=o/r forge_issue_edit 5 --remove-label stale
+# sweep holds.
-check "removing a label resolves its numeric id" 0 ""   grep -q '^DELETE repos/o/r/issues/5/labels/11 ' "$WRITES"
+ROSTER='[{"name":"state:old","id":11},{"name":"state:new","id":12},{"name":"scope:labels","id":13},{"name":"attention","id":14}]'
-check "...and never sends the name as the path segment" 1 ""   grep -q 'labels/stale' "$WRITES"
+FAKE_LABELS="$ROSTER" FAKE_LABEL_N=4 stub_writes
+FAKE_LABELS="$ROSTER" FAKE_LABEL_N=4 FAKE_ISSUE_LABELS='[{"name":"state:old","id":11},{"name":"scope:labels","id":13}]' \
+  REPO=o/r forge_issue_edit 5 --remove-label state:old
+check "removing a label PUTs the whole wanted set" 0 "" \
+  grep -q '^PUT repos/o/r/issues/5/labels ' "$WRITES"
+check "...and never DELETEs, which this instance answers 500" 1 "" \
+  grep -q '^DELETE ' "$WRITES"
+check "...carrying the surviving label's id and not the removed one" 0 '{"labels":[13]}' \
+  cat "$WRITES"
 
-# A label the repo does not have is a no-op, matching gh: the reconcilers
+# The contract @codex-reviewer-andresmgsl asked for (#5183): a full-set PUT
-# call --remove-label unconditionally to converge state.
+# replaces everything, so removal alone proves nothing about PRESERVATION. One
-FAKE_LABELS='[{"name":"ready","id":12}]' FAKE_LABEL_N=1 stub_writes
+# call, a combined delta, and two bystanders that must survive it.
-FAKE_LABELS='[{"name":"ready","id":12}]' FAKE_LABEL_N=1   REPO=o/r forge_issue_edit 5 --remove-label nonexistent
+FAKE_LABELS="$ROSTER" FAKE_LABEL_N=4 stub_writes
-check "removing an absent label writes nothing" 0 "" test ! -s "$WRITES"
+FAKE_LABELS="$ROSTER" FAKE_LABEL_N=4 \
+  FAKE_ISSUE_LABELS='[{"name":"state:old","id":11},{"name":"scope:labels","id":13},{"name":"attention","id":14}]' \
+  REPO=o/r forge_issue_edit 5 --remove-label state:old --add-label state:new
+check "a combined add+remove is ONE write" 0 "" test "$(wc -l <"$WRITES")" -eq 1
+preserves_bystanders() { # the PUT keeps state:new(12), scope:labels(13), attention(14)
+  grep -q 12 "$WRITES" && grep -q 13 "$WRITES" && grep -q 14 "$WRITES"
+}
+check "...and preserves every unrelated label" 0 "" preserves_bystanders
+check "...while dropping only what was asked for" 1 "" grep -qE '(^|[^0-9])11([^0-9]|$)' "$WRITES"
+
+# A label the issue does not carry is a successful no-op that writes NOTHING,
+# matching gh: the reconcilers call --remove-label unconditionally to converge
+# state, so most calls here ask to remove something absent. Writing the
+# unchanged set back would open ceremony#128's read-modify-write window for no
+# state change at all, and the GET above is already the proof the sweep reached
+# the forge (@codex-reviewer-andresmgsl, #192 review).
+FAKE_LABELS="$ROSTER" FAKE_LABEL_N=4 stub_writes
+FAKE_LABELS="$ROSTER" FAKE_LABEL_N=4 FAKE_ISSUE_LABELS='[{"name":"scope:labels","id":13}]' \
+  REPO=o/r forge_issue_edit 5 --remove-label state:old
+check "removing an absent label succeeds" 0 "" test "$?" -eq 0
+check "...writing nothing at all" 0 "" test ! -s "$WRITES"
+
+# A full clear is the empty set, which this instance answers 200 (run 701).
+FAKE_LABELS="$ROSTER" FAKE_LABEL_N=4 stub_writes
+FAKE_LABELS="$ROSTER" FAKE_LABEL_N=4 FAKE_ISSUE_LABELS='[{"name":"state:old","id":11}]' \
+  REPO=o/r forge_issue_edit 5 --remove-label state:old
+check "clearing the last label PUTs the empty set" 0 '{"labels":[]}' cat "$WRITES"
+
+# An add-label the repo does not have must refuse BEFORE any write: a PUT that
+# silently dropped an unresolvable name would remove a label nobody asked to
+# remove — a destructive write dressed as a partial success.
+FAKE_LABELS="$ROSTER" FAKE_LABEL_N=4 stub_writes
+edit_unknown_add() {
+  FAKE_LABELS="$ROSTER" FAKE_LABEL_N=4 FAKE_ISSUE_LABELS='[{"name":"state:old","id":11}]' \
+    REPO=o/r forge_issue_edit 5 --remove-label state:old --add-label no-such-label
+}
+check "an unknown add-label refuses" 1 "no label id" edit_unknown_add
+check "...before writing anything" 0 "" test ! -s "$WRITES"
+
+# The preserved-id contract, and the reason it is not merely an optimisation
+# (@codex-reviewer-andresmgsl, #192 review): a bystander's id comes from the
+# ISSUE payload, so preservation must not depend on a repository-wide list
+# that has nothing to do with this issue. Here `attention` is on the issue with
+# id 14 and is ABSENT from the repo-list fixture entirely — a resolution that
+# went through forgejo_label_ids would refuse or drop it.
+PARTIAL_ROSTER='[{"name":"state:old","id":11},{"name":"state:new","id":12},{"name":"scope:labels","id":13}]'
+FAKE_LABELS="$PARTIAL_ROSTER" FAKE_LABEL_N=3 stub_writes
+FAKE_LABELS="$PARTIAL_ROSTER" FAKE_LABEL_N=3 \
+  FAKE_ISSUE_LABELS='[{"name":"state:old","id":11},{"name":"attention","id":14}]' \
+  REPO=o/r forge_issue_edit 5 --remove-label state:old
+check "a bystander absent from the repo list is still preserved by its issue id" 0 \
+  '{"labels":[14]}' cat "$WRITES"
+
+# The two fault boundaries the acceptance plan names. Both must be non-zero
+# with the backend's own diagnostic, and neither may report success.
+fail_get() {
+  FAKE_LABELS="$ROSTER" FAKE_LABEL_N=4 FAKE_HTTP=500 FAKE_FAIL_URL='/issues/5' \
+    REPO=o/r forge_issue_edit 5 --remove-label state:old
+}
+check "a failed current-label GET refuses, non-zero" 1 "" fail_get
+check "...naming the verb, the path AND the status, in one diagnostic" 1 \
+  "HTTP 500 from 'GET repos/o/r/issues/5'" fail_get
+get_write_count() { : >"$WRITES"; fail_get >/dev/null 2>&1; wc -l <"$WRITES"; }
+check "...having written nothing: the read failed before any mutation" 0 "0" \
+  get_write_count
+fail_put() {
+  FAKE_LABELS="$ROSTER" FAKE_LABEL_N=4 \
+    FAKE_ISSUE_LABELS='[{"name":"state:old","id":11},{"name":"scope:labels","id":13}]' \
+    FAKE_HTTP=500 FAKE_FAIL_URL='/issues/5/labels' \
+    REPO=o/r forge_issue_edit 5 --remove-label state:old
+}
+check "a failed replacement PUT refuses, non-zero" 1 "" fail_put
+check "...naming the verb, the path AND the status, in one diagnostic" 1 \
+  "HTTP 500 from 'PUT repos/o/r/issues/5/labels'" fail_put
+put_write_count() { : >"$WRITES"; fail_put >/dev/null 2>&1; wc -l <"$WRITES"; }
+check "...having attempted only the one PUT" 0 "1" put_write_count
+
+# An ADD-ONLY call keeps the additive POST (ceremony#128): a read-modify-write
+# there clobbered a label set two seconds after a builder wrote it.
+FAKE_LABELS="$ROSTER" FAKE_LABEL_N=4 stub_writes
+FAKE_LABELS="$ROSTER" FAKE_LABEL_N=4 FAKE_ISSUE_LABELS='[{"name":"scope:labels","id":13}]' \
+  REPO=o/r forge_issue_edit 5 --add-label state:new
+check "an add-only edit still POSTs additively" 0 "" \
+  grep -q '^POST repos/o/r/issues/5/labels ' "$WRITES"
+check "...and never PUTs the whole set (ceremony#128)" 1 "" grep -q '^PUT ' "$WRITES"
 
 # Adding takes names directly — no lookup, one request.
 FAKE_LABELS='[]' FAKE_LABEL_N=0 stub_writes

test/labels-reconcile.test.sh

@@ -1456,6 +1456,132 @@ expect "...and a malformed panel= line in that same file is refused, not passed"
 
 
 
+# ---------------------------------------------------------------------------
+# A WRITE THAT DID NOT HAPPEN FAILS THE SWEEP (#192)
+#
+# The defect this replaces: forge_issue_edit returned non-zero, reconcile_pr
+# logged `WARNING: label edit failed`, fell through, and main printed
+# `reconciled.` and exited 0. On this forge that was every removal — the
+# workflow token gets HTTP 500 on DELETE .../labels/{id} — so the board could
+# only ever gain labels, and the sweep said it had reconciled.
+#
+# Driven through main() rather than the pure functions, because the swallow
+# was in the LOOP: reconcile_pr's non-zero became a log line and the loop
+# finished. A fixture-level probe cannot reach that (#91's lesson).
+# ---------------------------------------------------------------------------
+write_fail_probe() { # $1 = ok | fail — whether the label edit write succeeds
+  (
+    GITHUB_EVENT_NAME=schedule
+    REPO=owner/repo
+    LABELS_CONF=.github/labels.conf
+    CEREMONY_FORGE=github
+    WMODE="$1"
+    # 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' 401 402; return 0; fi
+      if [ "$1" = pr ] && [ "$2" = view ]; then
+        jq -n '{mergeable:"MERGEABLE",
+                statusCheckRollup:[{__typename:"CheckRun",workflowName:"ci",
+                                    name:"check",conclusion:"SUCCESS",
+                                    startedAt:"2026-07-01T00:00:00Z"}]}'
+        return 0
+      fi
+      if [ "$1" = issue ] && [ "$2" = edit ]; then
+        # #401's write is the one that fails; #402's succeeds, so the probe
+        # also proves the sweep KEPT GOING rather than aborting on the first.
+        if [ "$WMODE" = fail ] && [ "$3" = 401 ]; then
+          printf 'forge_api: HTTP 500 from DELETE repos/owner/repo/issues/401/labels/93\n' >&2
+          return 1
+        fi
+        printf '%s\n' "$*" >>"$RTMP/wedits"
+        return 0
+      fi
+      case "$*" in
+        */pulls/401) jq -n '{draft:false,user:{login:"author"},head:{sha:"h"},base:{sha:"b"},
+                             labels:[{name:"blocker:ci-red"}],requested_reviewers:[],
+                             created_at:"2026-07-01T00:00:00Z"}' ;;
+        */pulls/402) jq -n '{draft:false,user:{login:"author"},head:{sha:"h"},base:{sha:"b"},
+                             labels:[],requested_reviewers:[],
+                             created_at:"2026-07-01T00:00:00Z"}' ;;
+        *) printf '[]\n' ;;
+      esac
+    }
+    main
+  )
+}
+
+: >"$RTMP/wedits"
+wf_rc=0
+wf_out="$(write_fail_probe fail 2>&1)" || wf_rc=$?
+expect "a sweep whose label write failed exits non-zero" 1 "$wf_rc"
+expect "...and the output contains no 'reconciled.' token anywhere" \
+  no "$(grep -qF 'reconciled.' <<<"$wf_out" && echo yes || echo no)"
+expect "...saying instead that the write did not happen" \
+  yes "$(grep -q 'did not happen' <<<"$wf_out" && echo yes || echo no)"
+expect "...naming what it attempted, not a cause it has not established" \
+  yes "$(grep -q 'attempted: forge_issue_edit 401' <<<"$wf_out" && echo yes || echo no)"
+expect "...and asserting no bootstrap diagnosis it cannot support" \
+  no "$(grep -qi 'bootstrap' <<<"$wf_out" && echo yes || echo no)"
+# The per-PR tolerance is deliberately KEPT: one bad PR must not blind the
+# board. #402 is reconciled in the same pass that #401 failed in.
+expect "...while still reconciling the rest of the board" \
+  yes "$(grep -q '^issue edit 402' "$RTMP/wedits" && echo yes || echo no)"
+
+: >"$RTMP/wedits"
+ok_rc=0
+ok_out="$(write_fail_probe ok 2>&1)" || ok_rc=$?
+expect "the control: the same sweep with writes working exits 0" 0 "$ok_rc"
+expect "...and does print reconciled." \
+  yes "$(grep -q 'labels: reconciled\.' <<<"$ok_out" && echo yes || echo no)"
+
+# The tally must catch EVERY label mutation, not only the primary state edit.
+# `stale` is written from a different call site; before the one-helper change
+# a failure there fell into the generic per-PR branch and the sweep still
+# exited 0 (@codex-reviewer-andresmgsl, #192 review).
+stale_fail_probe() {
+  (
+    GITHUB_EVENT_NAME=schedule
+    REPO=owner/repo
+    LABELS_CONF=.github/labels.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' 501; return 0; fi
+      if [ "$1" = pr ] && [ "$2" = view ]; then
+        jq -n '{mergeable:"MERGEABLE",
+                statusCheckRollup:[{__typename:"CheckRun",workflowName:"ci",
+                                    name:"check",conclusion:"SUCCESS",
+                                    startedAt:"2026-07-01T00:00:00Z"}]}'
+        return 0
+      fi
+      if [ "$1" = issue ] && [ "$2" = edit ]; then
+        # only the `stale` write fails; the primary state edit succeeds, so
+        # this probe proves the NON-primary site reaches the tally
+        case "$*" in
+          *stale*) printf 'forge_api: HTTP 500 from PUT repos/owner/repo/issues/501/labels\n' >&2; return 1 ;;
+        esac
+        return 0
+      fi
+      case "$*" in
+        */pulls/501) jq -n '{draft:false,user:{login:"author"},head:{sha:"h"},base:{sha:"b"},
+                             labels:[{name:"stale"},{name:"blocked"}],requested_reviewers:[],
+                             created_at:"2026-07-01T00:00:00Z"}' ;;
+        *) printf '[]\n' ;;
+      esac
+    }
+    main
+  )
+}
+sf_rc=0
+sf_out="$(stale_fail_probe 2>&1)" || sf_rc=$?
+expect "a failed NON-primary label write also fails the sweep" 1 "$sf_rc"
+expect "...with no 'reconciled.' token in the output" \
+  no "$(grep -qF 'reconciled.' <<<"$sf_out" && echo yes || echo no)"
+expect "...naming the attempt that did not happen" \
+  yes "$(grep -q 'label edit FAILED' <<<"$sf_out" && echo yes || echo no)"
+
 # ---------------------------------------------------------------------------
 # outstanding_requests — the portable "who still owes a verdict" (#188 term 4)
 #