From c335bc09628f8667a3d408768f49901044b0a224 Mon Sep 17 00:00:00 2001 From: damocles Date: Wed, 14 Oct 2026 18:34:01 +0200 Subject: [PATCH] repo-search: don't tell the caller to raise --limit once the forge has already clamped --- docs/tools/forge.md | 15 ++++++++---- hive-forge/src/verbs/repo_search.rs | 36 ++++++++++++++++++++++++++++- 2 files changed, 45 insertions(+), 6 deletions(-) diff --git a/docs/tools/forge.md b/docs/tools/forge.md index 60987899..8ad403cf 100644 --- a/docs/tools/forge.md +++ b/docs/tools/forge.md @@ -345,9 +345,14 @@ to discover valid label names before triaging or to audit the label set. 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 falls back to the cruder `count == limit` heuristic, same - 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. + trust-the-trailer-not-the-row-count principle as `list` above. Unlike + `list`, this verb has no `--page` — so once the forge's own page cap + has clamped the result (`count < limit`), raising `--limit` again asks + for the same clamp and gets nothing new; the warning says to narrow + `--query` instead in that case, and only suggests raising `--limit` + while still under the cap. 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. diff --git a/hive-forge/src/verbs/repo_search.rs b/hive-forge/src/verbs/repo_search.rs index 91a6eef2..8b492481 100644 --- a/hive-forge/src/verbs/repo_search.rs +++ b/hive-forge/src/verbs/repo_search.rs @@ -155,10 +155,23 @@ fn truncation_warning(total: Option, count: u64, limit: u64) -> Option { let remaining = total.saturating_sub(count); (remaining > 0).then(|| { + // `count < limit` means the forge clamped the page below what + // was asked for — raising `--limit` again asks for the same + // clamp and gets the same `count` back. Unlike `list`, this + // verb has no `--page` to walk past the clamp with, so + // narrowing `--query` is the only remedy that can work once + // the server has capped the page. "raise --limit" only + // helps the other case: still under the cap, where asking + // for more actually serves more of it. + let remedy = if count < limit { + "the forge capped this page at its own limit — narrow --query to see them" + } else { + "raise --limit or narrow --query to see them" + }; 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" + signal either way); {remedy}" ) }) } @@ -194,6 +207,27 @@ mod tests { assert!(msg.contains("137 more"), "{msg}"); } + #[test] + fn clamp_case_tells_the_caller_to_narrow_not_to_raise_limit() { + // `--limit` already exceeded the forge's own page cap here, so + // "raise --limit" is a dead end — only `--query` can still narrow + // the result set below the cap. (The bug argus caught: both + // branches used to say "raise --limit", which can't work once the + // server itself has clamped the page.) + let msg = truncation_warning(Some(187), 50, 400).expect("137 more exist"); + assert!(msg.contains("narrow --query"), "{msg}"); + assert!(!msg.contains("raise --limit"), "{msg}"); + } + + #[test] + fn under_the_cap_case_still_offers_raise_limit() { + // `count == limit` and both are under the forge's cap: asking for + // more can actually serve more, so "raise --limit" is live advice + // here, not a dead end. + let msg = truncation_warning(Some(187), 30, 30).expect("157 more exist"); + assert!(msg.contains("raise --limit"), "{msg}"); + } + #[test] fn stays_quiet_once_the_total_is_accounted_for() { assert_eq!(truncation_warning(Some(30), 30, 30), None);