cluade-reviewer-andresmgsl
0f20f4b6ef
Two defects, one cause, and the second is why the first survived a week.
THE WRITE. Removal was 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 inside Actions, probe run 701, where the same
`PUT .../labels` with the desired full set returns 200 including the empty set
for a full clear. A PAT gets 204 on the same DELETE, which is exactly why it
went unseen: it fails only for `${{ github.token }}`.
Net effect before this: 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. Both PRs open right now carry stale
`blocker:*` labels that are false and that nothing can remove.
So the removal path is read-current, compute-wanted, one PUT — the same shape
the assignee branch beside it already used. An ADD-ONLY call keeps its additive
POST: ceremony#128 lost a `release` label to a read-modify-write that clobbered
a concurrent set, and forge_labels_add stays pinned against ever doing that.
The window is accepted here and only here, where the caller asked to REMOVE
and no additive verb can say that. An unresolvable --add-label refuses before
any write, so a replacement PUT can never drop a label nobody asked to remove.
THE REPORTING. `labels-reconcile` logged `WARNING: label edit failed`, fell
through, and `main` printed `reconciled.` and exited 0 — while
`issueflow-reconcile` treated the identical 500 as fatal. One cause, two
contradictory policies, and the wrong one hid the write defect.
A failed write is fatal now, and the tally reaches main's exit code. That
second half is load-bearing: making reconcile_pr fatal alone is not enough,
because the loop swallows a per-PR non-zero into a log line and finishes. The
per-PR tolerance is right and stays — one bad PR must not blind the board — but
it now applies to READS. A sweep that could not write exits non-zero and never
prints `reconciled.`
The diagnostic says what was attempted and that it did not happen. The old text
blamed a missing label and told the operator to bootstrap, when the label was
present and the call returned 500 — #101's rule is report, do not diagnose.
Mutation-tested, all three ways: restoring the warn-and-continue reds 5 cases,
removing the tally reds 2, restoring the DELETE loop reds 7.
test/run.sh 22 files 0 failed under jq 1.7 and jq 1.6; shellcheck 0.10.0 and
actionlint clean.
Refs #192
1.4 KiB
Fixed
-
Label removal on Forgejo is a full-set
PUT, not a per-labelDELETE. The workflow token gets HTTP 500 on everyDELETE .../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 everyblocker:*that should lift, can now actually clear. They were inert (#192). -
A label edit that fails is fatal to
labels-reconcile, matchingissueflow-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 never prints
reconciled.(#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
PUTcan never drop a label nobody asked to remove (#192).
Added
-
test/forge-backends.test.shpins 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, andforge_labels_addstillPOST-only (ceremony#128) (#192). -
test/labels-reconcile.test.shdrives a failing write throughmain()— the swallow was in the loop, where a fixture-level probe cannot reach (#192).