diff --git a/lib/forge-forgejo.sh b/lib/forge-forgejo.sh index 1756943..0000e33 100644 --- a/lib/forge-forgejo.sh +++ b/lib/forge-forgejo.sh @@ -118,11 +118,36 @@ forge_api() { fi forgejo_http_ok "$hdr" "$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 + # 9 stopped at 4 believing itself whole. A moving total means the read + # cannot have been atomic, so it is refused rather than reconciled. + local page_total + page_total="$(forgejo_total_count "$hdr")" || return 1 if [ -z "$total" ]; then - total="$(forgejo_total_count "$hdr")" || return 1 + total="$page_total" + elif [ "$page_total" != "$total" ]; then + cat >&2 </dev/null)" != array ]; then + cat >&2 <&2 < — install a curl stub serving as -# successive page bodies, declaring in x-total-count. An empty -# string for omits the header entirely (@kimi's #4699 case). +# fake_forge — install a curl stub serving as +# successive page bodies, declaring in x-total-count. An empty +# string omits the header entirely (@kimi's #4699 case). A comma-separated +# spec declares a DIFFERENT total per page ("4,9"), which is +# @codex-reviewer-andresmgsl's changing-between-pages case (#4700 / #4712): +# a server whose count moves under the walk cannot have been read whole. fake_forge() { FAKE_TOTAL="$1"; shift FAKE_PAGES=("$@") @@ -94,9 +97,16 @@ fake_forge() { done local page=1 case "$url" in *page=*) page="${url##*page=}"; page="${page%%&*}" ;; esac + local total="$FAKE_TOTAL" + case "$FAKE_TOTAL" in + *,*) + total="$(printf '%s' "$FAKE_TOTAL" | cut -d, -f"$page")" + [ -n "$total" ] || total="$(printf '%s' "$FAKE_TOTAL" | cut -d, -f1)" + ;; + esac { printf 'HTTP/1.1 200 OK\r\n' - [ -n "$FAKE_TOTAL" ] && printf 'X-Total-Count: %s\r\n' "$FAKE_TOTAL" + [ -n "$total" ] && printf 'X-Total-Count: %s\r\n' "$total" printf '\r\n' } >"$hdr" if [ "$page" -le "${#FAKE_PAGES[@]}" ]; then @@ -141,6 +151,51 @@ check "a missing x-total-count refuses" 1 "did not send x-total-count" \ check "...and says why it cannot prove completeness" 1 "cannot prove the gather is complete" \ forge_api --paginate 'repos/o/r/issues' --jq '.[].number' +# @codex-reviewer-andresmgsl's #4712 findings. Each one is a route by which +# an unprovable read could still have been reported as a whole one — the +# guard leaking the failure class it was built to stop, which is why they +# are refusals rather than warnings. + +# A total that is not a number went straight into arithmetic. Reproduced on +# ab23a3b: `X-Total-Count: not-a-number` returned rc=0 with that string as +# the total. +fake_forge 'not-a-number' '[{"number":1}]' +check "a non-numeric total is refused" 1 "not a non-negative integer" \ + forge_api --paginate 'repos/o/r/issues' --jq '.[].number' +check "...and the refusal quotes what arrived" 1 "not-a-number" \ + forge_api --paginate 'repos/o/r/issues' --jq '.[].number' +fake_forge '12x' '[{"number":1}]' +check "a partly-numeric total is refused" 1 "not a non-negative integer" \ + forge_api --paginate 'repos/o/r/issues' --jq '.[].number' +fake_forge '-3' '[{"number":1}]' +check "a negative total is refused" 1 "not a non-negative integer" \ + forge_api --paginate 'repos/o/r/issues' --jq '.[].number' + +# A total that MOVES under the walk. The loop read it once, so a board +# changing size mid-gather was invisible: page 1 said 4, page 2 said 9, and +# the walk stopped at 4 believing itself complete. +fake_forge '4,9' '[{"number":1},{"number":2}]' '[{"number":3},{"number":4}]' +check "a total that changes between pages is refused" 1 "changed between pages" \ + forge_api --paginate 'repos/o/r/issues' --jq '.[].number' +check "...and the refusal names both totals" 1 "4" \ + forge_api --paginate 'repos/o/r/issues' --jq '.[].number' + +# A 200 whose body is not a collection. `length` on a non-array counted 0, +# so an object or a scalar arriving where a list belongs read as a complete +# EMPTY collection when the declared total was 0 — silence dressed as a +# clean sweep. +fake_forge 0 '{"message":"Not found"}' +check "a non-array body is refused" 1 "did not return a collection" \ + forge_api --paginate 'repos/o/r/issues' --jq '.[].number' +fake_forge 0 '"a string"' +check "a scalar body is refused" 1 "did not return a collection" \ + forge_api --paginate 'repos/o/r/issues' --jq '.[].number' +# A genuinely empty collection is still fine — the refusal must not fire on +# a repo that legitimately has nothing. +fake_forge 0 '[]' +check "an empty collection is not an error" 0 "" \ + forge_api --paginate 'repos/o/r/issues' --jq '.[].number' + # --- HTTP failures are named, not swallowed ----------------------------- # gh exits non-zero on an HTTP error; curl does not without -f, and -f # discards the body that explains why. So the status is read explicitly.