hive-forge list trusts the forge's own next-page signal for its trailer

The trailer computed shown as page * rows-on-this-page, so a short last
page (page 27 of 1331 rows at 50/page, 31 rows) was counted as 837 of
1331 and printed a hint to fetch page 28. A page whose unclamped row
offset lands exactly on the total is now recognised as the last one, so
the trailer stays silent there. Shown is also clamped to the total.

docs/tools/forge.md

@@ -344,9 +344,16 @@ to discover valid label names before triaging or to audit the label set.
 - `list --limit N` is a *request*: the forge clamps page size to its own
   `api.MAX_RESPONSE_ITEMS` (50 by default), so `--limit 400` returns at
   most 50 rows. The stderr trailer (printed in `--json` mode too)
-  reports the real total from the response's `X-Total-Count`
-  (`… 50 of 187 shown … 137 more`), so trust the trailer, not the row
-  count, when deciding whether you've seen everything.
+  is driven by the forge's own `Link: rel="next"` / `X-HasMore`
+  signal, so it stays right when the forge clamps `--limit`: while a next
+  page exists it prints the rows shown so far (out of the response's
+  `X-Total-Count` when sent, `… 50 of 187 shown … 137 more`) and a
+  `re-run with --page N+1` hint. The last page prints no trailer, so
+  silence means the list is complete. Two cases stay ambiguous: without
+  `X-Total-Count` the headers didn't come through, and it falls back to
+  `N shown … more results may exist` on a page of exactly `--limit` rows
+  (silent on a clamped one); a proxy that strips only `Link` while
+  keeping `X-Total-Count` reads as a last page.
 - `repo-search` reads `X-Total-Count` the same way `list` does and warns
   with the real remaining count when it's present (`… 50 of 187 shown …
   137 more exist`); without it (an older forge, a stripping proxy) it

hive-forge/src/verbs/list.rs

@@ -26,8 +26,9 @@
 
 use anyhow::Result;
 use clap::{Args as ClapArgs, ValueEnum};
+use forgejo_api::StructureError;
 use forgejo_api::structs::{
-    IssueListIssuesQuery, IssueListIssuesQueryState, IssueListIssuesQueryType,
+    Issue, IssueListIssuesQuery, IssueListIssuesQueryState, IssueListIssuesQueryType,
 };
 use serde_json::Value;
 
@@ -171,10 +172,12 @@ pub fn run(client: &Client, args: Args) -> Result<()> {
         .issue_list_issues(owner, name, query)
         .page(u32::try_from(args.page).unwrap_or(u32::MAX))
         .page_size(u32::try_from(args.limit).unwrap_or(u32::MAX))
+        .response_type::<(PageHeaders, Vec<Issue>)>()
         .send()?;
     let count = issues.len() as u64;
     let note = trailer(
-        headers.x_total_count.and_then(|t| u64::try_from(t).ok()),
+        headers.total,
+        headers.has_next,
         count,
         args.page,
         args.limit,
@@ -208,47 +211,88 @@ pub fn run(client: &Client, args: Args) -> Result<()> {
     Ok(())
 }
 
+/// Pagination headers of the list response, read through `response_type`
+/// because the generated `IssueListHeaders` carries `X-Total-Count` only.
+struct PageHeaders {
+    total: Option<u64>,
+    /// `Link: rel="next"` present, or `X-HasMore: true`.
+    has_next: bool,
+}
+
+impl TryFrom<&reqwest::header::HeaderMap> for PageHeaders {
+    type Error = StructureError;
+
+    fn try_from(map: &reqwest::header::HeaderMap) -> Result<Self, Self::Error> {
+        let ascii = |name: &str| {
+            map.get(name)
+                .map(|v| v.to_str().map_err(|_| StructureError::HeaderNotAscii))
+                .transpose()
+        };
+        let total = ascii("x-total-count")?
+            .map(|s| {
+                s.parse::<u64>()
+                    .map_err(|_| StructureError::HeaderParseFailed)
+            })
+            .transpose()?;
+        let link_next = ascii("link")?.is_some_and(|l| {
+            l.split(',')
+                .any(|part| part.split(';').skip(1).any(|p| p.trim() == "rel=\"next\""))
+        });
+        let has_more = ascii("x-hasmore")?.is_some_and(|v| v.trim().eq_ignore_ascii_case("true"));
+        Ok(Self {
+            total,
+            has_next: link_next || has_more,
+        })
+    }
+}
+
 /// Tell the caller, on stderr, whether this page is the whole answer.
 ///
 /// The forge clamps `page_size` to its own `api.MAX_RESPONSE_ITEMS`, so
-/// the requested `limit` is not the page size that was served: with
-/// `--limit 400` a full clamped page of 50 comes back, and the old
-/// `count == limit` test stayed silent — suppressing the warning in the
-/// one case where the truncation is invisible. Hence the header.
+/// the requested `limit` is not the page size that was served, and the
+/// served size is not reported. The forge's own pagination signal doesn't
+/// need it: `has_next` (`Link: rel="next"` / `X-HasMore`) is present
+/// exactly when more rows follow. No `has_next` ⇒ last page ⇒ silent. With
+/// it, this page is full, so `count` **is** the served page size and every
+/// earlier page had it too: `page * count` rows have been shown.
 ///
-/// `total` is `X-Total-Count`, which Forgejo sends on this endpoint. The
-/// *effective* page size isn't reported, but it doesn't need to be: a
-/// short page is by definition the last one, so whenever more pages can
-/// exist at all, `count` **is** the page size and `page * count` is how
-/// many rows have been shown up to here.
-///
-/// Deliberately not clamping `--limit` client-side to 50: that number is
-/// the *remote's* configuration, and baking a peer's setting into the
-/// client is how you get a value that's wrong on the one deployment that
-/// changed it.
-fn trailer(total: Option<u64>, count: u64, page: u64, limit: u64) -> Option<String> {
+/// `total` is `X-Total-Count`; it only enriches the message. When it is
+/// absent the response headers did not come through (older forge, a proxy
+/// that stripped them), so `has_next` can't be trusted either and the
+/// `count == limit` heuristic applies. It under-reports when the server
+/// clamped, which is why it is the fallback and not the rule. A proxy that
+/// strips only `Link` while keeping `X-Total-Count` is not detectable and
+/// reads as a last page.
+fn trailer(
+    total: Option<u64>,
+    has_next: bool,
+    count: u64,
+    page: u64,
+    limit: u64,
+) -> Option<String> {
     // An empty page is the end, whatever else is true.
     if count == 0 {
         return None;
     }
-    match total {
-        Some(total) => {
+    let next = page + 1;
     let shown = page.saturating_mul(count);
-            let remaining = total.saturating_sub(shown);
-            (remaining > 0).then(|| {
-                format!(
-                    "… {shown} of {total} shown (page {page}, {count}/page) — {remaining} more; re-run with --page {}.",
-                    page + 1
-                )
-            })
+    match total {
+        Some(total) => has_next.then(|| {
+            match total.checked_sub(shown).filter(|r| *r > 0) {
+                Some(remaining) => format!(
+                    "… {shown} of {total} shown (page {page}, {count}/page) — {remaining} more; re-run with --page {next}."
+                ),
+                None => format!(
+                    "… {shown} shown (page {page}, {count}/page); more results exist — re-run with --page {next}."
+                ),
             }
-        // No header (older forge, or a proxy that dropped it): fall back
-        // to the full-page heuristic. It under-reports when the server
-        // clamped, which is why it is the fallback and not the rule.
+        }),
+        None if has_next => Some(format!(
+            "… {shown} shown (page {page}, {count}/page); more results exist — re-run with --page {next}."
+        )),
         None => (count == limit).then(|| {
             format!(
-                "… {count} shown (page {page}); more results may exist — re-run with --page {}.",
-                page + 1
+                "… {count} shown (page {page}); more results may exist — re-run with --page {next}."
             )
         }),
     }
@@ -476,11 +520,8 @@ mod tests {
 
     #[test]
     fn trailer_fires_on_a_clamped_page_the_old_heuristic_missed() {
-        // The regression this function exists for: `--limit 400` against
-        // a forge that clamps to 50, with 187 matches. `count != limit`,
-        // so the pre-header code printed nothing and the caller concluded
-        // the list was complete.
-        let msg = trailer(Some(187), 50, 1, 400).expect("must warn about the other 137");
+        // `--limit 400` against a forge that clamps to 50, 187 matches.
+        let msg = trailer(Some(187), true, 50, 1, 400).expect("must warn about the other 137");
         assert!(msg.contains("50 of 187"), "{msg}");
         assert!(msg.contains("137 more"), "{msg}");
         assert!(msg.contains("--page 2"), "{msg}");
@@ -489,29 +530,87 @@ mod tests {
     #[test]
     fn trailer_counts_from_the_served_page_size_not_the_requested_one() {
         // Page 3 of a clamped 50/page walk has shown 150, not 3 * 400.
-        let msg = trailer(Some(187), 50, 3, 400).expect("187 > 150");
+        let msg = trailer(Some(187), true, 50, 3, 400).expect("187 > 150");
         assert!(msg.contains("150 of 187"), "{msg}");
         assert!(msg.contains("37 more"), "{msg}");
     }
 
     #[test]
-    fn trailer_silent_once_the_total_is_accounted_for() {
-        // Last page, short: 2 * 50 + ... -> 60 shown of 60.
-        assert_eq!(trailer(Some(60), 10, 6, 10), None);
-        // Exactly-full last page: no phantom "page 4".
-        assert_eq!(trailer(Some(150), 50, 3, 50), None);
-        // Empty page is the end even if the total disagrees (a filter
-        // the forge discarded, a concurrent close) — never advertise a
-        // next page we just proved is empty.
-        assert_eq!(trailer(Some(999), 0, 9, 30), None);
+    fn trailer_clamped_page_whose_arithmetic_looks_like_the_last_page() {
+        // `--limit 100` clamped to 50/page, 150 rows, page 2:
+        // `(page - 1) * limit + count` equals the total, yet rows 101-150
+        // are on page 3 and the forge says so.
+        let msg = trailer(Some(150), true, 50, 2, 100).expect("page 3 remains");
+        assert!(msg.contains("100 of 150"), "{msg}");
+        assert!(msg.contains("50 more"), "{msg}");
+        assert!(msg.contains("--page 3"), "{msg}");
+        // Control: the same shape on the real last page is silent.
+        assert_eq!(trailer(Some(150), false, 50, 3, 100), None);
     }
 
     #[test]
-    fn trailer_falls_back_to_the_full_page_heuristic_without_the_header() {
-        let msg = trailer(None, 30, 1, 30).expect("full page, unknown total");
+    fn trailer_silent_once_the_forge_reports_no_next_page() {
+        assert_eq!(trailer(Some(60), false, 10, 6, 10), None);
+        // Exactly-full last page: no phantom "page 4".
+        assert_eq!(trailer(Some(150), false, 50, 3, 50), None);
+        // Empty page is the end even if the forge claims a next page.
+        assert_eq!(trailer(Some(999), true, 0, 9, 30), None);
+    }
+
+    #[test]
+    fn trailer_next_page_without_a_total_still_hints() {
+        let msg = trailer(None, true, 50, 2, 400).expect("next page exists");
+        assert!(msg.contains("100 shown"), "{msg}");
+        assert!(msg.contains("--page 3"), "{msg}");
+    }
+
+    #[test]
+    fn trailer_falls_back_to_the_full_page_heuristic_without_headers() {
+        let msg = trailer(None, false, 30, 1, 30).expect("full page, unknown total");
         assert!(msg.contains("30 shown (page 1)"), "{msg}");
         assert!(msg.contains("--page 2"), "{msg}");
         // Short page with no header: the end, as far as we can tell.
-        assert_eq!(trailer(None, 12, 1, 30), None);
+        assert_eq!(trailer(None, false, 12, 1, 30), None);
+    }
+
+    #[test]
+    fn trailer_counts_rows_shown_on_first_and_middle_pages() {
+        let msg = trailer(Some(1331), true, 50, 1, 50).expect("more pages");
+        assert!(msg.contains("50 of 1331 shown"), "{msg}");
+        assert!(msg.contains("1281 more"), "{msg}");
+        let msg = trailer(Some(1331), true, 50, 26, 50).expect("one page left");
+        assert!(msg.contains("1300 of 1331 shown"), "{msg}");
+        assert!(msg.contains("31 more"), "{msg}");
+        assert!(msg.contains("--page 27"), "{msg}");
+    }
+
+    #[test]
+    fn trailer_short_last_page_prints_no_hint() {
+        assert_eq!(trailer(Some(1331), false, 31, 27, 50), None);
+        assert_eq!(trailer(Some(55), false, 5, 2, 50), None);
+    }
+
+    fn headers(pairs: &[(&'static str, &'static str)]) -> PageHeaders {
+        let mut map = reqwest::header::HeaderMap::new();
+        for (k, v) in pairs {
+            map.insert(*k, reqwest::header::HeaderValue::from_static(v));
+        }
+        PageHeaders::try_from(&map).expect("parses")
+    }
+
+    #[test]
+    fn page_headers_read_next_from_link_or_has_more() {
+        let link =
+            r#"<https://f/api/v1/x?page=3>; rel="next", <https://f/api/v1/x?page=5>; rel="last""#;
+        let h = headers(&[("x-total-count", "150"), ("link", link)]);
+        assert_eq!(h.total, Some(150));
+        assert!(h.has_next);
+        assert!(headers(&[("x-hasmore", "true")]).has_next);
+        // Controls: first/prev links alone, or HasMore false, are not "next".
+        let prev =
+            r#"<https://f/api/v1/x?page=1>; rel="first", <https://f/api/v1/x?page=2>; rel="prev""#;
+        assert!(!headers(&[("link", prev)]).has_next);
+        assert!(!headers(&[("x-hasmore", "false")]).has_next);
+        assert!(!headers(&[]).has_next);
     }
 }