fix(review): honor workon.stackModel over stack auto-detection

git-workon-review/src/acquire.rs

@@ -6,7 +6,9 @@
 //! git2 diffs and then a [`DiffModel`].
 
 use git2::{DiffFindOptions, DiffOptions, Oid, Repository};
-use workon::{assemble_changesets, Changeset, ChangesetSpan, StackModel, UncommittedLayer};
+use workon::{
+    assemble_changesets, Changeset, ChangesetSpan, StackModel, UncommittedLayer, WorkonConfig,
+};
 
 use crate::error::DiffError;
 use crate::model::DiffModel;
@@ -200,10 +202,22 @@ pub fn diff_changesets(
         .collect()
 }
 
+/// The stack provider in effect for `repo`: `workon.stackModel` when set, else auto-detect
+/// (see [`WorkonConfig::stack_model`] for the precedence). Every stack-model decision in this
+/// crate goes through here so an explicit config always beats [`StackModel::detect`]'s
+/// Graphite-wins tiebreak, which otherwise blinds a gh-stack repo whose Graphite artifacts
+/// linger in the common dir. A config read error or an unrecognized value falls back to
+/// detection: the model only picks which metadata to read, so it is not worth failing over.
+pub fn stack_model(repo: &Repository) -> StackModel {
+    WorkonConfig::new(repo)
+        .and_then(|config| config.stack_model(None))
+        .unwrap_or_else(|_| StackModel::detect(repo))
+}
+
 /// Resolve the changeset stack the review App opens on for the worktree whose `HEAD` is
-/// `head_branch` (locked design decision: auto-detect Graphite, else a single uncommitted
-/// changeset): the full Graphite stack
-/// when one is active, or a single synthetic [`Changeset`] spanning the uncommitted worktree
+/// `head_branch` (locked design decision: auto-detect a stack tool, else a single uncommitted
+/// changeset): the full stack when a metadata-backed model (Graphite or gh-stack) is active
+/// per [`stack_model`], or a single synthetic [`Changeset`] spanning the uncommitted worktree
 /// otherwise.
 ///
 /// This does NOT simply forward to [`workon::assemble_changesets`] with the detected
