From fa73cd1ddbfa6fd6b0961e85234d4c219fab4dc1 Mon Sep 17 00:00:00 2001 From: damocles Date: Fri, 28 Aug 2026 15:37:07 +0200 Subject: [PATCH] hive-forge: flag stale-branches merge outcome as unknown when the PR fetch is truncated --- .../plugins/base/skills/forge-triage/SKILL.md | 5 +++- hive-forge/src/verbs/lint.rs | 25 ++++++++++++++++--- 2 files changed, 26 insertions(+), 4 deletions(-) diff --git a/claude-plugins/plugins/base/skills/forge-triage/SKILL.md b/claude-plugins/plugins/base/skills/forge-triage/SKILL.md index ffc96883..a0a438cd 100644 --- a/claude-plugins/plugins/base/skills/forge-triage/SKILL.md +++ b/claude-plugins/plugins/base/skills/forge-triage/SKILL.md @@ -33,7 +33,10 @@ dimension directly. heads of open PRs) and reports each one's merge outcome (PR merged / closed unmerged / no PR at all) — squash-merge ancestry can't tell those apart, so don't delete a stale branch on the age alone; check - the verdict first. + the verdict first. On a repo with enough PRs to exceed the verb's own + page cap, an old branch's outcome may come back "unknown" instead of + a verdict — that's not the same claim as "no PR", treat it as + "can't tell, don't delete" too. ## Using them as a sweep diff --git a/hive-forge/src/verbs/lint.rs b/hive-forge/src/verbs/lint.rs index 287de2f5..b85e1805 100644 --- a/hive-forge/src/verbs/lint.rs +++ b/hive-forge/src/verbs/lint.rs @@ -289,6 +289,12 @@ fn run_stale_branches(client: &Client, args: StaleBranchesArgs) -> Result<()> { // hyperhive squash-merges, a merged branch's tip is never an // ancestor of main, so ancestry can't tell these apart; this is the // only signal that can. + // Whether the walk below hit `MAX_PAGES` without ever seeing a + // short page — i.e. there are more PRs than the cap fetched, so + // `latest_pr_by_head` is missing an unknown number of the oldest + // ones. Distinct from "no PR was found": that's a claim this walk + // can only make honestly when it actually saw everything. + let mut prs_truncated = false; let mut prs = Vec::new(); for page in 1..=MAX_PAGES { let query = RepoListPullRequestsQuery { @@ -306,6 +312,9 @@ fn run_stale_branches(client: &Client, args: StaleBranchesArgs) -> Result<()> { if short { break; } + if page == MAX_PAGES { + prs_truncated = true; + } } let mut active_heads: std::collections::HashSet = std::collections::HashSet::new(); // Highest-numbered (most recent) PR per head branch, in case a @@ -343,14 +352,20 @@ fn run_stale_branches(client: &Client, args: StaleBranchesArgs) -> Result<()> { if age_days >= args.days { // A branch surviving to here can only have a *closed* PR // (an open one would already be in `active_heads`), so - // `merged` is unambiguous when a PR exists at all. + // `merged` is unambiguous when a PR exists at all. But + // "no PR found" is only true when the PR walk above wasn't + // truncated — otherwise this branch's PR may simply be + // older than the page cap reached, and reporting "no PR" + // would misrepresent an unknown as a verified negative. let pr = latest_pr_by_head.get(branch_name); + let outcome_unknown = pr.is_none() && prs_truncated; stale.push(json!({ "name": branch_name, "last_commit": rfc3339(Some(ts)), "age_days": age_days, "pr": pr.and_then(|p| p.number), "merged": pr.and_then(|p| p.merged), + "outcome_unknown": outcome_unknown, })); } } @@ -360,9 +375,13 @@ fn run_stale_branches(client: &Client, args: StaleBranchesArgs) -> Result<()> { let verdict = match ( it.get("pr").and_then(Value::as_i64), it.get("merged").and_then(Value::as_bool), + it.get("outcome_unknown") + .and_then(Value::as_bool) + .unwrap_or(false), ) { - (Some(pr), Some(true)) => format!("PR #{pr} merged"), - (Some(pr), Some(false)) => format!("PR #{pr} closed, not merged"), + (Some(pr), Some(true), _) => format!("PR #{pr} merged"), + (Some(pr), Some(false), _) => format!("PR #{pr} closed, not merged"), + (_, _, true) => "unknown (PR history too large to fully scan)".to_string(), _ => "no PR".to_string(), }; format!("{name} ({age}d) — {verdict}")