From 6681129a58d9c45661d657183f3a41e2758c8a03 Mon Sep 17 00:00:00 2001 From: rohoswagger Date: Tue, 12 May 2026 18:05:16 -0700 Subject: [PATCH 1/2] perf(adopt): scope to local branches by default, walk chains on demand MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ez adopt now uses targeted GraphQL lookups instead of paginating every PR in the repo. On onyx (~10k PRs) this drops the time to adopt 12 local PRs from ~120s to ~1.4s. Default mode (no args): - Fetch PRs only for local branches via get_pr_statuses_for. - No ancestor expansion. Local PRs whose base isn't local-with-PR or trunk are warned about with a hint to use ez adopt --pr to walk the remote chain explicitly. --branches : - Fetch named branches, then expand_ancestor_chains walks bases iteratively, fetching one batch per stack level via GraphQL. - Bounded by stack depth, not repo PR count. --pr : - Adds github::get_pr_by_number — single GraphQL call for one PR by number, returning head branch + info. - Same chain expansion as --branches. Internal: - Extract expand_ancestor_chains_with that takes the fetcher as a closure so tests can stub the network. - Extract orphan_local_prs helper for sorted, deterministic warnings. - Remove obsolete filter_prs_for_requested_branches and chain_to_trunk that the global-scan flow needed. 5 new unit tests: - expand_ancestor_chains fetches missing parents until trunk. - terminates when base has no upstream PR (no infinite loop on tried). - no fetches when all bases are already trunk. - orphan_local_prs flags branches with non-local non-trunk bases. - orphan output is sorted (deterministic warning order). Bumps version to 0.2.25. --- CLAUDE.md | 1 + Cargo.lock | 2 +- Cargo.toml | 2 +- src/cmd/adopt.rs | 314 ++++++++++++++++++++++++++++++++++++----------- src/github.rs | 35 ++++++ 5 files changed, 277 insertions(+), 77 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 9e484cd..dc83eac 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -156,6 +156,7 @@ These features exist specifically to make ez useable by AI agents: | 0.2.19 | Canonicalize `ez skill install` under `.agents/skills` with safe link-or-copy compatibility targets, improve worktree-awareness messaging and status output, and teach agents when to use `-A`/`-Am` for untracked files | | 0.2.20 | Add non-interactive `ez merge --yes`, support `ez merge --stack` for linear stacks, and restore remote branch cleanup after REST-based merges | | 0.2.24 | `ez sync`/`ez list` PR-status lookup uses one GraphQL request with aliased fields per branch instead of paginating every PR in the repo (122s → 1.8s on a 10k-PR repo); add local git-remote URL parser to skip the `gh repo view` round-trip when deriving owner/repo; `ez adopt` still uses the global PR scan because it needs the full PR graph. Adds `ez track [branch] [--parent ]` to bring a raw-git branch under ez management without rebasing — parent defaults to the closest tracked ancestor by merge-base, else trunk; updates `BranchNotInStack` error to hint at `ez track`. | +| 0.2.25 | `ez adopt` scopes to local branches by default — one GraphQL call for local-branch PRs instead of paginating every PR in the repo (122s → 1.4s adopting 12 PRs in a 10k-PR repo). `--branches ` and `--pr ` walk the ancestor chain on demand, fetching only the parents that show up in the chain (bounded by stack depth, not repo size). Adds `github::get_pr_by_number` for single-PR GraphQL lookup. Local PRs whose base isn't local-with-PR or trunk are warned about and skipped — users get a `Run \`ez adopt --pr \`` hint to walk the full chain via remote PR ancestors when needed. | --- diff --git a/Cargo.lock b/Cargo.lock index 80d750b..4cc7dd5 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -172,7 +172,7 @@ dependencies = [ [[package]] name = "ez-stack" -version = "0.2.24" +version = "0.2.25" dependencies = [ "anyhow", "clap", diff --git a/Cargo.toml b/Cargo.toml index 0b1e2b4..1637ebc 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "ez-stack" -version = "0.2.24" +version = "0.2.25" edition = "2024" rust-version = "1.85" description = "A CLI tool for managing stacked PRs with GitHub" diff --git a/src/cmd/adopt.rs b/src/cmd/adopt.rs index 70f5ef1..0a7fee1 100644 --- a/src/cmd/adopt.rs +++ b/src/cmd/adopt.rs @@ -101,49 +101,120 @@ fn build_adopt_graph(trunk: &str, prs: &HashMap) -> Vec< sorted } -fn chain_to_trunk( - prs: &HashMap, - trunk: &str, - branch: &str, -) -> Option> { - let mut chain = Vec::new(); - let mut current = branch.to_string(); - let mut seen = HashSet::new(); +fn adoption_parent_head(branch: &str, parent: &str) -> Result { + git::merge_base(branch, parent) +} +/// Walk the ancestor chain of every PR in `prs` and fetch any missing parent +/// PRs in batches. Terminates once no new bases are discovered. +/// +/// Each iteration issues at most one GraphQL request via `get_pr_statuses_for`. +/// Bounded by stack depth, not by repo PR count. +fn expand_ancestor_chains(prs: &mut HashMap, remote: &str, trunk: &str) { + expand_ancestor_chains_with(prs, trunk, |refs| github::get_pr_statuses_for(remote, refs)); +} + +/// Generic version of `expand_ancestor_chains` that takes the fetcher as a +/// closure so unit tests can stub out the network. +/// +/// A `tried` set prevents re-fetching branches that have no PR — without it +/// we'd loop forever on broken chains (an absent base is indistinguishable +/// from an unfetched one in a HashMap miss). +fn expand_ancestor_chains_with( + prs: &mut HashMap, + trunk: &str, + mut fetch: F, +) where + F: FnMut(&[&str]) -> HashMap, +{ + let mut tried: HashSet = HashSet::new(); loop { - if current == trunk { - return Some(chain); + let missing: Vec = prs + .values() + .map(|pr| pr.base.clone()) + .filter(|base| base != trunk && !prs.contains_key(base) && !tried.contains(base)) + .collect::>() + .into_iter() + .collect(); + if missing.is_empty() { + break; + } + for branch in &missing { + tried.insert(branch.clone()); } - if !seen.insert(current.clone()) { - return None; + let refs: Vec<&str> = missing.iter().map(String::as_str).collect(); + let new_prs = fetch(&refs); + if new_prs.is_empty() { + // None of the missing bases have PRs — chain terminates here. + break; } + prs.extend(new_prs); + } +} - let pr = prs.get(¤t)?; - chain.push(current.clone()); - current = pr.base.clone(); +/// Return the branches whose PRs base on something that's neither trunk nor +/// another local PR — these are dropped by `build_adopt_graph` as unrooted. +/// Output is sorted for deterministic warning order. +fn orphan_local_prs<'a>(prs: &'a HashMap, trunk: &str) -> Vec<&'a String> { + let mut orphans: Vec<&String> = prs + .iter() + .filter(|(_, pr)| pr.base != trunk && !prs.contains_key(&pr.base)) + .map(|(branch, _)| branch) + .collect(); + orphans.sort(); + orphans +} + +/// Default-mode fetcher: PRs for local branches only, no ancestor expansion. +/// One GraphQL call regardless of how many PRs exist on the remote. +/// +/// Returning only direct local PRs is intentional. If a local PR's base +/// isn't local-with-PR and isn't trunk, `build_adopt_graph` drops it as +/// unrooted and the caller warns the user. Hidden ancestor walking would +/// silently re-expand the per-PR cost we're trying to eliminate. +fn fetch_local_prs(remote: &str) -> Result> { + let local = git::branch_list().unwrap_or_default(); + if local.is_empty() { + return Ok(HashMap::new()); } + let refs: Vec<&str> = local.iter().map(String::as_str).collect(); + let mut prs = github::get_pr_statuses_for(remote, &refs); + prs.retain(|_, pr| pr.state == "OPEN" && !pr.merged); + Ok(prs) } -fn filter_prs_for_requested_branches( - prs: &HashMap, +/// Fetcher for `ez adopt feat/a feat/b`: PRs for named branches plus their +/// ancestor chain. Users name top-of-chain branches and expect ez to discover +/// the intermediate branches automatically. +fn fetch_prs_by_branches( + remote: &str, trunk: &str, branches: &[String], -) -> HashMap { - let mut branch_set = HashSet::new(); - for branch in branches { - if let Some(chain) = chain_to_trunk(prs, trunk, branch) { - branch_set.extend(chain); - } - } - - prs.iter() - .filter(|(branch, _)| branch_set.contains(*branch)) - .map(|(branch, pr)| (branch.clone(), pr.clone())) - .collect() +) -> Result> { + let refs: Vec<&str> = branches.iter().map(String::as_str).collect(); + let mut prs = github::get_pr_statuses_for(remote, &refs); + expand_ancestor_chains(&mut prs, remote, trunk); + prs.retain(|_, pr| pr.state == "OPEN" && !pr.merged); + Ok(prs) } -fn adoption_parent_head(branch: &str, parent: &str) -> Result { - git::merge_base(branch, parent) +/// Fetcher for `ez adopt --pr `: start from one PR (head branch may not +/// be local yet) and walk to trunk. Returns the seed PR's title so the +/// caller can format a useful error if the chain doesn't root. +fn fetch_prs_by_number( + remote: &str, + trunk: &str, + number: u64, +) -> Result<(String, HashMap)> { + let (head, pr) = github::get_pr_by_number(remote, number).ok_or_else(|| { + anyhow::anyhow!("PR #{number} not found — make sure it exists and is accessible") + })?; + let title = pr.title.clone(); + let mut prs = HashMap::new(); + prs.insert(head, pr); + expand_ancestor_chains(&mut prs, remote, trunk); + prs.retain(|_, p| p.state == "OPEN" && !p.merged); + Ok((title, prs)) } pub fn run(pr: Option, specific_branches: &[String]) -> Result<()> { @@ -163,56 +234,33 @@ pub fn run(pr: Option, specific_branches: &[String]) -> Result<()> { )); } - // Fetch all open PRs. - let sp = ui::spinner("Fetching PR graph from GitHub..."); - let all_prs = github::get_all_pr_statuses(); - sp.finish_and_clear(); - - if all_prs.is_empty() { - ui::info("No PRs found in this repository"); - return Ok(()); - } - - // If --pr is specified, find the specific PR and its chain. let candidates = if let Some(pr_number) = pr { - // Find the PR by number and clone the title for error messages. - let target_title = all_prs - .values() - .find(|p| p.number == pr_number) - .map(|p| p.title.clone()) - .ok_or_else(|| { - anyhow::anyhow!("PR #{pr_number} not found — make sure it exists and is open") - })?; - - let target_branch = all_prs - .iter() - .find(|(_, p)| p.number == pr_number) - .map(|(b, _)| b.clone()) - .unwrap(); - - // Build the full chain from this PR down to trunk. - let filtered = filter_prs_for_requested_branches(&all_prs, &state.trunk, &[target_branch]); - - let graph = build_adopt_graph(&state.trunk, &filtered); + let sp = ui::spinner(&format!("Fetching PR #{pr_number} and its chain...")); + let (title, prs) = fetch_prs_by_number(&state.remote, &state.trunk, pr_number)?; + sp.finish_and_clear(); + + let graph = build_adopt_graph(&state.trunk, &prs); if graph.is_empty() { bail!( "PR #{pr_number} (`{}`) does not lead back to trunk `{}`", - target_title, + title, state.trunk ); } graph } else if !specific_branches.is_empty() { - let filtered = filter_prs_for_requested_branches(&all_prs, &state.trunk, specific_branches); + let sp = ui::spinner("Fetching PRs for named branches..."); + let prs = fetch_prs_by_branches(&state.remote, &state.trunk, specific_branches)?; + sp.finish_and_clear(); - // Check for branches that have no PRs. + // Warn for branches the user named that have no open PR. for branch in specific_branches { - if !all_prs.contains_key(branch.as_str()) { + if !prs.contains_key(branch.as_str()) { ui::warn(&format!("Branch `{branch}` has no open PR — skipping")); } } - let graph = build_adopt_graph(&state.trunk, &filtered); + let graph = build_adopt_graph(&state.trunk, &prs); if graph.is_empty() { bail!( "None of the specified branches have open PRs rooted on `{}`", @@ -221,10 +269,40 @@ pub fn run(pr: Option, specific_branches: &[String]) -> Result<()> { } graph } else { - // Adopt all open PRs rooted on trunk. - let graph = build_adopt_graph(&state.trunk, &all_prs); + // Default: scope strictly to local branches with open PRs. No ancestor + // expansion — if a local PR's base isn't local-with-PR and isn't trunk, + // build_adopt_graph drops it as unrooted. Users who want broader + // adoption can pass --branches or --pr. + let sp = ui::spinner("Fetching PRs for local branches..."); + let prs = fetch_local_prs(&state.remote)?; + sp.finish_and_clear(); + + if prs.is_empty() { + ui::info("No open PRs found for local branches"); + ui::hint( + "Run `ez adopt --pr ` to adopt a specific PR, or `ez track` to track a branch without a PR", + ); + return Ok(()); + } + + // Warn about local PRs whose base isn't trunk and isn't another local + // PR — they would have been adopted by the old global-scan behavior + // but are dropped here because their parent isn't tracked. + for orphan in orphan_local_prs(&prs, &state.trunk) { + let pr_info = &prs[orphan]; + ui::warn(&format!( + "`{orphan}` (#{}) bases on `{}` which has no local PR — skipping", + pr_info.number, pr_info.base + )); + ui::hint(&format!( + "Run `ez adopt --pr {}` to walk the remote chain for this branch", + pr_info.number + )); + } + + let graph = build_adopt_graph(&state.trunk, &prs); if graph.is_empty() { - ui::info("No open PRs found that are rooted on trunk"); + ui::info("No open PRs found for local branches that root on trunk"); return Ok(()); } graph @@ -532,20 +610,106 @@ mod tests { } #[test] - fn requested_branch_chain_includes_ancestors() { + fn expand_ancestor_chains_fetches_missing_parents_until_trunk() { + // Seed: only feat/c. Walk should fetch feat/b then feat/a in stages. let mut prs = HashMap::new(); + let (k, v) = make_pr("feat/c", "feat/b", 3); + prs.insert(k, v); + + // The "remote" knows about feat/a → main and feat/b → feat/a. + let mut remote: HashMap = HashMap::new(); + let (k, v) = make_pr("feat/a", "main", 1); + remote.insert(k, v); + let (k, v) = make_pr("feat/b", "feat/a", 2); + remote.insert(k, v); + + let mut calls: Vec> = Vec::new(); + expand_ancestor_chains_with(&mut prs, "main", |refs| { + calls.push(refs.iter().map(|s| (*s).to_string()).collect()); + let mut out = HashMap::new(); + for r in refs { + if let Some(pr) = remote.get(*r) { + out.insert((*r).to_string(), pr.clone()); + } + } + out + }); + + // All three PRs are now present, with bases that root in trunk. + assert!(prs.contains_key("feat/a")); + assert!(prs.contains_key("feat/b")); + assert!(prs.contains_key("feat/c")); + // Two batches: first fetch feat/b, second fetch feat/a. (Iteration + // order within a batch isn't asserted because HashSet is unordered.) + assert_eq!(calls.len(), 2, "expected exactly two fetch batches"); + } + + #[test] + fn expand_ancestor_chains_terminates_when_base_has_no_pr() { + // feat/c bases on feat/b which has no PR upstream. The walk must not + // loop forever — `tried` prevents re-fetching the same missing branch. + let mut prs = HashMap::new(); + let (k, v) = make_pr("feat/c", "feat/b", 3); + prs.insert(k, v); + + let mut call_count = 0usize; + expand_ancestor_chains_with(&mut prs, "main", |_refs| { + call_count += 1; + HashMap::new() + }); + + // Exactly one batch: missing=[feat/b], fetch returns empty, loop exits. + assert_eq!(call_count, 1); + assert_eq!(prs.len(), 1); + assert!(prs.contains_key("feat/c")); + } + + #[test] + fn expand_ancestor_chains_does_nothing_when_all_bases_are_trunk() { + let mut prs = HashMap::new(); + let (k, v) = make_pr("feat/a", "main", 1); + prs.insert(k, v); + + let mut call_count = 0usize; + expand_ancestor_chains_with(&mut prs, "main", |_refs| { + call_count += 1; + HashMap::new() + }); + + // No fetches needed — the only base is already trunk. + assert_eq!(call_count, 0); + } + + #[test] + fn orphan_local_prs_flags_branches_whose_base_is_neither_trunk_nor_local() { + let mut prs = HashMap::new(); + // Rooted: feat/a → main. let (k, v) = make_pr("feat/a", "main", 1); prs.insert(k, v); + // Rooted via local: feat/b → feat/a (and feat/a is local). let (k, v) = make_pr("feat/b", "feat/a", 2); prs.insert(k, v); - let (k, v) = make_pr("feat/c", "feat/b", 3); + // Orphan: feat/c bases on feat/missing which is not in the map. + let (k, v) = make_pr("feat/c", "feat/missing", 3); prs.insert(k, v); - let filtered = filter_prs_for_requested_branches(&prs, "main", &["feat/c".to_string()]); - let graph = build_adopt_graph("main", &filtered); - let names: Vec<&str> = graph.iter().map(|c| c.branch.as_str()).collect(); + let orphans = orphan_local_prs(&prs, "main"); + assert_eq!(orphans, vec![&"feat/c".to_string()]); + } + + #[test] + fn orphan_local_prs_returns_sorted_for_stable_warning_order() { + let mut prs = HashMap::new(); + let (k, v) = make_pr("feat/z", "missing-x", 3); + prs.insert(k, v); + let (k, v) = make_pr("feat/a", "missing-y", 1); + prs.insert(k, v); + let (k, v) = make_pr("feat/m", "missing-z", 2); + prs.insert(k, v); - assert_eq!(names, vec!["feat/a", "feat/b", "feat/c"]); + let orphans = orphan_local_prs(&prs, "main"); + let names: Vec<&str> = orphans.iter().map(|s| s.as_str()).collect(); + assert_eq!(names, vec!["feat/a", "feat/m", "feat/z"]); } #[test] diff --git a/src/github.rs b/src/github.rs index 75f7437..ba078ae 100644 --- a/src/github.rs +++ b/src/github.rs @@ -194,6 +194,41 @@ pub fn get_pr_statuses_for( parse_pr_statuses_response(&value, branches) } +/// Fetch a single PR by number in one GraphQL request, returning the head +/// branch alongside its `PrInfo`. +/// +/// Used by `ez adopt --pr ` to bootstrap the adoption walk from one PR +/// without scanning the whole remote PR list. Returns `None` on any failure +/// (network, parse, auth, owner/repo resolution, or PR-not-found) — callers +/// surface the missing-PR case as a user-facing error. +pub fn get_pr_by_number(remote: &str, number: u64) -> Option<(String, PrInfo)> { + let (owner, name) = resolve_owner_repo(remote).ok()?; + + let query = "query($owner:String!,$name:String!,$num:Int!){repository(owner:$owner,name:$name){pullRequest(number:$num){number url state title baseRefName headRefName isDraft mergedAt}}}".to_string(); + let owner_arg = format!("owner={owner}"); + let name_arg = format!("name={name}"); + // `-F` (capital F) sends a typed value — Int for numeric inputs. + let num_arg = format!("num={number}"); + let query_arg = format!("query={query}"); + let json_str = run_gh(&[ + "api", "graphql", "-F", &owner_arg, "-F", &name_arg, "-F", &num_arg, "-f", &query_arg, + ]) + .ok()?; + let value: serde_json::Value = serde_json::from_str(&json_str).ok()?; + + parse_pr_by_number_response(&value) +} + +fn parse_pr_by_number_response(value: &serde_json::Value) -> Option<(String, PrInfo)> { + let node = &value["data"]["repository"]["pullRequest"]; + if node.is_null() { + return None; + } + let head = node["headRefName"].as_str()?.to_string(); + let pr = pr_info_from_graphql_node(node)?; + Some((head, pr)) +} + /// Resolve `(owner, name)` for the GitHub repo backing `remote`. /// /// Fast path: parse `git remote get-url ` locally (~10ms). Falls back From efd099ef1b317b6051f1140557dd8d2e95435672 Mon Sep 17 00:00:00 2001 From: rohoswagger Date: Tue, 12 May 2026 18:10:45 -0700 Subject: [PATCH 2/2] style: drop redundant comments from adopt and github diff Cleanup of comments that just restated function names or obvious code. Kept the two WHYs that matter: tried set prevents infinite loop on broken chains, default mode deliberately skips expansion to avoid re-introducing per-PR network cost. --- src/cmd/adopt.rs | 68 ++++++------------------------------------------ src/github.rs | 13 +++------ 2 files changed, 12 insertions(+), 69 deletions(-) diff --git a/src/cmd/adopt.rs b/src/cmd/adopt.rs index 0a7fee1..68af092 100644 --- a/src/cmd/adopt.rs +++ b/src/cmd/adopt.rs @@ -105,21 +105,10 @@ fn adoption_parent_head(branch: &str, parent: &str) -> Result { git::merge_base(branch, parent) } -/// Walk the ancestor chain of every PR in `prs` and fetch any missing parent -/// PRs in batches. Terminates once no new bases are discovered. -/// -/// Each iteration issues at most one GraphQL request via `get_pr_statuses_for`. -/// Bounded by stack depth, not by repo PR count. fn expand_ancestor_chains(prs: &mut HashMap, remote: &str, trunk: &str) { expand_ancestor_chains_with(prs, trunk, |refs| github::get_pr_statuses_for(remote, refs)); } -/// Generic version of `expand_ancestor_chains` that takes the fetcher as a -/// closure so unit tests can stub out the network. -/// -/// A `tried` set prevents re-fetching branches that have no PR — without it -/// we'd loop forever on broken chains (an absent base is indistinguishable -/// from an unfetched one in a HashMap miss). fn expand_ancestor_chains_with( prs: &mut HashMap, trunk: &str, @@ -127,6 +116,9 @@ fn expand_ancestor_chains_with( ) where F: FnMut(&[&str]) -> HashMap, { + // `tried` distinguishes "missing because we haven't fetched yet" from + // "missing because no PR exists upstream"; without it we'd loop forever + // re-fetching the same broken-chain base. let mut tried: HashSet = HashSet::new(); loop { let missing: Vec = prs @@ -145,16 +137,12 @@ fn expand_ancestor_chains_with( let refs: Vec<&str> = missing.iter().map(String::as_str).collect(); let new_prs = fetch(&refs); if new_prs.is_empty() { - // None of the missing bases have PRs — chain terminates here. break; } prs.extend(new_prs); } } -/// Return the branches whose PRs base on something that's neither trunk nor -/// another local PR — these are dropped by `build_adopt_graph` as unrooted. -/// Output is sorted for deterministic warning order. fn orphan_local_prs<'a>(prs: &'a HashMap, trunk: &str) -> Vec<&'a String> { let mut orphans: Vec<&String> = prs .iter() @@ -165,13 +153,6 @@ fn orphan_local_prs<'a>(prs: &'a HashMap, trunk: &str) - orphans } -/// Default-mode fetcher: PRs for local branches only, no ancestor expansion. -/// One GraphQL call regardless of how many PRs exist on the remote. -/// -/// Returning only direct local PRs is intentional. If a local PR's base -/// isn't local-with-PR and isn't trunk, `build_adopt_graph` drops it as -/// unrooted and the caller warns the user. Hidden ancestor walking would -/// silently re-expand the per-PR cost we're trying to eliminate. fn fetch_local_prs(remote: &str) -> Result> { let local = git::branch_list().unwrap_or_default(); if local.is_empty() { @@ -183,9 +164,6 @@ fn fetch_local_prs(remote: &str) -> Result> { Ok(prs) } -/// Fetcher for `ez adopt feat/a feat/b`: PRs for named branches plus their -/// ancestor chain. Users name top-of-chain branches and expect ez to discover -/// the intermediate branches automatically. fn fetch_prs_by_branches( remote: &str, trunk: &str, @@ -198,9 +176,6 @@ fn fetch_prs_by_branches( Ok(prs) } -/// Fetcher for `ez adopt --pr `: start from one PR (head branch may not -/// be local yet) and walk to trunk. Returns the seed PR's title so the -/// caller can format a useful error if the chain doesn't root. fn fetch_prs_by_number( remote: &str, trunk: &str, @@ -219,7 +194,6 @@ fn fetch_prs_by_number( pub fn run(pr: Option, specific_branches: &[String]) -> Result<()> { let mut state = StackState::load().or_else(|_| { - // If ez isn't initialized, try to auto-detect trunk and init. let trunk = git::default_branch().unwrap_or_else(|_| "main".to_string()); let state = StackState::new(trunk.clone()); state.save()?; @@ -227,7 +201,6 @@ pub fn run(pr: Option, specific_branches: &[String]) -> Result<()> { Ok::(state) })?; - // Check gh authentication. if !github::is_gh_authenticated() { bail!(EzError::GhError( "not authenticated — run `gh auth login` first".to_string() @@ -253,7 +226,6 @@ pub fn run(pr: Option, specific_branches: &[String]) -> Result<()> { let prs = fetch_prs_by_branches(&state.remote, &state.trunk, specific_branches)?; sp.finish_and_clear(); - // Warn for branches the user named that have no open PR. for branch in specific_branches { if !prs.contains_key(branch.as_str()) { ui::warn(&format!("Branch `{branch}` has no open PR — skipping")); @@ -269,10 +241,10 @@ pub fn run(pr: Option, specific_branches: &[String]) -> Result<()> { } graph } else { - // Default: scope strictly to local branches with open PRs. No ancestor - // expansion — if a local PR's base isn't local-with-PR and isn't trunk, - // build_adopt_graph drops it as unrooted. Users who want broader - // adoption can pass --branches or --pr. + // Default scopes strictly to local branches. Local PRs whose base + // isn't another local PR (or trunk) are warned and dropped — we + // deliberately don't auto-expand to the remote chain, since that + // would silently re-introduce per-PR network cost in large repos. let sp = ui::spinner("Fetching PRs for local branches..."); let prs = fetch_local_prs(&state.remote)?; sp.finish_and_clear(); @@ -285,9 +257,6 @@ pub fn run(pr: Option, specific_branches: &[String]) -> Result<()> { return Ok(()); } - // Warn about local PRs whose base isn't trunk and isn't another local - // PR — they would have been adopted by the old global-scan behavior - // but are dropped here because their parent isn't tracked. for orphan in orphan_local_prs(&prs, &state.trunk) { let pr_info = &prs[orphan]; ui::warn(&format!( @@ -308,7 +277,6 @@ pub fn run(pr: Option, specific_branches: &[String]) -> Result<()> { graph }; - // Report what we found. ui::header(&format!("Found {} branch(es) to adopt", candidates.len())); for c in &candidates { let draft = if c.is_draft { " [draft]" } else { "" }; @@ -323,13 +291,11 @@ pub fn run(pr: Option, specific_branches: &[String]) -> Result<()> { )); } - // Adopt each candidate. let mut adopted = 0; let mut skipped = 0; for candidate in &candidates { if state.is_managed(&candidate.branch) { - // Already tracked — just update PR number if missing. if let Ok(meta) = state.get_branch_mut(&candidate.branch) { if meta.pr_number.is_none() { meta.pr_number = Some(candidate.pr_number); @@ -343,14 +309,12 @@ pub fn run(pr: Option, specific_branches: &[String]) -> Result<()> { continue; } - // Ensure the local branch exists. Fetch from remote if needed. if !git::branch_exists(&candidate.branch) { ui::info(&format!("Fetching `{}` from remote...", candidate.branch)); let pr_ref = git::fetch_pr_head(&state.remote, candidate.pr_number)?; git::create_branch_at(&candidate.branch, &pr_ref)?; } - // Record the exact parent commit this branch is based on. let parent = &candidate.base; let parent_head = match adoption_parent_head(&candidate.branch, parent) { Ok(parent_head) => parent_head, @@ -373,10 +337,7 @@ pub fn run(pr: Option, specific_branches: &[String]) -> Result<()> { continue; } - // Add to stack state. state.add_branch(&candidate.branch, parent, &parent_head, None, None); - - // Set the PR number. if let Ok(meta) = state.get_branch_mut(&candidate.branch) { meta.pr_number = Some(candidate.pr_number); } @@ -392,7 +353,6 @@ pub fn run(pr: Option, specific_branches: &[String]) -> Result<()> { state.save()?; - // Summary. if adopted == 0 && skipped > 0 { ui::info(&format!("All {skipped} branch(es) were already tracked")); } else { @@ -611,12 +571,10 @@ mod tests { #[test] fn expand_ancestor_chains_fetches_missing_parents_until_trunk() { - // Seed: only feat/c. Walk should fetch feat/b then feat/a in stages. let mut prs = HashMap::new(); let (k, v) = make_pr("feat/c", "feat/b", 3); prs.insert(k, v); - // The "remote" knows about feat/a → main and feat/b → feat/a. let mut remote: HashMap = HashMap::new(); let (k, v) = make_pr("feat/a", "main", 1); remote.insert(k, v); @@ -635,19 +593,14 @@ mod tests { out }); - // All three PRs are now present, with bases that root in trunk. assert!(prs.contains_key("feat/a")); assert!(prs.contains_key("feat/b")); assert!(prs.contains_key("feat/c")); - // Two batches: first fetch feat/b, second fetch feat/a. (Iteration - // order within a batch isn't asserted because HashSet is unordered.) - assert_eq!(calls.len(), 2, "expected exactly two fetch batches"); + assert_eq!(calls.len(), 2, "expected one batch per stack level"); } #[test] fn expand_ancestor_chains_terminates_when_base_has_no_pr() { - // feat/c bases on feat/b which has no PR upstream. The walk must not - // loop forever — `tried` prevents re-fetching the same missing branch. let mut prs = HashMap::new(); let (k, v) = make_pr("feat/c", "feat/b", 3); prs.insert(k, v); @@ -658,7 +611,6 @@ mod tests { HashMap::new() }); - // Exactly one batch: missing=[feat/b], fetch returns empty, loop exits. assert_eq!(call_count, 1); assert_eq!(prs.len(), 1); assert!(prs.contains_key("feat/c")); @@ -676,20 +628,16 @@ mod tests { HashMap::new() }); - // No fetches needed — the only base is already trunk. assert_eq!(call_count, 0); } #[test] fn orphan_local_prs_flags_branches_whose_base_is_neither_trunk_nor_local() { let mut prs = HashMap::new(); - // Rooted: feat/a → main. let (k, v) = make_pr("feat/a", "main", 1); prs.insert(k, v); - // Rooted via local: feat/b → feat/a (and feat/a is local). let (k, v) = make_pr("feat/b", "feat/a", 2); prs.insert(k, v); - // Orphan: feat/c bases on feat/missing which is not in the map. let (k, v) = make_pr("feat/c", "feat/missing", 3); prs.insert(k, v); diff --git a/src/github.rs b/src/github.rs index ba078ae..edae633 100644 --- a/src/github.rs +++ b/src/github.rs @@ -194,20 +194,15 @@ pub fn get_pr_statuses_for( parse_pr_statuses_response(&value, branches) } -/// Fetch a single PR by number in one GraphQL request, returning the head -/// branch alongside its `PrInfo`. -/// -/// Used by `ez adopt --pr ` to bootstrap the adoption walk from one PR -/// without scanning the whole remote PR list. Returns `None` on any failure -/// (network, parse, auth, owner/repo resolution, or PR-not-found) — callers -/// surface the missing-PR case as a user-facing error. +/// Look up one PR by number. Returns `None` on any failure — callers surface +/// the missing-PR case as a user-facing error. pub fn get_pr_by_number(remote: &str, number: u64) -> Option<(String, PrInfo)> { let (owner, name) = resolve_owner_repo(remote).ok()?; - let query = "query($owner:String!,$name:String!,$num:Int!){repository(owner:$owner,name:$name){pullRequest(number:$num){number url state title baseRefName headRefName isDraft mergedAt}}}".to_string(); + let query = "query($owner:String!,$name:String!,$num:Int!){repository(owner:$owner,name:$name){pullRequest(number:$num){number url state title baseRefName headRefName isDraft mergedAt}}}"; let owner_arg = format!("owner={owner}"); let name_arg = format!("name={name}"); - // `-F` (capital F) sends a typed value — Int for numeric inputs. + // -F (capital) sends a typed value; required so $num arrives as Int. let num_arg = format!("num={number}"); let query_arg = format!("query={query}"); let json_str = run_gh(&[