Merge remote-tracking branch 'origin/main' into build/200-upstream-sync-doc
This commit is contained in:
commit
f8f318c634
5 changed files with 441 additions and 37 deletions
|
|
@ -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
|
||||
log "#$n: WARNING: label edit failed (missing label? run the workflow manually to bootstrap)"
|
||||
# A WRITE THAT DID NOT HAPPEN IS FATAL, not a warning (#192). This was
|
||||
# `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
|
||||
log "#$n: reconcile failed — continuing with the remaining PRs"
|
||||
# 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."
|
||||
}
|
||||
|
||||
|
|
|
|||
47
changelog.d/192.md
Normal file
47
changelog.d/192.md
Normal file
|
|
@ -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).
|
||||
|
|
@ -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
|
||||
|
||||
if [ "${#add_labels[@]}" -gt 0 ]; then
|
||||
# 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
|
||||
|
|
|
|||
|
|
@ -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
|
||||
# numeric id on remove. Measured: DELETE .../labels/probe:one -> 422,
|
||||
# DELETE .../labels/149 -> 204.
|
||||
FAKE_LABELS='[{"name":"stale","id":11},{"name":"ready","id":12}]' FAKE_LABEL_N=2 stub_writes
|
||||
FAKE_LABELS='[{"name":"stale","id":11},{"name":"ready","id":12}]' FAKE_LABEL_N=2 REPO=o/r forge_issue_edit 5 --remove-label stale
|
||||
check "removing a label resolves its numeric id" 0 "" grep -q '^DELETE repos/o/r/issues/5/labels/11 ' "$WRITES"
|
||||
check "...and never sends the name as the path segment" 1 "" grep -q 'labels/stale' "$WRITES"
|
||||
# Removal is a FULL-SET PUT, not a per-label DELETE (#192). Measured under a
|
||||
# real Actions token, probe run 701: DELETE .../labels/{id} -> 500 for every
|
||||
# removal, PUT .../labels -> 200 including the empty set. A PAT gets 204 on the
|
||||
# same DELETE, which is why it went unseen — it fails only for the identity the
|
||||
# sweep holds.
|
||||
ROSTER='[{"name":"state:old","id":11},{"name":"state:new","id":12},{"name":"scope:labels","id":13},{"name":"attention","id":14}]'
|
||||
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
|
||||
# call --remove-label unconditionally to converge state.
|
||||
FAKE_LABELS='[{"name":"ready","id":12}]' FAKE_LABEL_N=1 stub_writes
|
||||
FAKE_LABELS='[{"name":"ready","id":12}]' FAKE_LABEL_N=1 REPO=o/r forge_issue_edit 5 --remove-label nonexistent
|
||||
check "removing an absent label writes nothing" 0 "" test ! -s "$WRITES"
|
||||
# The contract @codex-reviewer-andresmgsl asked for (#5183): a full-set PUT
|
||||
# replaces everything, so removal alone proves nothing about PRESERVATION. One
|
||||
# call, a combined delta, and two bystanders that must survive it.
|
||||
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},{"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
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
#
|
||||
|
|
|
|||
Loading…
Reference in a new issue