@@ -219,7 +233,7 @@ pub fn resolve_changesets(
     repo: &Repository,
     head_branch: &str,
 ) -> Result<Vec<Changeset>, DiffError> {
-    match StackModel::detect(repo) {
+    match stack_model(repo) {
         model @ (StackModel::Graphite | StackModel::GhStack) => Ok(assemble_changesets(
             repo,
             head_branch,

git-workon-review/src/source.rs

@@ -22,7 +22,7 @@ use workon::{
     PrMetadata, StackModel, UncommittedLayer, WorkonError,
 };
 
-use crate::acquire::uncommitted_changeset;
+use crate::acquire::{stack_model, uncommitted_changeset};
 use crate::error::SourceError;
 
 /// Which dot form a [`Source::Range`] was spelled with — git-diff semantics differ (ADR-036).
@@ -214,17 +214,17 @@ fn remote_branch_tip(repo: &Repository, remote: &str, branch: &str) -> Option<Oi
         .and_then(|b| b.get().target())
 }
 
-/// `stack` keyword resolution: stack metadata (Graphite or gh-stack) when active, otherwise the
-/// git-inference arm (`StackModel::Git`, first wired into the binary here) — never a silent
-/// downgrade to `StackModel::None`'s empty result, since the keyword is an explicit ask for the
-/// real stack.
+/// `stack` keyword resolution: stack metadata (Graphite or gh-stack) when active per
+/// [`stack_model`] (config first, then detection), otherwise the git-inference arm
+/// (`StackModel::Git`, first wired into the binary here) — never a silent downgrade to
+/// `StackModel::None`'s empty result, since the keyword is an explicit ask for the real stack.
 /// The uncommitted layer rides along (`UncommittedLayer::Include`): `stack` always means
 /// "focused on real `HEAD`."
 fn resolve_stack(
     repo: &Repository,
     head_branch: &str,
 ) -> Result<Vec<workon::Changeset>, SourceError> {
-    let model = match StackModel::detect(repo) {
+    let model = match stack_model(repo) {
         StackModel::None | StackModel::Git => StackModel::Git,
         metadata @ (StackModel::Graphite | StackModel::GhStack) => metadata,
     };
@@ -250,8 +250,9 @@ fn map_assemble_err(branch: &str) -> impl Fn(WorkonError) -> SourceError + '_ {
 
 /// `<ref>` resolution — shape-aware dispatch (ADR-036), checked in order:
 ///
-/// 1. A Graphite-tracked LOCAL branch (`text` names a local branch, qualified spellings like
-///    `refs/heads/<name>`/`heads/<name>` included, AND that branch has a Graphite metadata row)
+/// 1. A stack-tracked LOCAL branch (`text` names a local branch, qualified spellings like
+///    `refs/heads/<name>`/`heads/<name>` included, AND that branch has a metadata row in the
+///    active provider per [`stack_model`], Graphite or gh-stack)
 ///    → the whole stack focused there, exactly like `stack` but pinned to `text`'s branch
 ///    instead of real `HEAD`. The uncommitted layer rides along only when the resolved branch
 ///    IS `head_branch` — this is the first caller to pass [`UncommittedLayer::Omit`].
@@ -269,17 +270,19 @@ fn resolve_ref(
     text: String,
 ) -> Result<Vec<workon::Changeset>, SourceError> {
     if let Some(branch_name) = resolve_local_branch_name(repo, &text) {
-        if workon::current_stack(repo, &branch_name, StackModel::Graphite)
-            .ok()
-            .flatten()
-            .is_some()
+        let model = stack_model(repo);
+        if matches!(model, StackModel::Graphite | StackModel::GhStack)
+            && workon::current_stack(repo, &branch_name, model)
+                .ok()
+                .flatten()
+                .is_some()
         {
             let layer = if branch_name == head_branch {
                 UncommittedLayer::Include
             } else {
                 UncommittedLayer::Omit
             };
-            return assemble_changesets(repo, &branch_name, StackModel::Graphite, layer)
+            return assemble_changesets(repo, &branch_name, model, layer)
                 .map_err(map_assemble_err(&branch_name));
         }
 

git-workon-review/tests/suite/source.rs

@@ -468,3 +468,80 @@ fn empty_range_between_same_tag_prints_named_nothing_to_review_and_exits_zero()
         .success()
         .stderr(predicate::str::contains("nothing to review in v1..v1"));
 }
+
+// `workon.stackModel` must beat auto-detection. `StackModel::detect` picks Graphite whenever a
+// `.graphite_repo_config` exists, so a gh-stack repo whose Graphite artifacts linger reads the
+// wrong (empty) metadata and the stack branch falls through to git inference with no upstream.
+// Each of the three stack-model decisions in the binary is pinned separately.
+
+fn lingering_graphite_over_gh_stack() -> Result<Fixture, Box<dyn Error>> {
+    let fixture = FixtureBuilder::new()
+        .graphite_config(&["main"])
+        .gh_stack(None, 1, "main", &["a", "b", "c"])
+        .config("workon.stackModel", "gh-stack")
+        .build()?;
+    Ok(fixture)
+}
+
+fn names(changesets: &[workon::Changeset]) -> Vec<&str> {
+    changesets.iter().map(|c| c.name.as_str()).collect()
+}
+
+#[test]
+fn stack_keyword_honors_stack_model_config_over_lingering_graphite_artifacts(
+) -> Result<(), Box<dyn Error>> {
+    let fixture = lingering_graphite_over_gh_stack()?;
+    let repo = fixture.repo()?;
+
+    let changesets = resolve_source(repo, "b", Source::classify("stack"))?;
+    assert_eq!(names(&changesets), vec!["a", "b", "c"]);
+    Ok(())
+}
+
+#[test]
+fn auto_detect_honors_stack_model_config_over_lingering_graphite_artifacts(
+) -> Result<(), Box<dyn Error>> {
+    let fixture = lingering_graphite_over_gh_stack()?;
+    let repo = fixture.repo()?;
+
+    let changesets = resolve_changesets(repo, "b")?;
+    assert_eq!(names(&changesets), vec!["a", "b", "c"]);
+    Ok(())
+}
+
+#[test]
+fn ref_on_gh_stack_tracked_branch_resolves_the_whole_stack_under_stack_model_config(
+) -> Result<(), Box<dyn Error>> {
+    let fixture = lingering_graphite_over_gh_stack()?;
+    let repo = fixture.repo()?;
+
+    let changesets = resolve_source(repo, "some-other-branch", Source::classify("b"))?;
+    assert_eq!(names(&changesets), vec!["a", "b", "c"]);
+    let current: Vec<&str> = changesets
+        .iter()
+        .filter(|c| c.current)
+        .map(|c| c.name.as_str())
+        .collect();
+    assert_eq!(current, vec!["b"]);
+    Ok(())
+}
+
+/// Without the config, detection's Graphite-wins tiebreak still applies and the gh-stack
+/// branch has no Graphite row: the `stack` keyword reports the missing upstream. Documents the
+/// trap the config escape hatch exists for.
+#[test]
+fn stack_keyword_without_stack_model_config_still_loses_to_lingering_graphite_artifacts(
+) -> Result<(), Box<dyn Error>> {
+    let fixture = FixtureBuilder::new()
+        .graphite_config(&["main"])
+        .gh_stack(None, 1, "main", &["a", "b", "c"])
+        .build()?;
+    let repo = fixture.repo()?;
+
+    let err = resolve_source(repo, "b", Source::classify("stack")).unwrap_err();
+    assert!(
+        matches!(err, SourceError::NoUpstream { ref branch } if branch == "b"),
+        "{err:?}"
+    );
+    Ok(())
+}