docs/tools/forge.md

@@ -344,16 +344,9 @@ 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)
-  is driven by the forge's own `Link: rel="next"` / `X-HasMore`
+  reports the real total from the response's `X-Total-Count`
-  signal, so it stays right when the forge clamps `--limit`: while a next
+  (`… 50 of 187 shown … 137 more`), so trust the trailer, not the row
-  page exists it prints the rows shown so far (out of the response's
+  count, when deciding whether you've seen everything.
-  `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,9 +26,8 @@
 
 use anyhow::Result;
 use clap::{Args as ClapArgs, ValueEnum};
-use forgejo_api::StructureError;
 use forgejo_api::structs::{
-    Issue, IssueListIssuesQuery, IssueListIssuesQueryState, IssueListIssuesQueryType,
+    IssueListIssuesQuery, IssueListIssuesQueryState, IssueListIssuesQueryType,
 };
 use serde_json::Value;
 
@@ -172,12 +171,10 @@ 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.total,
+        headers.x_total_count.and_then(|t| u64::try_from(t).ok()),
-        headers.has_next,
         count,
         args.page,
         args.limit,
@@ -211,88 +208,47 @@ 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, and the
+/// the requested `limit` is not the page size that was served: with
-/// served size is not reported. The forge's own pagination signal doesn't
+/// `--limit 400` a full clamped page of 50 comes back, and the old
-/// need it: `has_next` (`Link: rel="next"` / `X-HasMore`) is present
+/// `count == limit` test stayed silent — suppressing the warning in the
-/// exactly when more rows follow. No `has_next` ⇒ last page ⇒ silent. With
+/// one case where the truncation is invisible. Hence the header.
-/// 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`; it only enriches the message. When it is
+/// `total` is `X-Total-Count`, which Forgejo sends on this endpoint. The
-/// absent the response headers did not come through (older forge, a proxy
+/// *effective* page size isn't reported, but it doesn't need to be: a
-/// that stripped them), so `has_next` can't be trusted either and the
+/// short page is by definition the last one, so whenever more pages can
-/// `count == limit` heuristic applies. It under-reports when the server
+/// exist at all, `count` **is** the page size and `page * count` is how
-/// clamped, which is why it is the fallback and not the rule. A proxy that
+/// many rows have been shown up to here.
-/// strips only `Link` while keeping `X-Total-Count` is not detectable and
+///
-/// reads as a last page.
+/// Deliberately not clamping `--limit` client-side to 50: that number is
-fn trailer(
+/// the *remote's* configuration, and baking a peer's setting into the
-    total: Option<u64>,
+/// client is how you get a value that's wrong on the one deployment that
-    has_next: bool,
+/// changed it.
-    count: u64,
+fn trailer(total: Option<u64>, count: u64, page: u64, limit: u64) -> Option<String> {
-    page: u64,
-    limit: u64,
-) -> Option<String> {
     // An empty page is the end, whatever else is true.
     if count == 0 {
         return None;
     }
-    let next = page + 1;
-    let shown = page.saturating_mul(count);
     match total {
-        Some(total) => has_next.then(|| {
+        Some(total) => {
-            match total.checked_sub(shown).filter(|r| *r > 0) {
+            let shown = page.saturating_mul(count);
-                Some(remaining) => format!(
+            let remaining = total.saturating_sub(shown);
-                    "… {shown} of {total} shown (page {page}, {count}/page) — {remaining} more; re-run with --page {next}."
+            (remaining > 0).then(|| {
-                ),
+                format!(
-                None => format!(
+                    "… {shown} of {total} shown (page {page}, {count}/page) — {remaining} more; re-run with --page {}.",
-                    "… {shown} shown (page {page}, {count}/page); more results exist — re-run with --page {next}."
+                    page + 1
-                ),
+                )
-            }
+            })
-        }),
+        }
-        None if has_next => Some(format!(
+        // No header (older forge, or a proxy that dropped it): fall back
-            "… {shown} shown (page {page}, {count}/page); more results exist — re-run with --page {next}."
+        // to the full-page heuristic. It under-reports when the server
-        )),
+        // clamped, which is why it is the fallback and not the rule.
         None => (count == limit).then(|| {
             format!(
-                "… {count} shown (page {page}); more results may exist — re-run with --page {next}."
+                "… {count} shown (page {page}); more results may exist — re-run with --page {}.",
+                page + 1
             )
         }),
     }
@@ -520,8 +476,11 @@ mod tests {
 
     #[test]
     fn trailer_fires_on_a_clamped_page_the_old_heuristic_missed() {
-        // `--limit 400` against a forge that clamps to 50, 187 matches.
+        // The regression this function exists for: `--limit 400` against
-        let msg = trailer(Some(187), true, 50, 1, 400).expect("must warn about the other 137");
+        // 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");
         assert!(msg.contains("50 of 187"), "{msg}");
         assert!(msg.contains("137 more"), "{msg}");
         assert!(msg.contains("--page 2"), "{msg}");
@@ -530,87 +489,29 @@ 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), true, 50, 3, 400).expect("187 > 150");
+        let msg = trailer(Some(187), 50, 3, 400).expect("187 > 150");
         assert!(msg.contains("150 of 187"), "{msg}");
         assert!(msg.contains("37 more"), "{msg}");
     }
 
     #[test]
-    fn trailer_clamped_page_whose_arithmetic_looks_like_the_last_page() {
+    fn trailer_silent_once_the_total_is_accounted_for() {
-        // `--limit 100` clamped to 50/page, 150 rows, page 2:
+        // Last page, short: 2 * 50 + ... -> 60 shown of 60.
-        // `(page - 1) * limit + count` equals the total, yet rows 101-150
+        assert_eq!(trailer(Some(60), 10, 6, 10), None);
-        // 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_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);
+        assert_eq!(trailer(Some(150), 50, 3, 50), None);
-        // Empty page is the end even if the forge claims a next page.
+        // Empty page is the end even if the total disagrees (a filter
-        assert_eq!(trailer(Some(999), true, 0, 9, 30), None);
+        // 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);
     }
 
     #[test]
-    fn trailer_next_page_without_a_total_still_hints() {
+    fn trailer_falls_back_to_the_full_page_heuristic_without_the_header() {
-        let msg = trailer(None, true, 50, 2, 400).expect("next page exists");
+        let msg = trailer(None, 30, 1, 30).expect("full page, unknown total");
-        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, false, 12, 1, 30), None);
+        assert_eq!(trailer(None, 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);
     }
 }