From 8c0f5d53d794a4fce5d4302965e196fcad90c734 Mon Sep 17 00:00:00 2001 From: codex-bot-andresmgsl Date: Mon, 24 Aug 2026 17:50:50 +0000 Subject: [PATCH 1/3] test: expose truncated Forgejo timelines --- test/forge-backends.test.sh | 77 ++++++++++++++++++++++++++++++++++++- 1 file changed, 75 insertions(+), 2 deletions(-) diff --git a/test/forge-backends.test.sh b/test/forge-backends.test.sh index ad1bd65..8409acf 100644 --- a/test/forge-backends.test.sh +++ b/test/forge-backends.test.sh @@ -746,6 +746,7 @@ check "no api base refuses" 1 "cannot reach the forge" \ # .user.login -> .actor.login. Mutation-verified: collapsing add/remove or # emitting .user instead of .actor each reds its own case (#4853). timeline_stub() { + : >"$timeline_calls" # shellcheck disable=SC2317 # invoked indirectly, by forge_api curl() { local hdr="" out="" url="" @@ -753,14 +754,44 @@ timeline_stub() { case "$1" in -D) hdr="$2"; shift ;; -o) out="$2"; shift ;; -H) shift ;; *) url="$1" ;; esac shift done - printf 'HTTP/1.1 200 OK\r\nX-Total-Count: %s\r\n\r\n' "${FAKE_TL_N:-2}" >"$hdr" + printf '%s\n' "$url" >>"$timeline_calls" + local page=1 page_body="$FAKE_TIMELINE" page_total="${FAKE_TL_N:-2}" + case "$url" in *page=*) page="${url##*page=}"; page="${page%%&*}" ;; esac + if [ "${#FAKE_TL_PAGES[@]}" -gt 0 ]; then + if [ "$page" -le "${#FAKE_TL_PAGES[@]}" ]; then + page_body="${FAKE_TL_PAGES[$((page - 1))]}" + else + page_body='[]' + fi + page_total="$(jq 'length' <<<"$page_body")" + fi + { + printf 'HTTP/1.1 200 OK\r\n' + [ "${FAKE_TL_HEADERS:-yes}" = no ] || printf 'X-Total-Count: %s\r\n' "$page_total" + printf '\r\n' + } >"$hdr" case "$url" in - *timeline*) printf '%s' "$FAKE_TIMELINE" >"$out" ;; + *timeline*) printf '%s' "$page_body" >"$out" ;; *) printf '[]' >"$out" ;; esac return 0 } } +timeline_page() { + jq -nc --argjson first "$1" --argjson count "$2" ' + [range($first; $first + $count) + | { + type: "label", + body: "1", + user: {login: "setter"}, + label: {name: "needs-ruling"}, + created_at: ("event-" + tostring) + }] + ' +} +timeline_calls="$TMP/timeline_calls" +FAKE_TL_PAGES=() +FAKE_TL_HEADERS=yes FAKE_TIMELINE='[ {"type":"label","body":"1","user":{"login":"setter"},"label":{"name":"needs-ruling"},"created_at":"2026-08-02T14:58:13Z"}, {"type":"label","body":"","user":{"login":"setter"},"label":{"name":"needs-ruling"},"created_at":"2026-08-02T15:22:22Z"}, @@ -777,6 +808,48 @@ check "forge_timeline drops non-label events" 0 "" \ test "$(jq '[.[] | select(.event == null or .event == "")] | length' <<<"$tl")" = 0 check "forge_timeline uses .actor.login, not a bare .user" 0 "" \ jq -e 'all(.[]; has("actor") and (.user|not))' <<<"$tl" >/dev/null + +# Forgejo's timeline endpoint lies consistently: x-total-count echoes the +# current page size. With 151 events its pages declare 50, 50, 50 and 1, so +# strict pagination stops successfully after page 1 and drops the newest 101 +# events. Exhaustion is safe only here because timelines are append-only. +FAKE_TL_PAGES=( + "$(timeline_page 1 50)" + "$(timeline_page 51 50)" + "$(timeline_page 101 50)" + "$(timeline_page 151 1)" +) +timeline_stub +tl="$(REPO=o/r forge_timeline 188)" +check "forge_timeline exhausts all pages despite per-page total headers" 0 "" \ + test "$(jq 'length' <<<"$tl")" = 151 +check "forge_timeline retains the newest event beyond page one" 0 "" \ + jq -e 'any(.[]; .created_at == "event-151")' <<<"$tl" >/dev/null + +# A collection exactly divisible by the page size needs one final empty read; +# stopping after the second full page cannot prove exhaustion. +FAKE_TL_PAGES=("$(timeline_page 1 50)" "$(timeline_page 51 50)") +timeline_stub +exhaustive_count="$(REPO=o/r forge_api --paginate-exhaustive 'repos/o/r/issues/188/timeline' --jq 'length')" +check "exhaustive pagination terminates after an empty page" 0 "" \ + test "$exhaustive_count" = 100 +check "an exactly-full exhaustive gather reads the empty third page" 0 "" \ + test "$(wc -l <"$timeline_calls")" = 3 + +# The exhaustive path's completeness proof is the short page itself; it must +# never consult the endpoint's missing or dishonest total header. +FAKE_TL_PAGES=("$(timeline_page 1 50)" "$(timeline_page 51 1)") +FAKE_TL_HEADERS=no +timeline_stub +check "exhaustive pagination needs no x-total-count header" 0 "" \ + eq 51 forge_api --paginate-exhaustive 'repos/o/r/issues/188/timeline' --jq 'length' +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' + # 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. # shellcheck disable=SC2317 -- 2.45.2 From 40ebcea462cf95a648cf351f1c113a6e1d1c6930 Mon Sep 17 00:00:00 2001 From: codex-bot-andresmgsl Date: Mon, 24 Aug 2026 17:53:13 +0000 Subject: [PATCH 2/3] fix: exhaust Forgejo timeline pagination --- changelog.d/240.md | 3 ++ lib/forge-forgejo.sh | 64 +++++++++++++++++++++++++++---------- test/forge-backends.test.sh | 4 ++- 3 files changed, 53 insertions(+), 18 deletions(-) create mode 100644 changelog.d/240.md diff --git a/changelog.d/240.md b/changelog.d/240.md new file mode 100644 index 0000000..3b2f6f3 --- /dev/null +++ b/changelog.d/240.md @@ -0,0 +1,3 @@ +### Fixed + +- Read Forgejo timelines to exhaustion so busy issues retain their newest label events despite dishonest total-count headers (#240). diff --git a/lib/forge-forgejo.sh b/lib/forge-forgejo.sh index 1b05275..247c760 100644 --- a/lib/forge-forgejo.sh +++ b/lib/forge-forgejo.sh @@ -84,7 +84,7 @@ forgejo_page_url() { esac } -# forge_api [--paginate] [--jq ] +# forge_api [--paginate | --paginate-exhaustive] [--jq ] # # --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,21 +160,23 @@ forge_api() { fi 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 - # 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="$page_total" - elif [ "$page_total" != "$total" ]; then - cat >&2 <&2 <&2 < Date: Mon, 24 Aug 2026 18:01:04 +0000 Subject: [PATCH 3/3] test: close exhaustive pagination review gaps --- lib/forge-forgejo.sh | 5 +++++ test/forge-backends.test.sh | 11 +++++++++++ 2 files changed, 16 insertions(+) diff --git a/lib/forge-forgejo.sh b/lib/forge-forgejo.sh index 247c760..20deffe 100644 --- a/lib/forge-forgejo.sh +++ b/lib/forge-forgejo.sh @@ -109,6 +109,11 @@ forge_api() { method="$2" shift ;; + -X?*) method="${1#-X}" ;; + --method=*) + method="${1#*=}" + [ -n "$method" ] || { echo "forge_api: --method requires a value" >&2; return 1; } + ;; --jq) jqexpr="$2"; have_jq=true; shift ;; -*) ;; *) [ -n "$endpoint" ] || endpoint="$1" ;; diff --git a/test/forge-backends.test.sh b/test/forge-backends.test.sh index 08adc61..783530f 100644 --- a/test/forge-backends.test.sh +++ b/test/forge-backends.test.sh @@ -825,6 +825,8 @@ check "forge_timeline exhausts all pages despite per-page total headers" 0 "" \ test "$(jq 'length' <<<"$tl")" = 151 check "forge_timeline retains the newest event beyond page one" 0 "" \ jq -e 'any(.[]; .created_at == "event-151")' <<<"$tl" >/dev/null +check "a short final page terminates without an extra empty-page read" 0 "" \ + test "$(wc -l <"$timeline_calls")" = 4 # A collection exactly divisible by the page size needs one final empty read; # stopping after the second full page cannot prove exhaustion. @@ -843,14 +845,23 @@ FAKE_TL_HEADERS=no timeline_stub check "exhaustive pagination needs no x-total-count header" 0 "" \ eq 51 forge_api --paginate-exhaustive 'repos/o/r/issues/188/timeline' --jq 'length' +check "the same missing-header fixture is still refused by strict pagination" 1 \ + "did not send x-total-count" \ + forge_api --paginate 'repos/o/r/issues/188/timeline' --jq 'length' 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 -X POST 'repos/o/r/issues/188/timeline' +check "exhaustive pagination refuses compact -XPOST too" 1 "GET" \ + forge_api --paginate-exhaustive -XPOST 'repos/o/r/issues/188/timeline' +check "exhaustive pagination refuses --method=POST too" 1 "GET" \ + forge_api --paginate-exhaustive --method=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 +check "only forge_timeline invokes exhaustive pagination" 0 "" \ + test "$(grep -c 'forge_api --paginate-exhaustive' "$ROOT/lib/forge-forgejo.sh")" = 1 # 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. -- 2.45.2