From eab0abe5aee29570e408c4adef51c004c6f02d20 Mon Sep 17 00:00:00 2001 From: Eric Eldredge Date: Mon, 14 Sep 2026 17:29:49 -0400 Subject: [PATCH 1/4] feat(lib)!: read gh-stack merged state into stack metadata --- git-workon-lib/src/stack.rs | 7 +- git-workon-lib/src/stack/gh_stack.rs | 73 +++++++++++++-- git-workon-lib/src/stack/graphite.rs | 7 ++ git-workon-lib/src/stack/metadata.rs | 133 +++++++++++++++++++++++---- git-workon/src/display.rs | 1 + 5 files changed, 195 insertions(+), 26 deletions(-) diff --git a/git-workon-lib/src/stack.rs b/git-workon-lib/src/stack.rs index d9b613d6..bdfc0e8d 100644 --- a/git-workon-lib/src/stack.rs +++ b/git-workon-lib/src/stack.rs @@ -52,7 +52,7 @@ pub(crate) mod gh_stack; pub(crate) mod graphite; pub(crate) mod metadata; -use std::collections::{BTreeMap, HashMap}; +use std::collections::{BTreeMap, HashMap, HashSet}; use git2::Repository; @@ -147,6 +147,10 @@ pub struct Stack { /// `(trunk, sorted diff set)` and does not consult this field. Always `None` for /// [`StackModel::Graphite`] and [`StackModel::Git`], which have no numbering concept. pub number: Option, + /// Member branches (a subset of [`Stack::diffs`]) whose PR has merged, per + /// `BranchMetadata::merged`. Always empty for [`StackModel::Graphite`] — see + /// [`graphite::read_branch_metadata`]'s docs for why merged state isn't observable there. + pub merged: HashSet, } /// Return all stacks present in metadata, one per connected component. @@ -403,6 +407,7 @@ mod tests { current: current.to_string(), parents: HashMap::new(), number: None, + merged: HashSet::new(), } } diff --git a/git-workon-lib/src/stack/gh_stack.rs b/git-workon-lib/src/stack/gh_stack.rs index 89acaa7a..883d5c60 100644 --- a/git-workon-lib/src/stack/gh_stack.rs +++ b/git-workon-lib/src/stack/gh_stack.rs @@ -60,17 +60,11 @@ use super::metadata::{self, BranchMetadata, StackMetadata}; use super::Stack; use crate::error::StackError; -/// One `branchRef` entry (`{ branch, head, base, pullRequest }`), pulled from a raw -/// `serde_json::Value` rather than a derived struct (this crate has no `serde` derive -/// dependency, only `serde_json`; see `graphite.rs` for the same raw-`Value` convention). -/// `head` and `pullRequest` are read from the file but not carried into [`StackMetadata`]: -/// assembly uses the live tip for `head`, and `pullRequest` has no `StackMetadata` field. -/// Both matter to the write path (added in a later changeset), which round-trips the raw -/// `Value` to preserve them. #[derive(Debug)] struct GhStackBranchRef { branch: String, base: String, + merged: bool, } impl GhStackBranchRef { @@ -82,6 +76,11 @@ impl GhStackBranchRef { .and_then(|v| v.as_str()) .unwrap_or_default() .to_string(), + merged: value + .get("pullRequest") + .and_then(|pr| pr.get("merged")) + .and_then(|v| v.as_bool()) + .unwrap_or(false), }) } } @@ -329,6 +328,7 @@ pub(crate) fn read_metadata(repo: &Repository) -> Result, + pub merged: bool, } /// Provider-agnostic stack metadata: a trunk set plus `branch → (parent, parent_revision)`, @@ -42,7 +36,8 @@ pub(crate) struct StackMetadata { pub stack_numbers: HashMap, } -/// Return all stacks present in `meta`, one per connected component, ghost branches PRUNED. +/// Return all stacks present in `meta`, one per connected component, ghost AND merged +/// branches PRUNED. /// /// A "connected component" is the set of all non-trunk branches reachable from a single direct /// child of a trunk branch. This is the same grouping key used by `group_by_stack`, so each @@ -50,8 +45,10 @@ pub(crate) struct StackMetadata { /// /// Ghost branches — those present in metadata but whose branch ref no longer exists /// (merged/deleted while the provider's records linger) — are dropped before the BFS so they -/// do not surface as metadata nodes in `list`/`find`. (`current` deliberately does NOT prune — -/// routing needs deleted nodes to stay visible.) +/// do not surface as metadata nodes in `list`/`find`. Merged branches (per +/// `BranchMetadata::merged`) are dropped the same way: `gh stack sync` leaves a merged +/// branch's row behind rather than deleting it, and a merged branch with no worktree has +/// nothing left worth surfacing as a metadata-only node. pub(crate) fn enumerate(repo: &Repository, meta: &StackMetadata) -> Vec { let mut parent_map: HashMap = meta .parents @@ -67,6 +64,24 @@ pub(crate) fn enumerate(repo: &Repository, meta: &StackMetadata) -> Vec { // not in the parent map (they are values, not keys), so they are always preserved. parent_map.retain(|branch, _| crate::resolve::branch_exists(repo, branch)); + // Drop merged branches: `list.rs` still renders a merged branch that a worktree covers + // (its row comes from `current`, not this pruned map), but a metadata-only merged node has + // nothing left to show. Unlike the ghost prune above, a merged branch's live descendants + // must survive: `gh stack sync` merges a stack bottom-up and leaves the rest of the stack + // recorded as children of the merged row, so the BFS below (seeded only from direct trunk + // children) would otherwise lose the whole remaining stack. Reparent each branch past every + // merged ancestor first, then drop the merged rows. + let is_merged = |b: &str| meta.parents.get(b).map(|m| m.merged).unwrap_or(false); + for parent in parent_map.values_mut() { + while is_merged(parent) { + match meta.parents.get(parent.as_str()) { + Some(m) => *parent = m.parent.clone(), + None => break, + } + } + } + parent_map.retain(|branch, _| !is_merged(branch)); + if parent_map.is_empty() { return vec![]; } @@ -133,6 +148,7 @@ pub(crate) fn enumerate(repo: &Repository, meta: &StackMetadata) -> Vec { current, parents, number, + merged: HashSet::new(), // every merged row was dropped from `parent_map` above }); } } @@ -223,12 +239,18 @@ pub(crate) fn current(meta: &StackMetadata, head_branch: &str) -> Option let number = stack_branches .iter() .find_map(|b| meta.stack_numbers.get(b).copied()); + let merged: HashSet = stack_branches + .iter() + .filter(|b| meta.parents.get(*b).map(|m| m.merged).unwrap_or(false)) + .cloned() + .collect(); Some(Stack { trunk, diffs: stack_branches, current: head_branch.to_string(), parents, number, + merged, }) } @@ -316,7 +338,12 @@ mod tests { use super::*; use git_workon_fixture::prelude::*; - fn meta(trunk: &str, parents: &[(&str, &str)], numbers: &[(&str, u64)]) -> StackMetadata { + fn meta( + trunk: &str, + parents: &[(&str, &str)], + numbers: &[(&str, u64)], + merged: &[&str], + ) -> StackMetadata { StackMetadata { trunks: vec![trunk.to_string()], parents: parents @@ -327,6 +354,7 @@ mod tests { BranchMetadata { parent: parent.to_string(), parent_revision: None, + merged: merged.contains(branch), }, ) }) @@ -354,6 +382,7 @@ mod tests { "main", &[("feat-a", "main"), ("feat-b", "feat-a")], &[("feat-b", 12)], + &[], ); let stacks = enumerate(repo, &meta); @@ -366,19 +395,87 @@ mod tests { let fixture = FixtureBuilder::new().branch("feat-a").build().unwrap(); let repo = fixture.repo().unwrap(); - let meta = meta("main", &[("feat-a", "main")], &[]); + let meta = meta("main", &[("feat-a", "main")], &[], &[]); let stacks = enumerate(repo, &meta); assert_eq!(stacks.len(), 1); assert_eq!(stacks[0].number, None); } + #[test] + fn enumerate_drops_merged_branch_but_keeps_live_sibling() { + let fixture = FixtureBuilder::new() + .branch("feat-a") + .branch("feat-b") + .build() + .unwrap(); + let repo = fixture.repo().unwrap(); + + let meta = meta( + "main", + &[("feat-a", "main"), ("feat-b", "main")], + &[], + &["feat-a"], + ); + + let stacks = enumerate(repo, &meta); + assert_eq!(stacks.len(), 1); + assert_eq!(stacks[0].diffs, vec!["feat-b".to_string()]); + assert!(stacks[0].merged.is_empty()); + } + + #[test] + fn enumerate_reparents_live_descendants_of_merged_branch() { + let fixture = FixtureBuilder::new() + .branch("feat-a") + .branch("feat-b") + .branch("feat-c") + .build() + .unwrap(); + let repo = fixture.repo().unwrap(); + + let meta = meta( + "main", + &[ + ("feat-a", "main"), + ("feat-b", "feat-a"), + ("feat-c", "feat-b"), + ], + &[], + &["feat-a"], + ); + + let stacks = enumerate(repo, &meta); + assert_eq!(stacks.len(), 1); + assert_eq!( + stacks[0].diffs, + vec!["feat-b".to_string(), "feat-c".to_string()] + ); + assert_eq!(stacks[0].parents["feat-b"], "main"); + assert_eq!(stacks[0].parents["feat-c"], "feat-b"); + } + + #[test] + fn current_retains_merged_branch_and_records_it() { + let meta = meta( + "main", + &[("feat-a", "main"), ("feat-b", "feat-a")], + &[], + &["feat-a"], + ); + + let stack = current(&meta, "feat-b").expect("feat-b is tracked"); + assert!(stack.diffs.contains(&"feat-a".to_string())); + assert_eq!(stack.merged, HashSet::from(["feat-a".to_string()])); + } + #[test] fn current_sets_stack_number_from_any_member_branch() { let meta = meta( "main", &[("feat-a", "main"), ("feat-b", "feat-a")], &[("feat-a", 7)], + &[], ); let stack = current(&meta, "feat-b").expect("feat-b is tracked"); diff --git a/git-workon/src/display.rs b/git-workon/src/display.rs index eb965c24..11a190d6 100644 --- a/git-workon/src/display.rs +++ b/git-workon/src/display.rs @@ -1741,6 +1741,7 @@ mod tests { .map(|(c, p)| (c.to_string(), p.to_string())) .collect(), number: None, + merged: HashSet::new(), }, members: vec![], } From bc5f694f463fff16698fb66d67462977eef1ffb4 Mon Sep 17 00:00:00 2001 From: Eric Eldredge Date: Mon, 14 Sep 2026 17:30:01 -0400 Subject: [PATCH 2/4] feat(fixture): write merged pullRequest on gh-stack branch --- git-workon-fixture/src/fixture_builder.rs | 87 +++++++++++++++++++++-- 1 file changed, 83 insertions(+), 4 deletions(-) diff --git a/git-workon-fixture/src/fixture_builder.rs b/git-workon-fixture/src/fixture_builder.rs index 0e9513f5..3d93d5d2 100644 --- a/git-workon-fixture/src/fixture_builder.rs +++ b/git-workon-fixture/src/fixture_builder.rs @@ -126,6 +126,9 @@ struct GhStackBranchSpec { /// Ghost entries simulate a branch gh-stack still tracks but whose git ref was /// deleted/merged — no local branch ref is created for these. ghost: bool, + /// Writes `pullRequest: { number, merged: true }` instead of `pullRequest: null`, + /// simulating a branch `gh stack sync` has recorded as merged. + merged: bool, } /// One `stacks[]` entry queued by [`FixtureBuilder::gh_stack`]/[`FixtureBuilder::gh_stack_at`]. @@ -153,6 +156,13 @@ enum GhStackOp { number: u64, branch: String, }, + /// Flip a branch already queued by a prior [`GhStackOp::Stack`] on the `target` file's stack + /// numbered `number` to `merged: true`. + MergedBranch { + target: GhStackTarget, + number: u64, + branch: String, + }, /// Overwrite `target`'s file with raw bytes after all JSON writes — truncated, `v2`, or /// plain garbage content for error-path tests. Raw { @@ -435,6 +445,7 @@ impl<'fixture> FixtureBuilder<'fixture> { branch: branch.to_string(), base: GhStackBase::ResolveParentTip, ghost: false, + merged: false, }) .collect(), }; @@ -464,6 +475,7 @@ impl<'fixture> FixtureBuilder<'fixture> { branch: branch.to_string(), base: GhStackBase::Verbatim(base.to_string()), ghost: false, + merged: false, }) .collect(), }; @@ -493,6 +505,27 @@ impl<'fixture> FixtureBuilder<'fixture> { self } + /// Mark an already-queued branch on `worktree`'s stack numbered `number` as merged — + /// writes `pullRequest: { number, merged: true }` for it instead of `pullRequest: null`, + /// simulating `gh stack sync` recording a merged PR. + /// + /// A [`gh_stack`](Self::gh_stack)/[`gh_stack_at`](Self::gh_stack_at) call queueing `branch` + /// on this `(worktree, number)` must come first — `build()` panics otherwise, mirroring + /// [`gh_stack_ghost_branch`](Self::gh_stack_ghost_branch). + pub fn gh_stack_merged_branch( + mut self, + worktree: Option<&str>, + number: u64, + branch: &str, + ) -> Self { + self.gh_stack_ops.push(GhStackOp::MergedBranch { + target: worktree.map(str::to_string), + number, + branch: branch.to_string(), + }); + self + } + /// Overwrite `worktree`'s gh-stack file with raw `bytes` after all other gh-stack writes — /// for truncated-read, `schemaVersion: 2`, and garbage-content error-path tests. pub fn raw_gh_stack(mut self, worktree: Option<&str>, bytes: Vec) -> Self { @@ -913,12 +946,22 @@ impl<'fixture> FixtureBuilder<'fixture> { } } - fn branch_ref_json(branch: &str, head: String, base: String) -> serde_json::Value { + fn branch_ref_json( + branch: &str, + head: String, + base: String, + merged: bool, + ) -> serde_json::Value { + let pull_request = if merged { + serde_json::json!({ "number": 1, "merged": true }) + } else { + serde_json::Value::Null + }; serde_json::json!({ "branch": branch, "head": head, "base": base, - "pullRequest": serde_json::Value::Null, + "pullRequest": pull_request, }) } @@ -980,8 +1023,44 @@ impl<'fixture> FixtureBuilder<'fixture> { branch: branch.clone(), base: GhStackBase::ResolveParentTip, ghost: true, + merged: false, }); } + GhStackOp::MergedBranch { + target, + number, + branch, + } => { + let i = target_index(&stacks_by_target, target).unwrap_or_else(|| { + panic!( + "gh_stack_merged_branch({target:?}, {number}, {branch:?}): \ + no prior gh_stack/gh_stack_at queued a stack numbered {number} \ + for this target" + ) + }); + let entries = &mut stacks_by_target[i].1; + let spec = entries + .iter_mut() + .find(|s| s.number == *number) + .unwrap_or_else(|| { + panic!( + "gh_stack_merged_branch({target:?}, {number}, {branch:?}): \ + no queued stack numbered {number} for this target" + ) + }); + let branch_spec = spec + .branches + .iter_mut() + .find(|b| &b.branch == branch) + .unwrap_or_else(|| { + panic!( + "gh_stack_merged_branch({target:?}, {number}, {branch:?}): \ + no queued branch {branch:?} on stack numbered {number} \ + for this target" + ) + }); + branch_spec.merged = true; + } _ => {} } } @@ -1008,13 +1087,13 @@ impl<'fixture> FixtureBuilder<'fixture> { } }; let head = resolve_tip(&b.branch).unwrap_or_default(); - branch_ref_json(&b.branch, head, base) + branch_ref_json(&b.branch, head, base, b.merged) }) .collect(); serde_json::json!({ "id": format!("id-{}", spec.number), "number": spec.number, - "trunk": branch_ref_json(&spec.trunk, trunk_tip, String::new()), + "trunk": branch_ref_json(&spec.trunk, trunk_tip, String::new(), false), "branches": branches_json, }) }) From 59d5d3dbe8c214347e4cf70f69cdc7cdc1929f26 Mon Sep 17 00:00:00 2001 From: Eric Eldredge Date: Mon, 14 Sep 2026 17:34:57 -0400 Subject: [PATCH 3/4] feat(cli): mark merged stack branches and hide merged metadata-only nodes --- git-workon/src/cmd/list.rs | 3 + git-workon/src/display.rs | 159 ++++++++++++++++++++++++++++++++- git-workon/tests/suite/list.rs | 89 ++++++++++++++++++ 3 files changed, 249 insertions(+), 2 deletions(-) diff --git a/git-workon/src/cmd/list.rs b/git-workon/src/cmd/list.rs index ff8b5b60..ea224ec9 100644 --- a/git-workon/src/cmd/list.rs +++ b/git-workon/src/cmd/list.rs @@ -175,12 +175,15 @@ impl Run for List { .iter() .map(|(child, parent)| (child.clone(), json!(parent))) .collect(); + let mut merged: Vec<&String> = group.stack.merged.iter().collect(); + merged.sort(); json!({ "trunk": group.stack.trunk, "diffs": group.stack.diffs, "parents": parents, "checkouts": checkouts, "number": group.stack.number, + "merged": merged, }) }) .collect(); diff --git a/git-workon/src/display.rs b/git-workon/src/display.rs index 11a190d6..ddd48a88 100644 --- a/git-workon/src/display.rs +++ b/git-workon/src/display.rs @@ -271,6 +271,10 @@ pub struct TreeNode { /// trunk root itself — `build_tree` merges every stack on one trunk into a single root /// node, so the trunk has no single number to show). pub stack_number: Option, + /// Whether this branch's PR has merged (`Stack::merged`). Only ever `true` on a node that + /// also `has_worktree()` — a merged branch with no worktree is dropped from the tree + /// entirely in `build_children`, its live children reparented up. + pub merged: bool, } impl TreeNode { @@ -314,6 +318,7 @@ pub fn build_tree( // populated for descendants further down the tree, so a plain lookup at any depth in // `build_children` naturally only matches root children — see `TreeNode::stack_number`. let mut direct_child_numbers: HashMap = HashMap::new(); + let mut merged_branches: HashSet = HashSet::new(); for group in groups { let rev = per_trunk_reverse .entry(group.stack.trunk.clone()) @@ -328,6 +333,7 @@ pub fn build_tree( } } } + merged_branches.extend(group.stack.merged.iter().cloned()); } // Sort children for determinism for rev_map in per_trunk_reverse.values_mut() { @@ -372,6 +378,7 @@ pub fn build_tree( &mut rows, &mut visited, &direct_child_numbers, + &merged_branches, ); let subtree_activity = subtree_max(trunk_activity, &children); @@ -384,6 +391,7 @@ pub fn build_tree( subtree_size, children, stack_number: None, // never on the trunk root — see `TreeNode::stack_number`. + merged: false, // a trunk is never itself a stack diff. }); } @@ -414,6 +422,7 @@ pub fn build_tree( row: Some(row), children: vec![], stack_number: None, // ungrouped: not part of a numbered stack. + merged: false, // ungrouped: not part of any tracked stack. }); } @@ -452,6 +461,7 @@ fn build_children( rows: &mut HashMap, visited: &mut HashSet, direct_child_numbers: &HashMap, + merged_branches: &HashSet, ) -> Vec { let mut nodes = Vec::new(); for branch in branch_names { @@ -461,6 +471,7 @@ fn build_children( let idx = branch_to_idx.get(branch).copied(); let row = idx.and_then(|i| rows.remove(&i)); let epoch = row.as_ref().and_then(|r| r.activity_epoch); + let merged = merged_branches.contains(branch); let grandchildren_names = rev_map.get(branch.as_str()).cloned().unwrap_or_default(); let children = build_children( @@ -470,7 +481,17 @@ fn build_children( rows, visited, direct_child_numbers, + merged_branches, ); + + // A merged branch with no worktree has nothing left to show: fold its children up as + // if they were direct children of this node's own parent, instead of nesting them + // under a node that never renders. + if merged && row.is_none() { + nodes.extend(children); + continue; + } + let subtree_activity = subtree_max(epoch, &children); let subtree_size = subtree_size(&children); @@ -483,6 +504,7 @@ fn build_children( // Only true direct children of a trunk are keys here (see `build_tree`), so a // plain lookup at any recursion depth naturally yields `None` past the root. stack_number: direct_child_numbers.get(branch).copied(), + merged, }); } nodes @@ -549,6 +571,7 @@ fn lane_content_width(own_lane: usize, closing_count: usize, node: &TreeNode) -> + stack_number_suffix(node.stack_number) .map(|s| s.width()) .unwrap_or(0) + + merged_suffix(node.merged).map(|s| s.width()).unwrap_or(0) } /// The plain (unstyled) `" #N"` suffix for a stack number, or `None` when there isn't one. @@ -558,6 +581,11 @@ fn stack_number_suffix(number: Option) -> Option { number.map(|n| format!(" #{n}")) } +/// The plain (unstyled) `" merged"` suffix, or `None` when the branch isn't merged. +fn merged_suffix(merged: bool) -> Option { + merged.then(|| " merged".to_string()) +} + /// Render the gutter string for a lane row. /// /// Produces: `(│ | )* glyph [─╯ | ─┴─╯ | …]` @@ -792,6 +820,9 @@ pub fn format_tree_lines( // `style::dim`), set only on a direct child of a trunk — see `TreeNode::stack_number`. let number_ann = stack_number_suffix(node.stack_number).map(|s| style::dim(&s)); + // Merged annotation: dim ` merged`, right after the number in the same slot/style. + let merged_ann = merged_suffix(node.merged).map(|s| style::dim(&s)); + // Padding to align the indicator column. let this_content_w = lane_content_width(lr.own_lane, lr.closing_count, node); let content_pad = max_content_w.saturating_sub(this_content_w); @@ -827,13 +858,15 @@ pub fn format_tree_lines( // Assemble line: [gutter] [label][path][number][pad] [indicators][ind_pad] [activity][here] let path_str = path_ann.unwrap_or_default(); let number_str = number_ann.unwrap_or_default(); + let merged_str = merged_ann.unwrap_or_default(); let line = if node.row.is_some() { format!( - "{} {}{}{}{} {}{} {}{}", + "{} {}{}{}{}{} {}{} {}{}", gutter, label, path_str, number_str, + merged_str, " ".repeat(content_pad), ind_str, " ".repeat(ind_pad), @@ -990,12 +1023,16 @@ pub fn render_tree(forest: &[TreeNode], query: &str, matcher: &SkimMatcherV2) -> // No `← here` marker: the picker cursor serves that role. let path_str = path_ann.unwrap_or_default(); + let merged_str = merged_suffix(node.merged) + .map(|s| style::dim(&s)) + .unwrap_or_default(); let line = if node.row.is_some() { format!( - "{} {}{}{} {}{} {}", + "{} {}{}{}{} {}{} {}", gutter, label, path_str, + merged_str, " ".repeat(content_pad), ind_str, " ".repeat(ind_pad), @@ -1383,6 +1420,7 @@ mod tests { subtree_size: 1, children: vec![], stack_number: None, + merged: false, } } @@ -1399,6 +1437,7 @@ mod tests { subtree_size: 1, children: vec![], stack_number: None, + merged: false, } } @@ -1708,6 +1747,7 @@ mod tests { subtree_size: 1, children: vec![], stack_number: None, + merged: false, } } @@ -1827,6 +1867,85 @@ mod tests { assert_eq!(find_node(&forest, "top").stack_number, None); } + /// Like [`find_node`], but `None` instead of panicking — for asserting a branch is absent. + fn find_node_opt<'a>(forest: &'a [TreeNode], branch: &str) -> Option<&'a TreeNode> { + fn find<'a>(nodes: &'a [TreeNode], branch: &str) -> Option<&'a TreeNode> { + for node in nodes { + if node.branch == branch { + return Some(node); + } + if let Some(found) = find(&node.children, branch) { + return Some(found); + } + } + None + } + find(forest, branch) + } + + #[test] + fn build_tree_drops_merged_metadata_only_node_and_reparents_its_child() { + // "mid" is merged and no worktree checks it out (`all_worktrees` is empty), so it must + // be dropped entirely and its child "top" reparented under "base" instead of vanishing + // with it. + let mut group = meta_group( + &["base", "mid", "top"], + &[("base", "main"), ("mid", "base"), ("top", "mid")], + ); + group.stack.merged = HashSet::from(["mid".to_string()]); + let groups = vec![group]; + let forest = build_tree(&[], &groups, &[], Path::new("/repo"), Path::new("/repo")); + + assert!( + find_node_opt(&forest, "mid").is_none(), + "merged metadata-only node must be dropped" + ); + let base = find_node(&forest, "base"); + assert!( + base.children.iter().any(|c| c.branch == "top"), + "top must be reparented under base once mid is dropped, got children: {:?}", + base.children.iter().map(|c| &c.branch).collect::>() + ); + } + + #[test] + fn build_children_marks_merged_node_with_a_worktree_but_keeps_it() { + let mut branch_to_idx = HashMap::new(); + branch_to_idx.insert("base".to_string(), 0usize); + let mut rows = HashMap::new(); + rows.insert( + 0, + WorktreeDisplayRow { + is_active: false, + dir_name: "base".to_string(), + branch_annotation: None, + indicators: vec![], + last_activity: String::new(), + activity_epoch: None, + }, + ); + let mut visited = HashSet::new(); + let merged_branches = HashSet::from(["base".to_string()]); + + let nodes = build_children( + &["base".to_string()], + &HashMap::new(), + &branch_to_idx, + &mut rows, + &mut visited, + &HashMap::new(), + &merged_branches, + ); + + assert_eq!( + nodes.len(), + 1, + "a merged node with a worktree still renders" + ); + assert!(nodes[0].has_worktree()); + assert!(nodes[0].merged); + } + #[test] fn format_tree_lines_renders_dim_stack_number_suffix() { no_color(); @@ -1841,6 +1960,42 @@ mod tests { ); } + #[test] + fn format_tree_lines_renders_dim_merged_suffix_on_worktree_row() { + no_color(); + let mut root = leaf("base", false); + root.merged = true; + let (lines, _) = format_tree_lines(&[root], false); + assert_eq!(lines.len(), 1); + assert!( + lines[0].contains("base merged"), + "expected a ' merged' suffix (plain text under NO_COLOR) right after the label, got: {:?}", + lines[0] + ); + } + + #[test] + fn format_tree_lines_indicator_column_aligned_across_lanes_with_merged_suffix() { + no_color(); + let s1 = leaf_with_data("s1", false, &["*"], "2h ago"); + let shared = leaf_with_data("shared", false, &[], "3d ago"); + let mut root = leaf_with_data("main", true, &["↑"], "1d ago"); + root.merged = true; + set_children(&mut root, vec![s1, shared]); + let forest = vec![root]; + + let (lines, _) = format_tree_lines(&forest, true); + assert_eq!(lines.len(), 3, "expected 3 rows"); + + let s1_ind_col = display_col_of(&lines[0], "*", "s1"); + let main_ind_col = display_col_of(&lines[2], "↑", "main"); + assert_eq!( + s1_ind_col, main_ind_col, + "indicator column must be aligned despite the merged suffix:\ns1: {:?}\nmain: {:?}", + lines[0], lines[2] + ); + } + #[test] fn format_tree_lines_stack_member_indented_trunk_and_ungrouped_flush_left() { // Column 0 is reserved for bases (rule 1): the trunk and an ungrouped worktree both diff --git a/git-workon/tests/suite/list.rs b/git-workon/tests/suite/list.rs index 30811150..7873fc8a 100644 --- a/git-workon/tests/suite/list.rs +++ b/git-workon/tests/suite/list.rs @@ -1403,3 +1403,92 @@ fn list_json_gh_stack_two_stacks_on_shared_trunk_include_respective_numbers( Ok(()) } + +#[test] +fn list_json_reports_merged_branches_in_the_stack_merged_array( +) -> Result<(), Box> { + let fixture = FixtureBuilder::new() + .bare(true) + .default_branch("main") + .config("workon.stackModel", "gh-stack") + .worktree("main") + .worktree("feat-a") + .worktree("feat-b") + .gh_stack(None, 1, "main", &["feat-a", "feat-b"]) + .gh_stack_merged_branch(None, 1, "feat-a") + .build()?; + + let main_path = fixture.root()?.join("main"); + let output = cargo_bin_cmd!("git-workon") + .current_dir(&main_path) + .env("NO_COLOR", "1") + .arg("list") + .arg("--json") + .output()?; + + assert!(output.status.success()); + let stdout = std::str::from_utf8(&output.stdout)?; + let parsed: serde_json::Value = serde_json::from_str(stdout)?; + + let stacks = parsed["stacks"] + .as_array() + .unwrap_or_else(|| panic!("expected stacks array in: {stdout}")); + let group = stacks + .iter() + .find(|g| { + g["diffs"] + .as_array() + .is_some_and(|d| d.iter().any(|v| v == "feat-a")) + }) + .unwrap_or_else(|| panic!("no stack group containing feat-a in: {stdout}")); + assert_eq!( + group["merged"], + serde_json::json!(["feat-a"]), + "feat-a must appear in the stack's merged array: {stdout}" + ); + + Ok(()) +} + +#[test] +fn list_tree_marks_merged_worktree_branch_and_hides_merged_metadata_only_branch( +) -> Result<(), Box> { + // feat-a is merged and has a worktree: it must still render, with a " merged" marker. + // feat-b is merged and has NO worktree: it must not appear at all. + let fixture = FixtureBuilder::new() + .bare(true) + .default_branch("main") + .config("workon.stackModel", "gh-stack") + .worktree("main") + .worktree("feat-a") + .gh_stack(None, 1, "main", &["feat-a"]) + .gh_stack_merged_branch(None, 1, "feat-a") + .gh_stack_ghost_branch(None, 1, "feat-b") + .gh_stack_merged_branch(None, 1, "feat-b") + .build()?; + + let main_path = fixture.root()?.join("main"); + let output = cargo_bin_cmd!("git-workon") + .current_dir(&main_path) + .env("NO_COLOR", "1") + .arg("list") + .output()?; + + assert!(output.status.success()); + let stdout = std::str::from_utf8(&output.stdout)?; + + let feat_a_line = stdout + .lines() + .find(|l| l.contains("feat-a")) + .unwrap_or_else(|| panic!("no feat-a row in: {stdout}")); + assert!( + feat_a_line.contains("merged"), + "feat-a has a worktree, so it must render with a merged marker: {feat_a_line}" + ); + assert!( + !stdout.contains("feat-b"), + "feat-b is merged with no worktree, so it must not render at all: {stdout}" + ); + + Ok(()) +} From 0e169483558557933aa5fea5bbe7d0b31d2edc76 Mon Sep 17 00:00:00 2001 From: Eric Eldredge Date: Mon, 14 Sep 2026 17:35:25 -0400 Subject: [PATCH 4/4] docs: record merged-state handling in ADR-028 --- docs/adr/028-provider-neutral-stack-metadata.md | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/docs/adr/028-provider-neutral-stack-metadata.md b/docs/adr/028-provider-neutral-stack-metadata.md index bc6af286..d2d17c3b 100644 --- a/docs/adr/028-provider-neutral-stack-metadata.md +++ b/docs/adr/028-provider-neutral-stack-metadata.md @@ -251,6 +251,20 @@ sees the same state regardless of which worktree wrote it. as a dependency but no `serde` derive crate, and the write path in particular needs the raw `Value` round-trip to preserve `id` and `pullRequest` on stack entries workon itself never touches. +- **Merged-branch awareness.** `gh stack sync` leaves a merged branch's metadata row behind rather + than deleting it (its help text says the row is kept for rebase and display logic), and its + `--prune` cannot `git branch -D` a branch that a worktree has checked out, which is the normal + git-workon case. To reflect this, `gh_stack::read_metadata` reads `pullRequest.merged` into + `BranchMetadata.merged`, and `Stack` gains a `merged: HashSet` field. `enumerate` drops + merged rows the way it drops ghosts, but first reparents each surviving branch past every merged + ancestor: sync merges a stack bottom-up, so without that step a merged bottom branch would take + the whole remaining stack down with it. `current` retains merged rows, same reasoning as ghost + retention. The CLI tree builder (`build_children` in `display.rs`) drops a merged branch that + has no worktree and splices its live children onto its parent, and renders a dim ` merged` + suffix on a merged branch that does have one. `list --json` gains an additive per-stack + `"merged"` array. **Graphite gap:** neither `.graphite_metadata.db` (its `state` column is + empty in practice) nor the `refs/branch-metadata/*` blobs carry PR merged state, so + `BranchMetadata.merged` is hardcoded `false` for Graphite. ## References