repo-search: read X-Total-Count so the warning fires at the server's own clamp

docs/tools/forge.md

@@ -341,11 +341,12 @@ to discover valid label names before triaging or to audit the label set.
   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.
-- `repo-search` has no `X-Total-Count` to fall back on, so it can only
+- `repo-search` reads `X-Total-Count` the same way `list` does and warns
-  warn, not quantify: a result count that exactly equals `--limit`
+  with the real remaining count when it's present (`… 50 of 187 shown …
-  prints a stderr warning that more may exist, same trust-the-trailer
+  137 more exist`); without it (an older forge, a stripping proxy) it
-  principle as `list` above but without a real number behind it. It
+  falls back to the cruder `count == limit` heuristic, same
-  also can't tell "doesn't exist" from "you lack permission to see
+  trust-the-trailer-not-the-row-count principle as `list` above. Either
+  way it can't tell "doesn't exist" from "you lack permission to see
   it" — both render as an absent row with no signal either way — and
   it matches repo **name** only, not owner, so there's no way to
   enumerate an org's repos through this verb.

hive-forge/src/verbs/repo_search.rs

@@ -8,21 +8,57 @@
 //! Human output: one `owner/repo  description` line per hit. `--json`
 //! emits the full `SearchResults.data` array as raw JSON.
 //!
-//! A result count that exactly equals `--limit` is a hit on the cap, not
+//! Like `list`, the forge clamps the page size to its own
-//! necessarily the whole answer — the forge may be holding back more (it
+//! `api.MAX_RESPONSE_ITEMS` (50 by default), so a large `--limit` silently
-//! also silently drops repos the caller lacks permission to see, with no
+//! returns a smaller page. Unlike `list`'s generated binding, this crate's
-//! distinguishing signal at all). Either way we warn on stderr rather than
+//! `repo_search` method does not expose the response headers — but
-//! let a capped list read as complete; stdout — and `--json` stdout
+//! Forgejo's own docs name `/repos/search` as the worked example for
+//! `X-Total-Count`, so [`response_type`][forgejo_api::sync::Request::response_type]
+//! reinterprets the response as `(TotalCount, SearchResults)` to read it
+//! anyway, same number `list`'s trailer reports. When the header is
+//! missing or unparseable (an older forge, a stripping proxy) we fall back
+//! to the cruder `count == limit` heuristic, which under-reports exactly
+//! when the server clamped — the same fallback-not-rule relationship
+//! `list.rs`'s `trailer()` documents. Either way we warn on stderr rather
+//! than let a capped list read as complete; stdout — and `--json` stdout
 //! especially — stays exactly the raw result set, so a script piping it
 //! doesn't have to parse prose out of its own data.
+//!
+//! The total (when present) still can't tell "doesn't exist" from "you
+//! lack permission to see it" — a hidden repo is just absent from both the
+//! count and the rows, with no signal either way.
 
 use anyhow::Result;
 use clap::Args as ClapArgs;
-use forgejo_api::structs::RepoSearchQuery;
+use forgejo_api::StructureError;
+use forgejo_api::structs::{RepoSearchQuery, SearchResults};
 
 use crate::client::Client;
 use crate::verbs::print_json;
 
+/// `X-Total-Count`, read generically via `response_type` since this
+/// crate's `repo_search` binding (unlike `issue_list_issues`'s) has no
+/// headers-carrying `Response` type of its own. Mirrors the header's own
+/// error handling (`forgejo_api`'s `IssueListHeaders`) exactly, so a
+/// non-ASCII or unparseable header fails the request the same way there.
+struct TotalCount(Option<u64>);
+
+impl TryFrom<&reqwest::header::HeaderMap> for TotalCount {
+    type Error = StructureError;
+
+    fn try_from(map: &reqwest::header::HeaderMap) -> Result<Self, Self::Error> {
+        let total = map
+            .get("x-total-count")
+            .map(|v| -> Result<_, StructureError> {
+                let s = v.to_str().map_err(|_| StructureError::HeaderNotAscii)?;
+                s.parse::<u64>()
+                    .map_err(|_| StructureError::HeaderParseFailed)
+            })
+            .transpose()?;
+        Ok(Self(total))
+    }
+}
+
 #[derive(ClapArgs)]
 pub struct Args {
     /// Keyword to search for (matches repo name by default; combine with
@@ -48,7 +84,7 @@ pub struct Args {
 /// serialisation errors when emitting `--json` output.
 pub fn run(client: &Client, args: Args) -> Result<()> {
     let limit = u32::try_from(args.limit.min(u64::from(u32::MAX))).unwrap_or(u32::MAX);
-    let results = client
+    let (TotalCount(total), results) = client
         .api()
         .repo_search(RepoSearchQuery {
             q: args.query,
@@ -70,10 +106,11 @@ pub fn run(client: &Client, args: Args) -> Result<()> {
             order: None,
         })
         .page_size(limit)
+        .response_type::<(TotalCount, SearchResults)>()
         .send()?;
 
     let repos = results.data.unwrap_or_default();
-    if let Some(msg) = truncation_warning(repos.len() as u64, args.limit) {
+    if let Some(msg) = truncation_warning(total, repos.len() as u64, args.limit) {
         eprintln!("{msg}");
     }
     if client.json_mode() {
@@ -95,20 +132,45 @@ pub fn run(client: &Client, args: Args) -> Result<()> {
     Ok(())
 }
 
-/// A result count that exactly equals `--limit` is a hit on the cap, not
+/// Tell the caller, on stderr, whether this page is the whole answer.
-/// necessarily the whole answer — see the module doc. Returns the stderr
+///
-/// warning to print, or `None` when the count came in under the cap (a
+/// `total` is `X-Total-Count` when the header parsed; `repo-search` always
-/// short result *is* the complete answer, same reasoning `list`'s own
+/// starts from page 1 (it has no `--page`, unlike `list`), so `count` alone
-/// `trailer()` uses for its full-page heuristic).
+/// is how much of `total` has been shown — no page-number multiply needed.
-fn truncation_warning(count: u64, limit: u64) -> Option<String> {
+///
-    (count == limit).then(|| {
+/// Without a usable total, falls back to the cruder `count == limit`
-        format!(
+/// heuristic: a hit on the cap, not necessarily the whole answer, but a
-            "warning: got exactly --limit ({limit}) results — the forge may be holding back \
+/// short result *is* the complete answer. This is the case the server's
-             more (and silently drops any repo you lack permission to see, with no signal \
+/// own clamp (`api.MAX_RESPONSE_ITEMS`, 50 by default) makes silently
-             either way); raise --limit or narrow --query rather than trusting this as a \
+/// wrong — `--limit 400` returning a clamped 50 reads as complete here,
-             complete list"
+/// which is exactly why the header-based path above is the rule and this
-        )
+/// is only the fallback (same relationship as `list.rs`'s `trailer()`).
-    })
+fn truncation_warning(total: Option<u64>, count: u64, limit: u64) -> Option<String> {
+    if count == 0 {
+        // "(no results)" is printed separately in `run` — a truncation
+        // warning here would wrongly imply more might still exist.
+        return None;
+    }
+    match total {
+        Some(total) => {
+            let remaining = total.saturating_sub(count);
+            (remaining > 0).then(|| {
+                format!(
+                    "warning: {count} of {total} results shown — {remaining} more exist (the \
+                     forge still silently drops any repo you lack permission to see, with no \
+                     signal either way); raise --limit or narrow --query to see them"
+                )
+            })
+        }
+        None => (count == limit).then(|| {
+            format!(
+                "warning: got exactly --limit ({limit}) results — the forge may be holding back \
+                 more (and silently drops any repo you lack permission to see, with no signal \
+                 either way); raise --limit or narrow --query rather than trusting this as a \
+                 complete list"
+            )
+        }),
+    }
 }
 
 #[cfg(test)]
@@ -116,19 +178,39 @@ mod tests {
     use super::*;
 
     #[test]
-    fn warns_when_count_hits_the_cap() {
+    fn warns_with_the_real_remaining_count_when_the_total_is_known() {
-        assert!(truncation_warning(30, 30).is_some());
+        let msg = truncation_warning(Some(187), 30, 30).expect("137 more exist");
+        assert!(msg.contains("30 of 187"), "{msg}");
+        assert!(msg.contains("157 more"), "{msg}");
     }
 
     #[test]
-    fn stays_quiet_under_the_cap() {
+    fn catches_the_clamp_the_old_count_eq_limit_heuristic_missed() {
-        assert_eq!(truncation_warning(9, 30), None);
+        // The regression this header read exists for: `--limit 400` against
+        // a forge that clamps to 50, with 187 matches. `count != limit`
+        // (50 != 400), so the old heuristic alone stayed silent here.
+        let msg = truncation_warning(Some(187), 50, 400).expect("137 more exist");
+        assert!(msg.contains("50 of 187"), "{msg}");
+        assert!(msg.contains("137 more"), "{msg}");
     }
 
     #[test]
-    fn stays_quiet_on_zero_results() {
+    fn stays_quiet_once_the_total_is_accounted_for() {
-        // An empty result needs "(no results)" (printed separately in
+        assert_eq!(truncation_warning(Some(30), 30, 30), None);
-        // `run`), not a truncation warning implying more might exist.
+    }
-        assert_eq!(truncation_warning(0, 30), None);
+
+    #[test]
+    fn falls_back_to_the_cap_heuristic_without_a_usable_total() {
+        assert!(truncation_warning(None, 30, 30).is_some());
+        assert_eq!(truncation_warning(None, 9, 30), None);
+    }
+
+    #[test]
+    fn stays_quiet_on_zero_results_even_with_a_total() {
+        // Permission-filtering can make count 0 while total is nonzero
+        // (every match is hidden from this caller) — still not a
+        // truncation warning's job; "(no results)" covers it in `run`.
+        assert_eq!(truncation_warning(Some(5), 0, 30), None);
+        assert_eq!(truncation_warning(None, 0, 30), None);
     }
 }