fix: exhaust Forgejo timeline pagination
All checks were successful
CI / test (pull_request) Successful in 3m46s
CI / release-exercise (pull_request) Successful in 25s
CI / self-guards (pull_request) Successful in 21s
CI / action-exercise (pull_request) Successful in 20s
CI / docs-sync-exercise (pull_request) Successful in 20s
labels / labels (pull_request) Successful in 22s
Refs guard / refs-not-closing (pull_request) Successful in 20s
All checks were successful
CI / test (pull_request) Successful in 3m46s
CI / release-exercise (pull_request) Successful in 25s
CI / self-guards (pull_request) Successful in 21s
CI / action-exercise (pull_request) Successful in 20s
CI / docs-sync-exercise (pull_request) Successful in 20s
labels / labels (pull_request) Successful in 22s
Refs guard / refs-not-closing (pull_request) Successful in 20s
This commit is contained in:
parent
8c0f5d53d7
commit
40ebcea462
3 changed files with 53 additions and 18 deletions
3
changelog.d/240.md
Normal file
3
changelog.d/240.md
Normal file
|
|
@ -0,0 +1,3 @@
|
|||
### Fixed
|
||||
|
||||
- Read Forgejo timelines to exhaustion so busy issues retain their newest label events despite dishonest total-count headers (#240).
|
||||
|
|
@ -84,7 +84,7 @@ forgejo_page_url() {
|
|||
esac
|
||||
}
|
||||
|
||||
# forge_api [--paginate] <endpoint> [--jq <expr>]
|
||||
# forge_api [--paginate | --paginate-exhaustive] <endpoint> [--jq <expr>]
|
||||
#
|
||||
# --paginate walks page= until a short page, then PROVES the walk was
|
||||
# complete by comparing what it collected against the server's declared
|
||||
|
|
@ -94,11 +94,21 @@ forgejo_page_url() {
|
|||
# make the completeness check compare null to a number — the guard itself
|
||||
# degrading silently, which is the failure class re-entering through the
|
||||
# door built to stop it.
|
||||
#
|
||||
# --paginate-exhaustive is the narrow alternative for an endpoint whose
|
||||
# x-total-count is known not to describe the collection. It proves completion
|
||||
# by reading through the first short page and never consults that header.
|
||||
forge_api() {
|
||||
local paginate=false endpoint="" jqexpr="" have_jq=false
|
||||
local paginate=false paginate_exhaustive=false method=GET endpoint="" jqexpr="" have_jq=false
|
||||
while [ $# -gt 0 ]; do
|
||||
case "$1" in
|
||||
--paginate) paginate=true ;;
|
||||
--paginate-exhaustive) paginate_exhaustive=true ;;
|
||||
-X | --method)
|
||||
[ "$#" -ge 2 ] || { echo "forge_api: $1 requires a value" >&2; return 1; }
|
||||
method="$2"
|
||||
shift
|
||||
;;
|
||||
--jq) jqexpr="$2"; have_jq=true; shift ;;
|
||||
-*) ;;
|
||||
*) [ -n "$endpoint" ] || endpoint="$1" ;;
|
||||
|
|
@ -106,6 +116,14 @@ forge_api() {
|
|||
shift
|
||||
done
|
||||
[ -n "$endpoint" ] || { echo "forge_api: endpoint required" >&2; return 1; }
|
||||
if [ "$paginate" = true ] && [ "$paginate_exhaustive" = true ]; then
|
||||
echo "forge_api: --paginate and --paginate-exhaustive are mutually exclusive" >&2
|
||||
return 1
|
||||
fi
|
||||
if { [ "$paginate" = true ] || [ "$paginate_exhaustive" = true ]; } && [ "$method" != GET ]; then
|
||||
echo "forge_api: pagination is available only for GET requests" >&2
|
||||
return 1
|
||||
fi
|
||||
|
||||
local base token
|
||||
base="$(forgejo_api_base)" || return 1
|
||||
|
|
@ -116,7 +134,7 @@ forge_api() {
|
|||
# shellcheck disable=SC2064 # the paths are fixed at trap time on purpose
|
||||
trap "rm -f '$hdr' '$body'" RETURN
|
||||
|
||||
if [ "$paginate" = false ]; then
|
||||
if [ "$paginate" = false ] && [ "$paginate_exhaustive" = false ]; then
|
||||
if ! curl -sS -D "$hdr" -o "$body" \
|
||||
-H "Authorization: token $token" -H 'Accept: application/json' \
|
||||
"$base/$endpoint"; then
|
||||
|
|
@ -142,6 +160,7 @@ forge_api() {
|
|||
fi
|
||||
forgejo_http_ok "$hdr" "GET $endpoint" || return 1
|
||||
|
||||
if [ "$paginate_exhaustive" = false ]; then
|
||||
# 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
|
||||
# 9 stopped at 4 believing itself whole. A moving total means the read
|
||||
|
|
@ -158,6 +177,7 @@ forge_api: the declared total for '$endpoint' changed between pages — $total t
|
|||
EOF
|
||||
return 1
|
||||
fi
|
||||
fi
|
||||
|
||||
pagejson="$(cat "$body")"
|
||||
# A 200 whose body is not a collection counted as zero items (#4712),
|
||||
|
|
@ -175,13 +195,17 @@ EOF
|
|||
[ "$n" -gt 0 ] || break
|
||||
all="$(jq -s '.[0] + .[1]' <<<"$all"$'\n'"$pagejson")"
|
||||
got=$((got + n))
|
||||
if [ "$paginate_exhaustive" = true ]; then
|
||||
[ "$n" -eq 50 ] || break
|
||||
else
|
||||
[ "$got" -lt "$total" ] || break
|
||||
fi
|
||||
page=$((page + 1))
|
||||
done
|
||||
|
||||
# The assert. A short read here is the silent-truncation bug arriving by
|
||||
# another route, so it is fatal rather than a warning.
|
||||
if [ "$got" -ne "$total" ]; then
|
||||
if [ "$paginate_exhaustive" = false ] && [ "$got" -ne "$total" ]; then
|
||||
cat >&2 <<EOF
|
||||
forge_api: incomplete gather for '$endpoint' — collected $got of $total declared (#188).
|
||||
Refusing rather than reconciling a partial board: a sweep over part of the
|
||||
|
|
@ -561,7 +585,13 @@ forge_request_reviewer() {
|
|||
# empty timeline — the two states the ruling ladder must tell apart (#4853).
|
||||
forge_timeline() {
|
||||
local n="${1:?forge_timeline: number required}" raw
|
||||
raw="$(forge_api --paginate "repos/$REPO/issues/$n/timeline")" || return 1
|
||||
# Measured on this instance: limit=10 reports x-total-count=10 and limit=50
|
||||
# reports 50, while crew!96 held 151 events and strict pagination returned
|
||||
# only its first 50. No other measured endpoint echoes its page size this
|
||||
# way. Timelines are append-only, so exhaustion can include concurrent new
|
||||
# events but cannot create a deletion hole; that is why only this call site
|
||||
# may bypass the header-bound completeness proof (#240).
|
||||
raw="$(forge_api --paginate-exhaustive "repos/$REPO/issues/$n/timeline")" || return 1
|
||||
jq '
|
||||
[.[]
|
||||
| select(.type == "label")
|
||||
|
|
|
|||
|
|
@ -848,7 +848,9 @@ FAKE_TL_HEADERS=yes
|
|||
check "strict and exhaustive pagination are mutually exclusive" 1 "mutually exclusive" \
|
||||
forge_api --paginate --paginate-exhaustive 'repos/o/r/issues/188/timeline'
|
||||
check "exhaustive pagination refuses a non-GET method" 1 "GET" \
|
||||
forge_api --paginate-exhaustive --method POST 'repos/o/r/issues/188/timeline'
|
||||
forge_api --paginate-exhaustive -X POST 'repos/o/r/issues/188/timeline'
|
||||
check "the exhaustive flag has exactly one production call site" 0 "" \
|
||||
test "$(grep -c 'paginate-exhaustive' "$ROOT/lib/forge-forgejo.sh")" = 5
|
||||
|
||||
# Unreadable: curl fails. Status must surface through forge_timeline itself
|
||||
# (not a later jq), or the ruling ladder invents a verdict on a half-read.
|
||||
|
|
|
|||
Loading…
Reference in a new issue