From b06fbdb30d2ae5318023a37a8f289f410e0af884 Mon Sep 17 00:00:00 2001 From: Eric Eldredge Date: Thu, 10 Sep 2026 12:15:48 -0400 Subject: [PATCH 1/4] feat(lib)!: add StackModel::Mixed with per-branch provider fallback --- git-workon-lib/src/changeset.rs | 25 ++- git-workon-lib/src/config.rs | 29 +++- git-workon-lib/src/lib.rs | 5 +- git-workon-lib/src/stack.rs | 222 +++++++++++++++++++++--- git-workon-lib/src/stack/gh_stack.rs | 18 ++ git-workon-lib/src/stack/graphite.rs | 25 +++ git-workon-lib/tests/suite/changeset.rs | 25 +++ git-workon-lib/tests/suite/config.rs | 85 ++++++++- git-workon-lib/tests/suite/stack.rs | 81 ++++++++- git-workon/src/cmd/doctor.rs | 7 + 10 files changed, 488 insertions(+), 34 deletions(-) diff --git a/git-workon-lib/src/changeset.rs b/git-workon-lib/src/changeset.rs index b7d5158c..46c49779 100644 --- a/git-workon-lib/src/changeset.rs +++ b/git-workon-lib/src/changeset.rs @@ -17,6 +17,10 @@ //! still walked through, so live descendants of a ghost still appear. Falls back to the //! `Git` arm when `head_branch` is a trunk branch or has no metadata row at all (mirrors //! the nvim prototype's factory behavior). +//! - [`StackModel::Mixed`] → reads both providers' metadata and assembles from whichever one's +//! `parents` contains `head_branch`, checked in [`StackModel::providers`] order (primary +//! first); falls to the primary's metadata when neither tracks it, which then falls to `Git` +//! through the same trunk/untracked check as the single-provider arms. //! - [`StackModel::Git`] → no metadata; walks `upstream..head_branch` commit-by-commit //! (oldest first), one [`Changeset`] per commit. //! @@ -30,7 +34,7 @@ use git2::{BranchType, Oid, Repository, StatusOptions}; use crate::error::{ChangesetError, Result}; use crate::stack::metadata::{self, StackMetadata}; -use crate::stack::{gh_stack, graphite, StackModel}; +use crate::stack::{gh_stack, graphite, StackModel, StackProvider}; /// What a [`Changeset`] spans: a resolved commit range, or the working tree + index. #[derive(Debug, Clone, Copy, PartialEq, Eq)] @@ -82,6 +86,25 @@ pub fn assemble_changesets( StackModel::GhStack => { assemble_from_metadata(repo, head_branch, &gh_stack::read_metadata(repo)?) } + StackModel::Mixed { primary } => { + // Read both providers' metadata and assemble from whichever one actually tracks + // `head_branch` — checked in provider order, primary first. Neither tracking it + // falls to the primary's metadata, which `assemble_from_metadata` itself routes to + // `assemble_git` (head_branch is a trunk or has no metadata row either way). + let (graphite_meta, gh_stack_meta) = ( + graphite::read_metadata(repo)?, + gh_stack::read_metadata(repo)?, + ); + let meta_for = |provider: StackProvider| match provider { + StackProvider::Graphite => &graphite_meta, + StackProvider::GhStack => &gh_stack_meta, + }; + let owner = model + .providers() + .find(|&p| meta_for(p).parents.contains_key(head_branch)) + .unwrap_or(primary); + assemble_from_metadata(repo, head_branch, meta_for(owner)) + } } } diff --git a/git-workon-lib/src/config.rs b/git-workon-lib/src/config.rs index 1ef0023d..92c85c5b 100644 --- a/git-workon-lib/src/config.rs +++ b/git-workon-lib/src/config.rs @@ -29,7 +29,8 @@ //! - **workon.pruneProtectedBranches** - Branches protected from pruning (multi-value, default: []) //! - **workon.pruneGone** - Treat gone-upstream worktrees as prune candidates by default (bool, default: false) //! - **workon.pruneFetch** - Fetch from tracked remotes before evaluating gone status (bool, default: false) -//! - **workon.stackModel** - Active stack model: "auto", "graphite", "git", or "none" (string, default: "auto") +//! - **workon.stackModel** - Active stack model: "auto", "graphite", "gh-stack", "git", "none", +//! "mixed:graphite", or "mixed:gh-stack" (string, default: "auto") //! - **workon.stackWorktreeGranularity** - Worktree granularity for stacked diffs: "stack" (string, default: "stack") //! - **workon.stackAutoTrack** - Auto-register new branches with the active stack tool after //! `workon new` (bool, default: true) @@ -62,7 +63,7 @@ use std::time::Duration; use git2::Repository; use crate::error::{ConfigError, Result, StackError}; -use crate::stack::{Granularity, StackModel}; +use crate::stack::{Granularity, StackModel, StackProvider}; /// Configuration reader for workon settings stored in git config. /// @@ -318,15 +319,19 @@ impl<'repo> WorkonConfig<'repo> { /// /// Auto-detection: returns `Graphite` when the repo has been `gt init`-ed /// (`.graphite_repo_config` or `.graphite_metadata.db` exists), else `GhStack` when a - /// gh-stack file is present, else `None`. Graphite wins when both are present — see - /// [`StackModel::detect`]. + /// gh-stack file is present, else `None`. When both tools' artifacts are present, ties + /// break on which has ref-backed tracked branches: both live → `Mixed` (gh-stack primary, + /// Graphite fallback); only one live → that provider, strict; neither live → `GhStack` — + /// see [`StackModel::detect`] for the full table. /// /// Accepted config values: `"graphite"`, `"gh-stack"`, `"git"`, `"none"`, `"auto"` - /// (re-runs detection). `"git"` opts into metadata-less git-inference - /// ([`StackModel::Git`]) explicitly — it is never the result of `"auto"`. `"ghstack"` - /// (no hyphen) is a *different* tool (Meta's Phabricator-style stacker) and is rejected - /// as unsupported rather than treated as a typo for `"gh-stack"`. Anything else returns - /// an error. + /// (re-runs detection), `"mixed:graphite"`, `"mixed:gh-stack"` (explicit `Mixed` pin, + /// naming the primary directly, skipping the liveness read). `"git"` opts into + /// metadata-less git-inference ([`StackModel::Git`]) explicitly — it is never the result + /// of `"auto"`. `"ghstack"` (no hyphen) is a *different* tool (Meta's Phabricator-style + /// stacker) and is rejected as unsupported rather than treated as a typo for `"gh-stack"`. + /// Bare `"mixed"` and any other `"mixed:"` fall through to `UnknownModel`, same as any + /// other unrecognized value. pub fn stack_model(&self, cli_override: Option<&str>) -> Result { let raw = if let Some(val) = cli_override { Some(val.to_string()) @@ -341,6 +346,12 @@ impl<'repo> WorkonConfig<'repo> { Some("graphite") => Ok(StackModel::Graphite), Some("gh-stack") => Ok(StackModel::GhStack), Some("git") => Ok(StackModel::Git), + Some("mixed:graphite") => Ok(StackModel::Mixed { + primary: StackProvider::Graphite, + }), + Some("mixed:gh-stack") => Ok(StackModel::Mixed { + primary: StackProvider::GhStack, + }), Some(other) if matches!(other, "branchless" | "sapling" | "spr" | "ghstack") => { Err(StackError::UnsupportedModel { model: other.to_string(), diff --git a/git-workon-lib/src/lib.rs b/git-workon-lib/src/lib.rs index c3fc426e..49495f97 100644 --- a/git-workon-lib/src/lib.rs +++ b/git-workon-lib/src/lib.rs @@ -89,8 +89,9 @@ pub use crate::resolve::*; pub use crate::stack::{ current_stack, enumerate_stacks, gh_stack_divergent_stack_numbers, gh_stack_readability_errors, gh_stack_worktree_link_status, graphite_trunk, group_by_stack, is_gh_stack_repo, - is_graphite_active, is_graphite_repo, link_worktree, migrate_worktree, register_branch, - GhStackLinkStatus, Granularity, Stack, StackGroup, StackGrouping, StackModel, + is_graphite_active, is_graphite_repo, link_worktree, migrate_worktree, + provider_has_live_branches, register_branch, GhStackLinkStatus, Granularity, Stack, StackGroup, + StackGrouping, StackModel, StackProvider, }; pub use crate::stash::*; pub use crate::workon_root::*; diff --git a/git-workon-lib/src/stack.rs b/git-workon-lib/src/stack.rs index bdfc0e8d..ba48cc6e 100644 --- a/git-workon-lib/src/stack.rs +++ b/git-workon-lib/src/stack.rs @@ -7,8 +7,9 @@ //! //! Stack awareness is two-dimensional: //! -//! - [`StackModel`] — which tool manages stacks (v1: Graphite and `gh stack`, plus -//! metadata-less git-inference via [`StackModel::Git`]; future: branchless, sapling) +//! - [`StackModel`] — which tool manages stacks (v1: Graphite and `gh stack`, [`StackModel::Mixed`] +//! when both have live branches, plus metadata-less git-inference via [`StackModel::Git`]; +//! future: branchless, sapling) //! - [`Granularity`] — how worktrees map to stacks (v1: [`Granularity::Stack`], one per stack) //! //! ## Default-on behavior @@ -58,6 +59,16 @@ use git2::Repository; use crate::error::Result; +/// A concrete stacked-diff tool. [`StackModel::Mixed`] names one as primary, the other as +/// fallback. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum StackProvider { + /// Graphite (`gt`), read via `stack/graphite.rs`. + Graphite, + /// `gh stack`, read via `stack/gh_stack.rs`. + GhStack, +} + /// Which stacked-diff tool is managing stacks in this repository. #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum StackModel { @@ -68,6 +79,13 @@ pub enum StackModel { /// `gh stack` (the `github/gh-stack` extension) manages stacks via a JSON file. See /// `stack/gh_stack.rs`'s module docs for the canonical-file-plus-symlinks model. GhStack, + /// Both Graphite and gh-stack have ref-backed tracked branches in this repository. + /// `primary` is consulted first by [`current_stack`], [`enumerate_stacks`], and + /// [`crate::assemble_changesets`]; the other provider answers only when `primary` has no + /// row for the branch in question. Only reachable via `"auto"` (or unset) config — an + /// explicit `workon.stackModel = graphite`/`gh-stack` stays strict, never falling back to + /// the other provider even when it also has live branches. See [`StackModel::detect`]. + Mixed { primary: StackProvider }, /// No stack-metadata tool; changesets are inferred purely from git, one per commit in /// `upstream..HEAD`. Unlike [`StackModel::Graphite`]/[`StackModel::GhStack`], this carries /// no branch-level stack topology: [`enumerate_stacks`] and [`current_stack`] treat it as @@ -101,21 +119,65 @@ impl StackModel { /// explicit `workon.stackModel = git` config, or a caller mapping `None` to `Git` before /// calling [`crate::assemble_changesets`] (the review crate does this from M3 onward). /// - /// **Graphite wins** when both tools' artifacts are present: `.graphite_repo_config` - /// comes from an explicit, repo-wide `gt init`, while a `gh-stack` file can appear as a - /// side effect of one `gh stack add` run in one worktree. The more deliberate, - /// repo-scoped signal wins, so no repo that resolves to `Graphite` today can silently flip - /// to `GhStack` just because someone tried the other tool once. The escape hatch is an - /// explicit `workon.stackModel = gh-stack`. + /// **When both tools' artifacts are present**, detection tie-breaks on *live* metadata — + /// whether each provider has at least one ref-backed tracked branch (see + /// [`graphite::has_live_branches`]/[`gh_stack::has_live_branches`]) — rather than always + /// preferring one provider: + /// + /// - Both live → [`StackModel::Mixed`] with gh-stack as primary: GitHub's native stack + /// support is the more deliberate signal now that it exists, so it answers first, but + /// neither provider's tracked branches are hidden. + /// - Only one live → that provider, strict (not `Mixed`): the other's artifacts are stale + /// (e.g. a lingering `.graphite_repo_config` from a repo that migrated to gh-stack), so + /// there is nothing to fall back to. + /// - Neither live → `GhStack`: an ambiguous, doubly-stale state that `doctor`'s + /// `BothStackToolsDetected` check explains rather than detection resolving further. + /// + /// A single artifact present (no ambiguity) skips the liveness read entirely and returns + /// that provider, matching today's behavior. The escape hatch out of `Mixed` — or out of a + /// tie-break you disagree with — is an explicit `workon.stackModel = graphite`/`gh-stack`, + /// which is strict and never reachable via `"auto"`. pub fn detect(repo: &Repository) -> Self { - if graphite::is_graphite_repo(repo) { - Self::Graphite - } else if gh_stack::is_gh_stack_repo(repo) { - Self::GhStack - } else { - Self::None + let graphite_artifacts = graphite::is_graphite_repo(repo); + let gh_artifacts = gh_stack::is_gh_stack_repo(repo); + + match (graphite_artifacts, gh_artifacts) { + (false, false) => Self::None, + (true, false) => Self::Graphite, + (false, true) => Self::GhStack, + (true, true) => { + let graphite_live = graphite::has_live_branches(repo); + let gh_stack_live = gh_stack::has_live_branches(repo); + match (graphite_live, gh_stack_live) { + (true, true) => Self::Mixed { + primary: StackProvider::GhStack, + }, + (true, false) => Self::Graphite, + (false, true) => Self::GhStack, + (false, false) => Self::GhStack, + } + } } } + + /// Providers to consult, in fallback order. `Graphite`/`GhStack` consult only themselves + /// (strict, no fallback); `Mixed` consults `primary` first, then the other; `None`/`Git` + /// have no provider to consult. + pub fn providers(self) -> impl Iterator { + let (first, second) = match self { + Self::Graphite => (Some(StackProvider::Graphite), None), + Self::GhStack => (Some(StackProvider::GhStack), None), + Self::Mixed { primary } => { + let other = match primary { + StackProvider::Graphite => StackProvider::GhStack, + StackProvider::GhStack => StackProvider::Graphite, + }; + (Some(primary), Some(other)) + } + Self::None | Self::Git => (None, None), + }; + first.into_iter().chain(second) + } } /// How worktrees map to stacks. @@ -153,18 +215,58 @@ pub struct Stack { pub merged: HashSet, } +/// Dispatch [`enumerate_stacks`] to one concrete provider's implementation. +fn provider_enumerate_stacks(repo: &Repository, provider: StackProvider) -> Result> { + match provider { + StackProvider::Graphite => graphite::enumerate_stacks(repo).map_err(Into::into), + StackProvider::GhStack => gh_stack::enumerate_stacks(repo).map_err(Into::into), + } +} + +/// Dispatch [`current_stack`] to one concrete provider's implementation. +fn provider_current_stack( + repo: &Repository, + head_branch: &str, + provider: StackProvider, +) -> Result> { + match provider { + StackProvider::Graphite => graphite::current_stack(repo, head_branch).map_err(Into::into), + StackProvider::GhStack => gh_stack::current_stack(repo, head_branch).map_err(Into::into), + } +} + /// Return all stacks present in metadata, one per connected component. /// /// Each returned [`Stack`] corresponds to one potential [`StackGroup`] — the same `(trunk, /// sorted diffs)` key used by [`group_by_stack`]. Used by the `list` command to surface /// stacks that have no checked-out worktrees. +/// +/// Under [`StackModel::Mixed`], returns the primary provider's stacks followed by the +/// secondary's, excluding any secondary stack that shares a branch with a primary stack — a +/// branch belongs to one place in the tree, and the primary's placement always wins (the CLI's +/// `display::build_tree` merges stacks per trunk with a `visited`-based dedupe that would +/// otherwise drop the shared branch non-deterministically). pub fn enumerate_stacks(repo: &Repository, model: StackModel) -> Result> { match model { StackModel::None => Ok(vec![]), - StackModel::Graphite => graphite::enumerate_stacks(repo).map_err(Into::into), - StackModel::GhStack => gh_stack::enumerate_stacks(repo).map_err(Into::into), // Git-inference has no branch-level stack topology to enumerate — flat, like None. StackModel::Git => Ok(vec![]), + StackModel::Graphite => provider_enumerate_stacks(repo, StackProvider::Graphite), + StackModel::GhStack => provider_enumerate_stacks(repo, StackProvider::GhStack), + StackModel::Mixed { .. } => { + let mut providers = model.providers(); + let primary = providers.next().expect("Mixed always has a primary"); + let secondary = providers.next().expect("Mixed always has a secondary"); + let primary_stacks = provider_enumerate_stacks(repo, primary)?; + let claimed: std::collections::HashSet = primary_stacks + .iter() + .flat_map(|s| s.diffs.iter().cloned()) + .collect(); + let secondary_stacks = provider_enumerate_stacks(repo, secondary)? + .into_iter() + .filter(|s| !s.diffs.iter().any(|b| claimed.contains(b))); + Ok(primary_stacks.into_iter().chain(secondary_stacks).collect()) + } } } @@ -173,6 +275,11 @@ pub fn enumerate_stacks(repo: &Repository, model: StackModel) -> Result Result> { match model { StackModel::None => Ok(None), - StackModel::Graphite => graphite::current_stack(repo, head_branch).map_err(Into::into), - StackModel::GhStack => gh_stack::current_stack(repo, head_branch).map_err(Into::into), // Git-inference has no branch-level stack topology — flat, like None. StackModel::Git => Ok(None), + _ => { + for provider in model.providers() { + if let Some(stack) = provider_current_stack(repo, head_branch, provider)? { + return Ok(Some(stack)); + } + } + Ok(None) + } } } @@ -241,6 +354,18 @@ pub fn is_gh_stack_repo(repo: &Repository) -> bool { gh_stack::is_gh_stack_repo(repo) } +/// `true` if `provider` has at least one ref-backed tracked branch in this repository — see +/// [`graphite::has_live_branches`]/[`gh_stack::has_live_branches`] for the definition. Used by +/// [`StackModel::detect`]'s tie-break, and by `list`'s pin hint and `doctor`'s +/// `StackModelPinHidesLiveBranches` check to tell whether an explicit `workon.stackModel` pin +/// is hiding the other provider's tracked branches. +pub fn provider_has_live_branches(repo: &Repository, provider: StackProvider) -> bool { + match provider { + StackProvider::Graphite => graphite::has_live_branches(repo), + StackProvider::GhStack => gh_stack::has_live_branches(repo), + } +} + /// Per-worktree gh-stack link status, for `doctor`'s `GhStackWorktreeNotLinked` check. pub use gh_stack::LinkStatus as GhStackLinkStatus; @@ -384,7 +509,7 @@ mod tests { } #[test] - fn detect_prefers_graphite_when_both_tools_artifacts_are_present() { + fn detect_resolves_mixed_when_both_tools_have_live_branches() { let fixture = FixtureBuilder::new() .graphite_config(&["main"]) .branch_metadata("a", "main") @@ -393,10 +518,67 @@ mod tests { .unwrap(); let repo = fixture.repo().unwrap(); + assert_eq!( + StackModel::detect(repo), + StackModel::Mixed { + primary: StackProvider::GhStack + }, + "both tools have a ref-backed tracked branch, so neither is hidden" + ); + } + + #[test] + fn detect_resolves_gh_stack_when_only_gh_stack_has_live_branches() { + // Graphite's only metadata row is a ghost (branch ref deleted) — Graphite's artifacts + // are stale, gh-stack's are not. + let fixture = FixtureBuilder::new() + .graphite_config(&["main"]) + .ghost_branch_metadata("a", "main") + .gh_stack(None, 1, "main", &["feat-a"]) + .build() + .unwrap(); + let repo = fixture.repo().unwrap(); + + assert_eq!( + StackModel::detect(repo), + StackModel::GhStack, + "Graphite's only row is a ghost; gh-stack's tracked branch is live" + ); + } + + #[test] + fn detect_resolves_graphite_when_gh_stack_has_only_ghost_branches() { + let fixture = FixtureBuilder::new() + .graphite_config(&["main"]) + .branch_metadata("a", "main") + .gh_stack(None, 1, "main", &[]) + .gh_stack_ghost_branch(None, 1, "feat-a") + .build() + .unwrap(); + let repo = fixture.repo().unwrap(); + assert_eq!( StackModel::detect(repo), StackModel::Graphite, - "Graphite's repo-wide gt init must win over a gh-stack file appearing alongside it" + "gh-stack's only row is a ghost; Graphite's tracked branch is live" + ); + } + + #[test] + fn detect_resolves_graphite_when_both_tools_have_only_ghost_branches() { + let fixture = FixtureBuilder::new() + .graphite_config(&["main"]) + .ghost_branch_metadata("a", "main") + .gh_stack(None, 1, "main", &[]) + .gh_stack_ghost_branch(None, 1, "feat-a") + .build() + .unwrap(); + let repo = fixture.repo().unwrap(); + + assert_eq!( + StackModel::detect(repo), + StackModel::GhStack, + "neither tool has a live branch; gh-stack is the default, doctor explains the ambiguity" ); } diff --git a/git-workon-lib/src/stack/gh_stack.rs b/git-workon-lib/src/stack/gh_stack.rs index 883d5c60..50e972a7 100644 --- a/git-workon-lib/src/stack/gh_stack.rs +++ b/git-workon-lib/src/stack/gh_stack.rs @@ -347,6 +347,24 @@ pub(crate) fn read_metadata(repo: &Repository) -> Result bool { + match read_metadata(repo) { + Ok(meta) => meta + .parents + .keys() + .any(|b| crate::resolve::branch_exists(repo, b)), + Err(e) => { + log::debug!("gh-stack: has_live_branches: read_metadata failed: {e}"); + false + } + } +} + /// Return all gh-stack stacks, one per connected component, ghost branches PRUNED. pub(crate) fn enumerate_stacks(repo: &Repository) -> Result, StackError> { Ok(metadata::enumerate(repo, &read_metadata(repo)?)) diff --git a/git-workon-lib/src/stack/graphite.rs b/git-workon-lib/src/stack/graphite.rs index aa0cc10a..39600b71 100644 --- a/git-workon-lib/src/stack/graphite.rs +++ b/git-workon-lib/src/stack/graphite.rs @@ -63,6 +63,31 @@ pub fn is_graphite_repo(repo: &Repository) -> bool { git_dir.join(".graphite_metadata.db").exists() || git_dir.join(".graphite_repo_config").exists() } +/// `true` if Graphite metadata has at least one non-trunk branch whose ref still resolves. +/// +/// Used by [`StackModel::detect`](super::StackModel::detect)'s live-metadata tie-break: a repo +/// that migrated off Graphite keeps `.graphite_repo_config`/`.graphite_metadata.db` behind, and +/// their metadata rows for deleted branches must NOT count as "live" — that ghost state is +/// exactly the case that has to lose the tie-break. Checks `parents`' keys (non-trunk branches), +/// never `trunks`' values, per [`StackMetadata`]'s own doc: trunks are not metadata rows. +/// +/// A `read_metadata` error (corrupt store) yields `false` here, logged via `log::debug!` — never +/// probes the `gt` binary, and never surfaces as an error itself. Detection stays infallible; +/// the corrupt store is surfaced later by the provider call that actually needs the data, and by +/// `doctor`. +pub(crate) fn has_live_branches(repo: &Repository) -> bool { + match read_metadata(repo) { + Ok(meta) => meta + .parents + .keys() + .any(|b| crate::resolve::branch_exists(repo, b)), + Err(e) => { + log::debug!("graphite: has_live_branches: read_metadata failed: {e}"); + false + } + } +} + /// Return the first trunk branch name from `.graphite_repo_config`, or `None` if the /// file is missing, unparseable, or contains no trunk entries. /// diff --git a/git-workon-lib/tests/suite/changeset.rs b/git-workon-lib/tests/suite/changeset.rs index aec2ebd7..52a10c40 100644 --- a/git-workon-lib/tests/suite/changeset.rs +++ b/git-workon-lib/tests/suite/changeset.rs @@ -683,3 +683,28 @@ fn gh_stack_needs_restack_true_when_base_differs_from_parent_live_tip() -> Resul ); Ok(()) } + +// ── StackModel::Mixed ──────────────────────────────────────────────────────────── + +#[test] +fn mixed_head_tracked_only_by_gh_stack_assembles_from_gh_stack_metadata( +) -> Result<(), Box> { + let fixture = FixtureBuilder::new() + .graphite_config(&["main"]) + .branch_metadata("graphite-only", "main") + .gh_stack(None, 1, "main", &["a", "b", "c"]) + .build()?; + let repo = fixture.repo()?; + + let model = StackModel::Mixed { + primary: workon::StackProvider::Graphite, + }; + + // "b" has no Graphite row at all — assembly must fall to gh-stack's metadata rather than + // reporting it untracked. + let changesets = assemble_changesets(repo, "b", model)?; + let names: Vec<&str> = changesets.iter().map(|c| c.name.as_str()).collect(); + assert_eq!(names, vec!["a", "b", "c"]); + + Ok(()) +} diff --git a/git-workon-lib/tests/suite/config.rs b/git-workon-lib/tests/suite/config.rs index 1e5bdc13..3fb296b1 100644 --- a/git-workon-lib/tests/suite/config.rs +++ b/git-workon-lib/tests/suite/config.rs @@ -1,6 +1,6 @@ use git_workon_fixture::prelude::*; use std::error::Error; -use workon::{Granularity, StackModel, WorkonConfig}; +use workon::{Granularity, StackModel, StackProvider, WorkonConfig}; #[test] fn read_default_branch_config() -> Result<(), Box> { @@ -245,6 +245,89 @@ fn stack_model_gh_stack_returns_gh_stack_variant() -> Result<(), Box> Ok(()) } +#[test] +fn stack_model_mixed_graphite_returns_mixed_variant_with_graphite_primary( +) -> Result<(), Box> { + let fixture = FixtureBuilder::new() + .config("workon.stackModel", "mixed:graphite") + .build()?; + let repo = fixture.repo()?; + let cfg = WorkonConfig::new(repo)?; + assert_eq!( + cfg.stack_model(None)?, + StackModel::Mixed { + primary: StackProvider::Graphite + } + ); + Ok(()) +} + +#[test] +fn stack_model_mixed_gh_stack_returns_mixed_variant_with_gh_stack_primary( +) -> Result<(), Box> { + let fixture = FixtureBuilder::new() + .config("workon.stackModel", "mixed:gh-stack") + .build()?; + let repo = fixture.repo()?; + let cfg = WorkonConfig::new(repo)?; + assert_eq!( + cfg.stack_model(None)?, + StackModel::Mixed { + primary: StackProvider::GhStack + } + ); + Ok(()) +} + +#[test] +fn stack_model_mixed_cli_override_wins_over_config() -> Result<(), Box> { + let fixture = FixtureBuilder::new() + .config("workon.stackModel", "graphite") + .build()?; + let repo = fixture.repo()?; + let cfg = WorkonConfig::new(repo)?; + assert_eq!( + cfg.stack_model(Some("mixed:gh-stack"))?, + StackModel::Mixed { + primary: StackProvider::GhStack + } + ); + Ok(()) +} + +#[test] +fn stack_model_bare_mixed_is_rejected() -> Result<(), Box> { + let fixture = FixtureBuilder::new() + .config("workon.stackModel", "mixed") + .build()?; + let repo = fixture.repo()?; + let cfg = WorkonConfig::new(repo)?; + let err = cfg.stack_model(None).unwrap_err(); + assert!( + err.to_string().contains("Unknown stack model"), + "expected 'Unknown stack model' for bare 'mixed', got: {err}" + ); + Ok(()) +} + +#[test] +fn stack_model_mixed_unknown_provider_is_rejected() -> Result<(), Box> { + // "mixed:ghstack" (no hyphen) must not slip through as a typo fix for "mixed:gh-stack". + for bad in &["mixed:foo", "mixed:ghstack"] { + let fixture = FixtureBuilder::new() + .config("workon.stackModel", bad) + .build()?; + let repo = fixture.repo()?; + let cfg = WorkonConfig::new(repo)?; + let err = cfg.stack_model(None).unwrap_err(); + assert!( + err.to_string().contains("Unknown stack model"), + "expected 'Unknown stack model' for '{bad}', got: {err}" + ); + } + Ok(()) +} + #[test] fn stack_model_bare_ghstack_is_rejected_as_a_different_tool() -> Result<(), Box> { // "ghstack" (no hyphen) is Meta's Phabricator-style stacker, a different tool from diff --git a/git-workon-lib/tests/suite/stack.rs b/git-workon-lib/tests/suite/stack.rs index 795ef84c..a3685b2f 100644 --- a/git-workon-lib/tests/suite/stack.rs +++ b/git-workon-lib/tests/suite/stack.rs @@ -1,6 +1,6 @@ use git_workon_fixture::prelude::*; use std::error::Error; -use workon::{current_stack, enumerate_stacks, graphite_trunk, StackModel}; +use workon::{current_stack, enumerate_stacks, graphite_trunk, StackModel, StackProvider}; // ── both-format parameterization ────────────────────────────────────────────── // @@ -591,6 +591,85 @@ fn current_stack_returns_only_the_member_stack_when_trunk_is_shared() -> Result< Ok(()) } +// ── StackModel::Mixed — per-branch fallback to the secondary provider ──────── + +#[test] +fn current_stack_under_mixed_falls_back_to_secondary_provider_per_branch( +) -> Result<(), Box> { + let fixture = FixtureBuilder::new() + .graphite_config(&["main"]) + .branch_metadata("graphite-only", "main") + .gh_stack(None, 1, "main", &["gh-stack-only"]) + .branch("untracked") + .build()?; + let repo = fixture.repo()?; + + let model = StackModel::Mixed { + primary: StackProvider::Graphite, + }; + + let graphite_stack = current_stack(repo, "graphite-only", model)?.unwrap(); + assert_eq!(graphite_stack.diffs, vec!["graphite-only"]); + + let gh_stack_stack = current_stack(repo, "gh-stack-only", model)?.unwrap(); + assert_eq!(gh_stack_stack.diffs, vec!["gh-stack-only"]); + + assert!(current_stack(repo, "untracked", model)?.is_none()); + + Ok(()) +} + +#[test] +fn current_stack_under_explicit_gh_stack_config_ignores_a_graphite_tracked_branch( +) -> Result<(), Box> { + // Explicit config stays strict — even though Graphite has a live tracked branch, the + // explicit `GhStack` model must not fall back to it. + let fixture = FixtureBuilder::new() + .graphite_config(&["main"]) + .branch_metadata("graphite-only", "main") + .build()?; + let repo = fixture.repo()?; + + assert!(current_stack(repo, "graphite-only", StackModel::GhStack)?.is_none()); + + Ok(()) +} + +#[test] +fn enumerate_stacks_under_mixed_returns_both_providers_deduping_shared_branches( +) -> Result<(), Box> { + let fixture = FixtureBuilder::new() + .graphite_config(&["main"]) + .branch_metadata("shared", "main") + .gh_stack(None, 1, "main", &["shared"]) + .gh_stack(None, 2, "main", &["gh-stack-only"]) + .build()?; + let repo = fixture.repo()?; + + let model = StackModel::Mixed { + primary: StackProvider::Graphite, + }; + let mut stacks = enumerate_stacks(repo, model)?; + stacks.sort_by(|a, b| a.diffs[0].cmp(&b.diffs[0])); + + // "shared" is tracked by both providers; it must appear exactly once, under the primary + // (Graphite)'s placement, not duplicated by gh-stack's stack #1. + let shared_occurrences: usize = stacks + .iter() + .filter(|s| s.diffs.iter().any(|b| b == "shared")) + .count(); + assert_eq!(shared_occurrences, 1, "shared branch must appear once"); + + let all_diffs: Vec<&str> = stacks + .iter() + .flat_map(|s| s.diffs.iter().map(String::as_str)) + .collect(); + assert!(all_diffs.contains(&"shared")); + assert!(all_diffs.contains(&"gh-stack-only")); + + Ok(()) +} + // ── fixture predicates ──────────────────────────────────────────────────────── #[test] diff --git a/git-workon/src/cmd/doctor.rs b/git-workon/src/cmd/doctor.rs index ad5e99d3..79aa9f55 100644 --- a/git-workon/src/cmd/doctor.rs +++ b/git-workon/src/cmd/doctor.rs @@ -900,6 +900,13 @@ fn read_config_entries( "git".to_string(), scalar_source(repo, &git_config, "workon.stackModel"), ), + Ok(StackModel::Mixed { primary }) => ( + match primary { + workon::StackProvider::Graphite => "mixed (graphite primary)".to_string(), + workon::StackProvider::GhStack => "mixed (gh-stack primary)".to_string(), + }, + scalar_source(repo, &git_config, "workon.stackModel"), + ), Err(_) => ( "(invalid)".to_string(), scalar_source(repo, &git_config, "workon.stackModel"), From 01c9769d338025848130ae6642281c8ab12bde43 Mon Sep 17 00:00:00 2001 From: Eric Eldredge Date: Thu, 10 Sep 2026 12:16:21 -0400 Subject: [PATCH 2/4] feat(cli): register forks with the parent branch's stack provider --- git-workon/src/cmd/new.rs | 219 +++++++++++++++++++++++++--------- git-workon/tests/suite/new.rs | 141 ++++++++++++++++++++++ 2 files changed, 301 insertions(+), 59 deletions(-) diff --git a/git-workon/src/cmd/new.rs b/git-workon/src/cmd/new.rs index 72d9a9f2..3e562c3b 100644 --- a/git-workon/src/cmd/new.rs +++ b/git-workon/src/cmd/new.rs @@ -58,7 +58,8 @@ use crate::output; use workon::{ add_worktree, copy_untracked, current_stack, current_worktree, get_repo, get_worktrees, graphite_trunk, link_worktree, register_branch, resolve_remote_tracking, workon_root, - BranchType, CopyOptions, RemoteResolution, StackModel, WorkonConfig, WorktreeDescriptor, + BranchType, CopyOptions, RemoteResolution, StackModel, StackProvider, WorkonConfig, + WorktreeDescriptor, }; use super::Run; @@ -323,71 +324,66 @@ impl Run for New { // Register the new branch with the active stack tool (non-fatal on failure — `new` // has already created the worktree). match effective_model { - StackModel::Graphite => { - // Deliberately NOT guarded on `detect_gt()`: `StackModel::detect` resolves - // `Graphite` from repo metadata alone, so this can run on a machine without - // `gt`. The resulting "gt track unavailable" warning is the point — the new - // branch really is untracked, and silence would hide that until the stack - // looked wrong later. See `new_gt_track_failure_is_non_fatal`. - if !self.no_stack && !branch_pre_existed && config.gt_auto_track(None)? { - // Prefer the explicit base branch, then the repo's graphite trunk. - // If neither is known, omit --parent so gt infers from its own config. - let parent = base_branch - .as_deref() - .map(String::from) - .or_else(|| graphite_trunk(&repo)); - debug!( - "Running: gt track{} in {}", - parent - .as_deref() - .map(|p| format!(" --parent {p}")) - .unwrap_or_default(), - worktree.path().display() - ); - let mut cmd = std::process::Command::new("gt"); - cmd.arg("track"); - if let Some(p) = &parent { - cmd.arg("--parent").arg(p); - } - match cmd.current_dir(worktree.path()).output() { - Ok(out) if out.status.success() => { - debug!("gt track succeeded"); - } - Ok(out) => { - let stderr = String::from_utf8_lossy(&out.stderr); - output::warn(&format!("gt track failed: {}", stderr.trim())); - } - Err(e) => { - output::warn(&format!("gt track unavailable: {}", e)); - } - } + StackModel::Graphite => register_graphite( + &repo, + &worktree, + &config, + base_branch.as_deref(), + branch_pre_existed, + self.no_stack, + )?, + StackModel::GhStack => register_gh_stack( + &repo, + &worktree, + &config, + &effective_branch, + base_branch.as_deref(), + branch_pre_existed, + self.no_stack, + )?, + StackModel::Mixed { primary } => { + // Follow the parent's provider: the fork belongs wherever `base_branch` is + // already tracked, checked in `providers()` order (primary first). No base, or + // neither provider knows it, falls to `primary` — matching a plain `Graphite`/ + // `GhStack` model's own "untracked base" behavior (Graphite tracks with no + // `--parent`; gh-stack skips with a debug log, both unconditionally). + let owner = base_branch + .as_deref() + .and_then(|base| { + effective_model + .providers() + .find(|&p| provider_tracks(&repo, base, p)) + }) + .unwrap_or(primary); + match owner { + StackProvider::Graphite => register_graphite( + &repo, + &worktree, + &config, + base_branch.as_deref(), + branch_pre_existed, + self.no_stack, + )?, + StackProvider::GhStack => register_gh_stack( + &repo, + &worktree, + &config, + &effective_branch, + base_branch.as_deref(), + branch_pre_existed, + self.no_stack, + )?, } - } - StackModel::GhStack => { - // Plant the gh-stack canonical-file symlinks for the new worktree first. - // Unlike `gt track`, there's nothing to run inside another process: this is - // pure filesystem plumbing that makes gh-stack's own writes visible from - // every worktree, so it isn't gated on `branch_pre_existed`. - if !self.no_stack { + // `link_worktree` is filesystem plumbing (making gh-stack's canonical file + // visible from this worktree), not registration — it runs whenever gh-stack is + // one of Mixed's providers, regardless of which one owns this fork. + if owner != StackProvider::GhStack && !self.no_stack { if let Some(name) = worktree.name() { if let Err(e) = link_worktree(&repo, name) { output::warn(&format!("gh-stack link failed: {}", e)); } } } - - // Register the branch in the canonical file, after the symlinks above are - // in place. Skipped (not warned) when there's no known base branch — there - // is no stack to append onto without one. - if !self.no_stack && !branch_pre_existed && config.stack_auto_track(None)? { - if let Some(base) = base_branch.as_deref() { - if let Err(e) = register_branch(&repo, &effective_branch, base) { - output::warn(&format!("gh-stack register failed: {}", e)); - } - } else { - debug!("gh-stack register skipped: no base branch known"); - } - } } _ => {} } @@ -432,6 +428,111 @@ impl Run for New { } } +/// Register `worktree`'s branch with Graphite via `gt track`, non-fatal on failure. +/// +/// Deliberately NOT guarded on `detect_gt()`: `StackModel::detect` resolves `Graphite` from +/// repo metadata alone, so this can run on a machine without `gt`. The resulting "gt track +/// unavailable" warning is the point — the new branch really is untracked, and silence would +/// hide that until the stack looked wrong later. See `new_gt_track_failure_is_non_fatal`. +fn register_graphite( + repo: &git2::Repository, + worktree: &WorktreeDescriptor, + config: &WorkonConfig, + base_branch: Option<&str>, + branch_pre_existed: bool, + no_stack: bool, +) -> Result<()> { + if !no_stack && !branch_pre_existed && config.gt_auto_track(None)? { + // Prefer the explicit base branch, then the repo's graphite trunk. If neither is + // known, omit --parent so gt infers from its own config. + let parent = base_branch + .map(String::from) + .or_else(|| graphite_trunk(repo)); + debug!( + "Running: gt track{} in {}", + parent + .as_deref() + .map(|p| format!(" --parent {p}")) + .unwrap_or_default(), + worktree.path().display() + ); + let mut cmd = std::process::Command::new("gt"); + cmd.arg("track"); + if let Some(p) = &parent { + cmd.arg("--parent").arg(p); + } + match cmd.current_dir(worktree.path()).output() { + Ok(out) if out.status.success() => { + debug!("gt track succeeded"); + } + Ok(out) => { + let stderr = String::from_utf8_lossy(&out.stderr); + output::warn(&format!("gt track failed: {}", stderr.trim())); + } + Err(e) => { + output::warn(&format!("gt track unavailable: {}", e)); + } + } + } + Ok(()) +} + +/// Register `effective_branch` with gh-stack: plant the canonical-file symlinks, then append +/// the branch onto `base_branch`'s stack. Non-fatal on failure. +fn register_gh_stack( + repo: &git2::Repository, + worktree: &WorktreeDescriptor, + config: &WorkonConfig, + effective_branch: &str, + base_branch: Option<&str>, + branch_pre_existed: bool, + no_stack: bool, +) -> Result<()> { + // Plant the gh-stack canonical-file symlinks for the new worktree first. Unlike `gt + // track`, there's nothing to run inside another process: this is pure filesystem + // plumbing that makes gh-stack's own writes visible from every worktree, so it isn't + // gated on `branch_pre_existed`. + if !no_stack { + if let Some(name) = worktree.name() { + if let Err(e) = link_worktree(repo, name) { + output::warn(&format!("gh-stack link failed: {}", e)); + } + } + } + + // Register the branch in the canonical file, after the symlinks above are in place. + // Skipped (not warned) when there's no known base branch — there is no stack to append + // onto without one. + if !no_stack && !branch_pre_existed && config.stack_auto_track(None)? { + if let Some(base) = base_branch { + if let Err(e) = register_branch(repo, effective_branch, base) { + output::warn(&format!("gh-stack register failed: {}", e)); + } + } else { + debug!("gh-stack register skipped: no base branch known"); + } + } + Ok(()) +} + +/// `true` if `provider` has a tracked-stack row for `branch` — used under `StackModel::Mixed` +/// to find which provider owns the fork's base branch. A read error (corrupt store) counts as +/// "doesn't track it" with a debug log: registration is already non-fatal, and the provider +/// that does own the base can still claim the fork. +fn provider_tracks(repo: &git2::Repository, branch: &str, provider: StackProvider) -> bool { + let model = match provider { + StackProvider::Graphite => StackModel::Graphite, + StackProvider::GhStack => StackModel::GhStack, + }; + match current_stack(repo, branch, model) { + Ok(stack) => stack.is_some(), + Err(e) => { + debug!("{provider:?} metadata unreadable while resolving fork owner: {e}"); + false + } + } +} + /// Return the HEAD branch of the current stack-worktree under `model`, or None if cwd /// is not inside any worktree or the worktree's branch is not part of a tracked stack. fn current_stack_branch(repo: &git2::Repository, model: StackModel) -> Result> { diff --git a/git-workon/tests/suite/new.rs b/git-workon/tests/suite/new.rs index 65b79c25..38a28336 100644 --- a/git-workon/tests/suite/new.rs +++ b/git-workon/tests/suite/new.rs @@ -1394,3 +1394,144 @@ fn new_gh_stack_register_lock_contention_warns_and_exits_zero( Ok(()) } + +// ── StackModel::Mixed registration ──────────────────────────────────────────── +// Both tools have live branches (auto resolves to Mixed, gh-stack primary); a fork's base +// branch decides which provider registers it (plan: "follow the parent's provider"). + +#[test] +fn new_under_mixed_forks_off_gh_stack_branch_registers_with_gh_stack( +) -> Result<(), Box> { + let fixture = FixtureBuilder::new() + .bare(true) + .default_branch("main") + .worktree("main") + .graphite_config(&["main"]) + .branch_metadata("graphite-only", "main") + .gh_stack(None, 1, "main", &["gh-stack-base"]) + .build()?; + + let output = cargo_bin_cmd!("git-workon") + .current_dir(&fixture) + .env("PATH", path_without_gt_new()) + .arg("new") + .arg("feat-2") + .arg("--base") + .arg("gh-stack-base") + .output()?; + + assert!( + output.status.success(), + "new must succeed; stderr: {}", + std::str::from_utf8(&output.stderr).unwrap_or("(invalid utf8)") + ); + + let bare_path = fixture.root()?.join(".bare"); + let bare_repo = git2::Repository::open_bare(&bare_path)?; + let base_oid = bare_repo + .find_branch("gh-stack-base", git2::BranchType::Local)? + .get() + .target() + .unwrap(); + bare_repo.assert(predicate::repo::gh_stack_branch_base( + None, + "feat-2", + base_oid.to_string(), + )); + + let stderr = std::str::from_utf8(&output.stderr)?; + assert!( + !stderr.contains("gt track"), + "a fork owned by gh-stack must never attempt gt track: {stderr}" + ); + + Ok(()) +} + +#[test] +fn new_under_mixed_forks_off_trunk_uses_primary_gh_stack() -> Result<(), Box> +{ + // "main" is the trunk, not a tracked diff in either provider, so neither `providers()` + // check in the Mixed arm claims it as an owner and the fork falls to `primary` (gh-stack). + // gh-stack has no stack ending at "main" (its one stack ends at "gh-stack-only"), so + // registration itself fails — but non-fatally, and via gh-stack, never `gt track`. + let fixture = FixtureBuilder::new() + .bare(true) + .default_branch("main") + .worktree("main") + .graphite_config(&["main"]) + .branch_metadata("graphite-only", "main") + .gh_stack(None, 1, "main", &["gh-stack-only"]) + .build()?; + + let output = cargo_bin_cmd!("git-workon") + .current_dir(&fixture) + .env("PATH", path_without_gt_new()) + .arg("new") + .arg("feat-2") + .arg("--base") + .arg("main") + .output()?; + + assert!( + output.status.success(), + "new must succeed even when gh-stack registration fails; stderr: {}", + std::str::from_utf8(&output.stderr).unwrap_or("(invalid utf8)") + ); + + let stderr = std::str::from_utf8(&output.stderr)?; + assert!( + stderr.contains("Warning:") && stderr.contains("gh-stack register failed"), + "an untracked base falls to the primary (gh-stack) provider: {stderr}" + ); + assert!( + !stderr.contains("gt track"), + "gh-stack owns this fork under the new default; graphite must not run: {stderr}" + ); + + Ok(()) +} + +#[test] +fn new_under_auto_detected_mixed_base_less_fork_links_gh_stack_without_registering( +) -> Result<(), Box> { + // No explicit workon.stackModel: both providers have live branches, so `auto` resolves to + // `Mixed` with gh-stack primary. No `--base` means neither provider's `providers()` check + // can claim ownership via a tracked base branch, so the fork falls to `primary` (gh-stack) + // — which links the new worktree's gh-stack admin-dir symlinks but skips the + // canonical-file append (nothing to append onto), and never touches Graphite. + let fixture = FixtureBuilder::new() + .bare(true) + .default_branch("main") + .worktree("main") + .graphite_config(&["main"]) + .branch_metadata("graphite-only", "main") + .gh_stack(None, 1, "main", &["gh-stack-only"]) + .build()?; + + let output = cargo_bin_cmd!("git-workon") + .current_dir(&fixture) + .env("PATH", path_without_gt_new()) + .arg("new") + .arg("feat-3") + .output()?; + + assert!( + output.status.success(), + "new must succeed with no base branch; stderr: {}", + std::str::from_utf8(&output.stderr).unwrap_or("(invalid utf8)") + ); + + let bare_path = fixture.root()?.join(".bare"); + let bare_repo = git2::Repository::open_bare(&bare_path)?; + bare_repo.assert(predicate::repo::gh_stack_is_linked("feat-3")); + bare_repo.assert(predicate::repo::gh_stack_contains_branch(None, "feat-3", 0).not()); + + let stderr = std::str::from_utf8(&output.stderr)?; + assert!( + !stderr.contains("gt track"), + "gh-stack owns base-less forks under the new default; graphite must not run: {stderr}" + ); + + Ok(()) +} From 9121258ebc1938ec5c05a1b6151aea38e8977463 Mon Sep 17 00:00:00 2001 From: Eric Eldredge Date: Thu, 10 Sep 2026 12:16:31 -0400 Subject: [PATCH 3/4] feat(cli): report dual stack metadata in doctor and hint hidden branches in list --- git-workon/src/cmd/doctor.rs | 160 +++++++++++++++-- git-workon/src/cmd/list.rs | 29 +++- git-workon/tests/suite/doctor.rs | 283 ++++++++++++++++++++++++++++++- git-workon/tests/suite/list.rs | 109 ++++++++++++ 4 files changed, 563 insertions(+), 18 deletions(-) diff --git a/git-workon/src/cmd/doctor.rs b/git-workon/src/cmd/doctor.rs index 79aa9f55..26a72e5b 100644 --- a/git-workon/src/cmd/doctor.rs +++ b/git-workon/src/cmd/doctor.rs @@ -25,7 +25,9 @@ //! - Worktrees whose gh-stack admin-dir file isn't linked to the canonical store — fixable //! with --fix (plants the link, or merges a pre-existing real file first) //! - gh-stack files that fail to parse, or whose stacks disagree across worktrees -//! - Both Graphite and gh-stack artifacts present with the model left on `auto` +//! - Both Graphite and gh-stack artifacts present with the model left on `auto`: reports the +//! dual state, what `auto` resolved to, and each provider's liveness +//! - An explicit `workon.stackModel` pin hiding the other provider's live tracked branches //! //! ## Flags: //! - `--fix` - Automatically repair fixable issues (missing directory entries, renamed keys) @@ -40,8 +42,8 @@ use workon::{ encode_worktree_name, get_repo, get_worktrees, gh_stack_divergent_stack_numbers, gh_stack_readability_errors, gh_stack_worktree_link_status, is_gh_stack_repo, is_graphite_active, is_graphite_repo, link_worktree, migrate_worktree, preferred_remote_order, - relative_worktree_path, rename_worktree_metadata, GhStackLinkStatus, Granularity, StackModel, - WorkonConfig, WorktreeDescriptor, + provider_has_live_branches, relative_worktree_path, rename_worktree_metadata, + GhStackLinkStatus, Granularity, StackModel, StackProvider, WorkonConfig, WorktreeDescriptor, }; use crate::cli::Doctor; @@ -104,7 +106,18 @@ enum IssueKind { GhStackDivergentStacks { number: u64, }, - BothStackToolsDetected, + BothStackToolsDetected { + resolved: String, + graphite_live: bool, + gh_stack_live: bool, + }, + StackModelPinHidesLiveBranches { + pinned: String, + hidden: String, + }, + MixedPinPrimaryNotInitialized { + primary: StackProvider, + }, } struct Issue { @@ -226,9 +239,39 @@ impl Issue { "stack #{number} is defined differently in more than one worktree's gh-stack file" ) } - IssueKind::BothStackToolsDetected => { - "both Graphite and gh-stack artifacts are present; set workon.stackModel explicitly to avoid ambiguity".to_string() + IssueKind::BothStackToolsDetected { + resolved, + graphite_live, + gh_stack_live, + } => match (graphite_live, gh_stack_live) { + (true, true) => format!( + "both Graphite and gh-stack have tracked branches; auto resolves to {resolved}; pin one with workon.stackModel to make it strict" + ), + (true, false) => format!( + "both Graphite and gh-stack artifacts are present; only Graphite has tracked branches, auto resolves to {resolved}" + ), + (false, true) => format!( + "both Graphite and gh-stack artifacts are present; only gh-stack has tracked branches, auto resolves to {resolved}" + ), + (false, false) => { + "both Graphite and gh-stack artifacts are present; set workon.stackModel explicitly to avoid ambiguity".to_string() + } + }, + IssueKind::StackModelPinHidesLiveBranches { pinned, hidden } => { + format!( + "{hidden} has tracked branches that workon.stackModel={pinned} hides; unset it (auto) to show both" + ) } + IssueKind::MixedPinPrimaryNotInitialized { primary } => match primary { + StackProvider::Graphite => { + "stackModel=mixed:graphite but repo not gt-initialized — run: gt init" + .to_string() + } + StackProvider::GhStack => { + "stackModel=mixed:gh-stack but no gh-stack file found — run: gh stack init" + .to_string() + } + }, } } @@ -253,7 +296,11 @@ impl Issue { IssueKind::GhStackNotInitialized => "gh_stack_not_initialized", IssueKind::GhStackFileUnreadable { .. } => "gh_stack_file_unreadable", IssueKind::GhStackDivergentStacks { .. } => "gh_stack_divergent_stacks", - IssueKind::BothStackToolsDetected => "both_stack_tools_detected", + IssueKind::BothStackToolsDetected { .. } => "both_stack_tools_detected", + IssueKind::StackModelPinHidesLiveBranches { .. } => { + "stack_model_pin_hides_live_branches" + } + IssueKind::MixedPinPrimaryNotInitialized { .. } => "mixed_pin_primary_not_initialized", } } } @@ -265,8 +312,12 @@ impl Run for Doctor { let config = WorkonConfig::new(&repo)?; // Computed once up front: gates the per-worktree link check below and the // dependency-section gh-stack checks further down, mirroring the GraphiteNotInitialized - // gate later in this function. - let gh_stack_active = matches!(config.stack_model(None), Ok(StackModel::GhStack)); + // gate later in this function. True whenever gh-stack is one of the model's providers, + // so a `Mixed` repo still gets its unlinked worktree files flagged (and `--fix`ed). + let gh_stack_active = config + .stack_model(None) + .map(|m| m.providers().any(|p| p == StackProvider::GhStack)) + .unwrap_or(false); debug!("found {} worktree(s)", worktrees.len()); output::status(&format!("Checking {} worktree(s)...", worktrees.len())); @@ -580,10 +631,27 @@ impl Run for Doctor { } } - // Both tools' artifacts present, model left on auto/unset: `StackModel::detect` picks - // Graphite deterministically (see its docs), but the user tried both tools and might - // not know which one workon is actually reading. An explicit `workon.stackModel = - // graphite` silences this — it's read as "I know, and I mean it." + // Explicit `mixed:` pin naming a primary with no artifacts at all: mirrors + // GraphiteNotInitialized/GhStackNotInitialized above, but for the pinned Mixed primary + // rather than a strict Graphite/GhStack pin. + if let Ok(StackModel::Mixed { primary }) = config.stack_model(None) { + let primary_has_artifacts = match primary { + StackProvider::Graphite => is_graphite_repo(&repo), + StackProvider::GhStack => is_gh_stack_repo(&repo), + }; + if !primary_has_artifacts { + debug!("workon.stackModel=mixed: but primary has no artifacts"); + let issue = Issue::config(IssueKind::MixedPinPrimaryNotInitialized { primary }); + output::check_warn("stack", &issue.message()); + issues.push(issue); + } + } + + // Both tools' artifacts present, model left on auto/unset: report the dual state and + // what `auto` actually resolved to (see `StackModel::detect`'s live-metadata + // tie-break), plus each provider's liveness. An explicit `workon.stackModel = + // graphite`/`gh-stack` silences this — it's read as "I know, and I mean it" — but gets + // its own `StackModelPinHidesLiveBranches` warning below if it hides live branches. let stack_model_is_explicit = !matches!( repo.config() .ok() @@ -592,12 +660,58 @@ impl Run for Doctor { None | Some("auto") ); if !stack_model_is_explicit && is_graphite_repo(&repo) && is_gh_stack_repo(&repo) { - debug!("both graphite and gh-stack artifacts present, model left on auto"); - let issue = Issue::config(IssueKind::BothStackToolsDetected); + let graphite_live = provider_has_live_branches(&repo, StackProvider::Graphite); + let gh_stack_live = provider_has_live_branches(&repo, StackProvider::GhStack); + let resolved = match config.stack_model(None) { + Ok(StackModel::Mixed { + primary: StackProvider::Graphite, + }) => "mixed (graphite primary)".to_string(), + Ok(StackModel::Mixed { + primary: StackProvider::GhStack, + }) => "mixed (gh-stack primary)".to_string(), + Ok(StackModel::Graphite) => "graphite".to_string(), + Ok(StackModel::GhStack) => "gh-stack".to_string(), + _ => "gh-stack".to_string(), + }; + debug!( + "both graphite and gh-stack artifacts present, model left on auto (resolved={resolved})" + ); + let issue = Issue::config(IssueKind::BothStackToolsDetected { + resolved, + graphite_live, + gh_stack_live, + }); output::check_warn("stack", &issue.message()); issues.push(issue); } + // An explicit pin hides the other provider's live branches — same predicate as the + // `list` hint (see `crate::cmd::list`). Under `auto` this state can't occur (`detect` + // would have produced `Mixed`), so seeing a plain `Graphite`/`GhStack` model already + // means the pin is explicit. + if stack_model_is_explicit { + let hidden = match config.stack_model(None) { + Ok(StackModel::Graphite) => { + provider_has_live_branches(&repo, StackProvider::GhStack) + .then(|| ("gh-stack".to_string(), "graphite".to_string())) + } + Ok(StackModel::GhStack) => { + provider_has_live_branches(&repo, StackProvider::Graphite) + .then(|| ("Graphite".to_string(), "gh-stack".to_string())) + } + _ => None, + }; + if let Some((hidden_provider, pinned)) = hidden { + debug!("workon.stackModel pin hides the other provider's live branches"); + let issue = Issue::config(IssueKind::StackModelPinHidesLiveBranches { + pinned, + hidden: hidden_provider, + }); + output::check_warn("stack", &issue.message()); + issues.push(issue); + } + } + debug!("found {} issue(s) total", issues.len()); // JSON output: serialize all collected issues @@ -648,6 +762,22 @@ impl Run for Doctor { if let IssueKind::GhStackDivergentStacks { number } = &issue.kind { obj["number"] = json!(number); } + if let IssueKind::BothStackToolsDetected { + resolved, + graphite_live, + gh_stack_live, + } = &issue.kind + { + obj["resolved"] = json!(resolved); + obj["graphite_live"] = json!(graphite_live); + obj["gh_stack_live"] = json!(gh_stack_live); + } + if let IssueKind::StackModelPinHidesLiveBranches { pinned, hidden } = + &issue.kind + { + obj["pinned"] = json!(pinned); + obj["hidden"] = json!(hidden); + } if let IssueKind::RenamedConfigKey { old_key, new_key, diff --git a/git-workon/src/cmd/list.rs b/git-workon/src/cmd/list.rs index ea224ec9..83b86acc 100644 --- a/git-workon/src/cmd/list.rs +++ b/git-workon/src/cmd/list.rs @@ -67,14 +67,15 @@ use log::debug; use miette::{IntoDiagnostic, Result}; use serde_json::json; use workon::{ - current_stack, enumerate_stacks, get_repo, get_worktrees, group_by_stack, WorkonConfig, - WorktreeDescriptor, + current_stack, enumerate_stacks, get_repo, get_worktrees, group_by_stack, + provider_has_live_branches, StackProvider, WorkonConfig, WorktreeDescriptor, }; use crate::cli::List; use crate::cmd::filter::StatusFilter; use crate::display::{build_tree, format_aligned_rows, format_tree_lines, worktree_display_row}; use crate::json::worktree_to_json; +use crate::output; use super::Run; @@ -94,6 +95,30 @@ impl Run for List { WorkonConfig::new(&repo)?.stack_model(None)? }; + // An explicit `workon.stackModel` pin can hide the other provider's live branches — + // under `auto` this state can't occur (detect would have produced `Mixed` instead), so + // seeing a plain `Graphite`/`GhStack` model here already means the pin is explicit; no + // need to re-read config to confirm it. Mixed itself stays quiet — both providers are + // already visible. + if !self.no_stack { + let hidden = match effective_model { + workon::StackModel::Graphite => { + provider_has_live_branches(&repo, StackProvider::GhStack) + .then_some(("gh-stack", "graphite")) + } + workon::StackModel::GhStack => { + provider_has_live_branches(&repo, StackProvider::Graphite) + .then_some(("Graphite", "gh-stack")) + } + _ => None, + }; + if let Some((hidden_provider, pinned_model)) = hidden { + output::detail(&format!( + "{hidden_provider} has tracked branches that workon.stackModel={pinned_model} hides; unset it (auto) to show both" + )); + } + } + // Apply filters (AND logic) let filtered: Vec<_> = worktrees .into_iter() diff --git a/git-workon/tests/suite/doctor.rs b/git-workon/tests/suite/doctor.rs index 6fb87254..f214c867 100644 --- a/git-workon/tests/suite/doctor.rs +++ b/git-workon/tests/suite/doctor.rs @@ -631,6 +631,51 @@ fn doctor_detects_unlinked_gh_stack_worktree_and_fixes_with_link( Ok(()) } +#[test] +fn doctor_links_unlinked_gh_stack_worktree_under_mixed_model( +) -> Result<(), Box> { + // Both providers live, so `auto` resolves to Mixed. The gh-stack link checks are gated on + // gh-stack being one of the model's providers, not on the model being exactly GhStack — + // otherwise a Mixed repo would never get its unlinked worktree file flagged or fixed. + let fixture = FixtureBuilder::new() + .bare(true) + .default_branch("main") + .worktree("main") + .worktree("feat-a") + .graphite_config(&["main"]) + .branch_metadata("gt-a", "main") + .gh_stack(None, 1, "main", &["feat-a"]) + .build()?; + + let main_path = fixture.root()?.join("main"); + cargo_bin_cmd!("git-workon") + .current_dir(&main_path) + .env("NO_COLOR", "1") + .arg("doctor") + .assert() + .success() + .stderr(predicate::str::contains("mixed (gh-stack primary)")) + .stderr(predicate::str::contains( + "is not linked to the canonical gh-stack file", + )); + + cargo_bin_cmd!("git-workon") + .current_dir(&main_path) + .env("NO_COLOR", "1") + .arg("doctor") + .arg("--fix") + .assert() + .success() + .stderr(predicate::str::contains( + "Linked to canonical gh-stack file", + )); + + let bare_repo = git2::Repository::open_bare(fixture.root()?.join(".bare"))?; + bare_repo.assert(predicate::repo::gh_stack_is_linked("feat-a")); + + Ok(()) +} + #[test] fn doctor_migrates_worktree_holding_a_real_gh_stack_file() -> Result<(), Box> { @@ -704,6 +749,85 @@ fn doctor_json_gh_stack_extension_not_found_emits_kind() -> Result<(), Box Result<(), Box> { + let fixture = FixtureBuilder::new() + .bare(true) + .default_branch("main") + .worktree("main") + .config("workon.stackModel", "mixed:graphite") + .build()?; + + let main_path = fixture.root()?.join("main"); + cargo_bin_cmd!("git-workon") + .current_dir(&main_path) + .env("NO_COLOR", "1") + .arg("doctor") + .assert() + .success() + .stderr(predicate::str::contains( + "stackModel=mixed:graphite but repo not gt-initialized — run: gt init", + )); + + Ok(()) +} + +#[test] +fn doctor_warns_mixed_gh_stack_pin_when_gh_stack_has_no_artifacts( +) -> Result<(), Box> { + let fixture = FixtureBuilder::new() + .bare(true) + .default_branch("main") + .worktree("main") + .config("workon.stackModel", "mixed:gh-stack") + .build()?; + + let main_path = fixture.root()?.join("main"); + cargo_bin_cmd!("git-workon") + .current_dir(&main_path) + .env("NO_COLOR", "1") + .arg("doctor") + .assert() + .success() + .stderr(predicate::str::contains( + "stackModel=mixed:gh-stack but no gh-stack file found — run: gh stack init", + )); + + Ok(()) +} + +#[test] +fn doctor_json_mixed_pin_primary_not_initialized_emits_kind( +) -> Result<(), Box> { + let fixture = FixtureBuilder::new() + .bare(true) + .default_branch("main") + .worktree("main") + .config("workon.stackModel", "mixed:graphite") + .build()?; + + let main_path = fixture.root()?.join("main"); + let output = cargo_bin_cmd!("git-workon") + .current_dir(&main_path) + .arg("doctor") + .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 issues = parsed["issues"].as_array().expect("issues must be array"); + assert!( + issues + .iter() + .any(|i| i["kind"] == "mixed_pin_primary_not_initialized"), + "expected mixed_pin_primary_not_initialized issue in: {stdout}" + ); + + Ok(()) +} + #[test] fn doctor_warns_both_stack_tools_detected_when_model_is_auto( ) -> Result<(), Box> { @@ -724,12 +848,137 @@ fn doctor_warns_both_stack_tools_detected_when_model_is_auto( .assert() .success() .stderr(predicate::str::contains( - "both Graphite and gh-stack artifacts are present", + "both Graphite and gh-stack have tracked branches; auto resolves to mixed (gh-stack primary)", + )); + + Ok(()) +} + +#[test] +fn doctor_json_both_stack_tools_detected_reports_resolved_and_liveness( +) -> Result<(), Box> { + let fixture = FixtureBuilder::new() + .bare(true) + .default_branch("main") + .worktree("main") + .graphite_config(&["main"]) + .branch_metadata("feat-a", "main") + .gh_stack(None, 1, "main", &["feat-b"]) + .build()?; + + let main_path = fixture.root()?.join("main"); + let output = cargo_bin_cmd!("git-workon") + .current_dir(&main_path) + .arg("doctor") + .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 issues = parsed["issues"].as_array().expect("issues must be array"); + let issue = issues + .iter() + .find(|i| i["kind"] == "both_stack_tools_detected") + .unwrap_or_else(|| panic!("no both_stack_tools_detected issue in: {stdout}")); + + assert_eq!(issue["resolved"], "mixed (gh-stack primary)"); + assert_eq!(issue["graphite_live"], true); + assert_eq!(issue["gh_stack_live"], true); + + Ok(()) +} + +#[test] +fn doctor_warns_both_stack_tools_detected_when_only_one_is_live( +) -> Result<(), Box> { + // Graphite's only row is a ghost (branch deleted); gh-stack's tracked branch is live. + let fixture = FixtureBuilder::new() + .bare(true) + .default_branch("main") + .worktree("main") + .graphite_config(&["main"]) + .ghost_branch_metadata("feat-a", "main") + .gh_stack(None, 1, "main", &["feat-b"]) + .build()?; + + let main_path = fixture.root()?.join("main"); + cargo_bin_cmd!("git-workon") + .current_dir(&main_path) + .env("NO_COLOR", "1") + .arg("doctor") + .assert() + .success() + .stderr(predicate::str::contains( + "only gh-stack has tracked branches, auto resolves to gh-stack", )); Ok(()) } +#[test] +fn doctor_warns_stack_model_pin_hides_live_branches() -> Result<(), Box> { + let fixture = FixtureBuilder::new() + .bare(true) + .default_branch("main") + .worktree("main") + .config("workon.stackModel", "graphite") + .graphite_config(&["main"]) + .branch_metadata("feat-a", "main") + .gh_stack(None, 1, "main", &["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("doctor") + .output()?; + + assert!(output.status.success()); + let stderr = std::str::from_utf8(&output.stderr)?; + assert!( + stderr.contains("gh-stack has tracked branches that workon.stackModel=graphite hides"), + "expected pin-hides-live-branches warning in stderr: {stderr}" + ); + + Ok(()) +} + +#[test] +fn doctor_json_stack_model_pin_hides_live_branches() -> Result<(), Box> { + let fixture = FixtureBuilder::new() + .bare(true) + .default_branch("main") + .worktree("main") + .config("workon.stackModel", "graphite") + .graphite_config(&["main"]) + .branch_metadata("feat-a", "main") + .gh_stack(None, 1, "main", &["feat-b"]) + .build()?; + + let main_path = fixture.root()?.join("main"); + let output = cargo_bin_cmd!("git-workon") + .current_dir(&main_path) + .arg("doctor") + .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 issues = parsed["issues"].as_array().expect("issues must be array"); + let issue = issues + .iter() + .find(|i| i["kind"] == "stack_model_pin_hides_live_branches") + .unwrap_or_else(|| panic!("no stack_model_pin_hides_live_branches issue in: {stdout}")); + + assert_eq!(issue["pinned"], "graphite"); + assert_eq!(issue["hidden"], "gh-stack"); + + Ok(()) +} + #[test] fn doctor_does_not_warn_both_stack_tools_when_model_is_explicit( ) -> Result<(), Box> { @@ -809,3 +1058,35 @@ fn doctor_flags_unreadable_gh_stack_file_for_unsupported_schema( Ok(()) } + +#[test] +fn doctor_json_config_summary_reports_mixed_with_primary() -> Result<(), Box> +{ + let fixture = FixtureBuilder::new() + .bare(true) + .default_branch("main") + .worktree("main") + .graphite_config(&["main"]) + .branch_metadata("feat-a", "main") + .gh_stack(None, 1, "main", &["feat-b"]) + .build()?; + + let main_path = fixture.root()?.join("main"); + let output = cargo_bin_cmd!("git-workon") + .current_dir(&main_path) + .arg("doctor") + .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 config = &parsed["configuration"]; + + assert_eq!( + config["workon.stackModel"]["value"], "mixed (gh-stack primary)", + "configuration summary must report the resolved Mixed model: {stdout}" + ); + + Ok(()) +} diff --git a/git-workon/tests/suite/list.rs b/git-workon/tests/suite/list.rs index 7873fc8a..3d7c1ec5 100644 --- a/git-workon/tests/suite/list.rs +++ b/git-workon/tests/suite/list.rs @@ -1492,3 +1492,112 @@ fn list_tree_marks_merged_worktree_branch_and_hides_merged_metadata_only_branch( Ok(()) } + +// ── Mixed stack model pin hint ──────────────────────────────────────────────── +// Both providers have live tracked branches. Under `auto` this resolves to `Mixed` and stays +// quiet; an explicit pin hides the other provider and gets a dimmed stderr hint (NO_COLOR=1 +// per the FORCE_COLOR trap — see git-workon-fixture/src/lib.rs's module docs). + +#[test] +fn list_auto_with_both_providers_live_shows_both_stacks_and_no_hint( +) -> Result<(), Box> { + let fixture = FixtureBuilder::new() + .bare(true) + .default_branch("main") + .worktree("main") + .graphite_config(&["main"]) + .branch_metadata("graphite-only", "main") + .gh_stack(None, 1, "main", &["gh-stack-only"]) + .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 stderr = std::str::from_utf8(&output.stderr)?; + assert!(stderr.is_empty(), "Mixed must stay quiet: {stderr}"); + + let stdout = std::str::from_utf8(&output.stdout)?; + assert!( + stdout.contains("graphite-only") && stdout.contains("gh-stack-only"), + "both providers' tracked branches must appear: {stdout}" + ); + + Ok(()) +} + +#[test] +fn list_explicit_graphite_pin_hints_hidden_gh_stack_branches( +) -> Result<(), Box> { + let fixture = FixtureBuilder::new() + .bare(true) + .default_branch("main") + .worktree("main") + .config("workon.stackModel", "graphite") + .graphite_config(&["main"]) + .branch_metadata("graphite-only", "main") + .gh_stack(None, 1, "main", &["gh-stack-only"]) + .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 stderr = std::str::from_utf8(&output.stderr)?; + assert!( + stderr.contains("gh-stack has tracked branches") + && stderr.contains("workon.stackModel=graphite"), + "expected pin hint in stderr: {stderr}" + ); + + Ok(()) +} + +#[test] +fn list_json_and_no_stack_suppress_pin_hint() -> Result<(), Box> { + let fixture = FixtureBuilder::new() + .bare(true) + .default_branch("main") + .worktree("main") + .config("workon.stackModel", "graphite") + .graphite_config(&["main"]) + .branch_metadata("graphite-only", "main") + .gh_stack(None, 1, "main", &["gh-stack-only"]) + .build()?; + + let main_path = fixture.root()?.join("main"); + + let json_output = cargo_bin_cmd!("git-workon") + .current_dir(&main_path) + .env("NO_COLOR", "1") + .arg("list") + .arg("--json") + .output()?; + assert!(json_output.status.success()); + assert!( + std::str::from_utf8(&json_output.stderr)?.is_empty(), + "--json must never print the hint" + ); + + let no_stack_output = cargo_bin_cmd!("git-workon") + .current_dir(&main_path) + .env("NO_COLOR", "1") + .arg("list") + .arg("--no-stack") + .output()?; + assert!(no_stack_output.status.success()); + assert!( + std::str::from_utf8(&no_stack_output.stderr)?.is_empty(), + "--no-stack must never print the hint" + ); + + Ok(()) +} From 601fd9170114f45836ed81042be8d29d325b8a6f Mon Sep 17 00:00:00 2001 From: Eric Eldredge Date: Thu, 10 Sep 2026 12:16:41 -0400 Subject: [PATCH 4/4] docs: describe mixed stack-model detection --- README.md | 5 +- .../028-provider-neutral-stack-metadata.md | 48 ++++++++++++++----- docs/recipes/stacked-diffs.md | 43 +++++++++++++---- 3 files changed, 74 insertions(+), 22 deletions(-) diff --git a/README.md b/README.md index 88415ebb..3a6ff616 100644 --- a/README.md +++ b/README.md @@ -159,7 +159,7 @@ git workon doctor --dry-run # preview fixes without applying Checks performed: - **Worktrees**: missing directories, broken git links, gone upstreams - **Dependencies**: `gh` CLI (PR features), `gh` auth status, git remote, `gt` CLI (stack features), hook commands in PATH -- **Configuration**: renamed config keys (auto-fixable), invalid `stackModel`/`stackWorktreeGranularity`, invalid `prFormat`, `defaultBranch` not found in repo, `stackModel=graphite` without `gt init` +- **Configuration**: renamed config keys (auto-fixable), invalid `stackModel`/`stackWorktreeGranularity`, invalid `prFormat`, `defaultBranch` not found in repo, `stackModel=graphite` without `gt init`, both Graphite and gh-stack artifacts present (reports what `auto` resolved to and each provider's liveness), an explicit `stackModel` pin hiding the other provider's tracked branches ### Copy untracked files between worktrees @@ -256,7 +256,8 @@ man git-workon pruneBranches = true # also consider local branches with no worktree # Stacked diffs (Graphite or gh-stack) - stackModel = auto # "auto", "graphite", "gh-stack", "git", or "none" + stackModel = auto # "auto", "graphite", "gh-stack", "git", "none", + # "mixed:graphite", or "mixed:gh-stack" stackWorktreeGranularity = stack # "stack" (one worktree per stack) stackAutoTrack = true # auto-register new branches with the active stack tool after 'workon new' gtAutoTrack = true # deprecated alias for stackAutoTrack, read only as a fallback diff --git a/docs/adr/028-provider-neutral-stack-metadata.md b/docs/adr/028-provider-neutral-stack-metadata.md index d2d17c3b..93e4eae5 100644 --- a/docs/adr/028-provider-neutral-stack-metadata.md +++ b/docs/adr/028-provider-neutral-stack-metadata.md @@ -101,18 +101,42 @@ pre-existing per-worktree files via `migrate_worktree`; the union read in `gh_stack::read_metadata` survives as a degraded fallback for whatever isn't yet linked. See "The shared canonical file" below for why the symlink approach works at all. -## Detection: Graphite wins - -`StackModel::detect` (`stack.rs`) checks Graphite first, then gh-stack: `.graphite_repo_config` -or `.graphite_metadata.db` existing means `Graphite`; otherwise a `gh-stack` file anywhere -`gh_stack::is_gh_stack_repo` looks means `GhStack`; otherwise `None`. `.graphite_repo_config` -comes from an explicit, repo-wide `gt init`, while a `gh-stack` file can appear as a side effect -of one `gh stack add` run in a single worktree, so the more deliberate, repo-scoped signal wins. -No repository that resolves to `Graphite` today can silently flip to `GhStack` because someone -tried the other tool once in one worktree. The escape hatch is an explicit -`workon.stackModel = gh-stack`; `doctor`'s `BothStackToolsDetected` check surfaces the ambiguity -when both artifacts are present so the user knows to pin the config if `auto` picked the wrong -one. +## Detection: live-metadata tie-break + +> **Amended 2026-09-10, 2026-09-16.** This section replaces the original "Graphite wins" rule +> (both artifacts present always resolved to `Graphite`) with the live-metadata tie-break below. +> The old rule is in this file's git history. + +`StackModel::detect` (`stack.rs`) checks artifacts first: `.graphite_repo_config` or +`.graphite_metadata.db` existing means Graphite artifacts are present; a `gh-stack` file +anywhere `gh_stack::is_gh_stack_repo` looks means gh-stack artifacts are present. With only one +artifact present, detection returns that provider directly (`None` if neither) — unchanged from +before, and no metadata read is needed since there is no ambiguity. + +With **both** artifacts present, detection reads each provider's liveness — whether it has at +least one ref-backed tracked branch (`graphite::has_live_branches` / +`gh_stack::has_live_branches`, checking `StackMetadata::parents`' keys against +`resolve::branch_exists`, never probing the `gt` binary) — and ties break on the table: + +| Graphite live | gh-stack live | Resolves to | +| --- | --- | --- | +| yes | yes | `StackModel::Mixed { primary: GhStack }` | +| yes | no | `StackModel::Graphite` (gh-stack's artifact is stale) | +| no | yes | `StackModel::GhStack` (Graphite's artifact is stale) | +| no | no | `StackModel::GhStack` (doubly stale; `doctor` explains) | + +`Mixed` makes gh-stack primary — GitHub's native stack support removed the reason to treat +Graphite's repo-wide `gt init` as the more deliberate signal, so gh-stack answers first now — but +unlike the old "Graphite wins" rule, Graphite's tracked branches are never hidden: `current_stack` +and `enumerate_stacks` fall back to the secondary provider per branch (see +`StackModel::providers`), and `assemble_changesets`'s `Mixed` arm reads whichever provider's +metadata actually tracks the head branch. The escape hatch out of `Mixed`, or out of a tie-break +you disagree with, is an explicit `workon.stackModel = graphite`/`gh-stack` (strict, no fallback) +or `workon.stackModel = mixed:graphite`/`mixed:gh-stack` (pins `Mixed`'s primary directly, +skipping the liveness read); none of these are ever reachable via `auto`. `doctor`'s +`BothStackToolsDetected` check reports the dual state, what `auto` resolved to, and each +provider's liveness; a `StackModelPinHidesLiveBranches` check (and a matching `list` stderr hint) +fires when an explicit pin hides the other provider's live branches. ## The shared canonical file diff --git a/docs/recipes/stacked-diffs.md b/docs/recipes/stacked-diffs.md index b351007f..9dfc30d6 100644 --- a/docs/recipes/stacked-diffs.md +++ b/docs/recipes/stacked-diffs.md @@ -5,8 +5,9 @@ git-workon integrates with [Graphite](https://graphite.dev) and with workflows. When either tool is active, `list`, `find`, and `new` all become stack-aware by default. Use `--no-stack` on any invocation to fall back to branch-flat behavior. -If both tools' artifacts are present in the same repository, Graphite wins: see -"Auto-detection and both tools present" below. +If both tools' artifacts are present in the same repository, `auto` ties on which one has +tracked branches — using both at once is supported: see "Auto-detection and both tools present" +below. ## Setup @@ -46,17 +47,43 @@ git config workon.stackModel none ### Auto-detection and both tools present -`workon.stackModel = auto` (the default) checks Graphite's artifacts first, then gh-stack's: -`.graphite_repo_config` comes from an explicit, repo-wide `gt init`, while a `gh-stack` file can -appear from a single `gh stack add` run in one worktree, so the more deliberate signal wins. If -you've tried both tools in the same repository and want gh-stack instead, pin it explicitly: +`workon.stackModel = auto` (the default) checks artifacts first — `.graphite_repo_config`/ +`.graphite_metadata.db` for Graphite, a `gh-stack` file anywhere for gh-stack. With only one +present, that tool is used, same as ever. With **both** present, `auto` checks which one has +live, ref-backed tracked branches: + +- **Both have tracked branches** — you're using both tools in the same repository (e.g. one + stack under Graphite, another under `gh stack`). `auto` resolves to a mixed mode, gh-stack + first: gh-stack answers for any branch, Graphite answers for branches gh-stack doesn't know + about. `list` shows both stacks; nothing is hidden. +- **Only one has tracked branches** — the other tool's artifacts are stale (e.g. you ran + `gt init` once and never tracked a branch, or migrated off it). `auto` uses the one with + tracked branches. +- **Neither has tracked branches** — an ambiguous, doubly-stale state; `auto` falls back to + gh-stack and `git workon doctor` explains why. + +If you'd rather pin one tool explicitly and stop `auto` from ever falling back to the other, +set it directly: ```bash git config workon.stackModel gh-stack ``` -`git workon doctor` reports `BothStackToolsDetected` when it sees artifacts for both, so you -know when `auto` made a call you might want to override. +An explicit pin is strict: it never falls back to the other provider, even if that provider also +has tracked branches. If it does, `list` prints a dimmed stderr hint and `git workon doctor` +reports `StackModelPinHidesLiveBranches`, so you know branches are being hidden on purpose (or +by accident). `git workon doctor` also reports `BothStackToolsDetected` whenever both artifacts +are present under `auto`, showing what it resolved to and each provider's liveness. + +You can also pin the mixed mode itself, naming its primary directly and skipping the liveness +read entirely: + +```bash +git config workon.stackModel mixed:graphite # or mixed:gh-stack +``` + +This is useful when you know you want the fallback behavior but disagree with which provider +`auto` would pick as primary. ## Worktree per stack