Merge pull request #4161 from vitorpamplona/claude/tender-goldberg-6xv9hx

Fix nav-bar tab switching to restore stacks from other tabs

amethyst/src/main/java/com/vitorpamplona/amethyst/ui/navigation/navs/Nav.kt

@@ -30,6 +30,7 @@ import androidx.compose.runtime.mutableStateOf
 import androidx.compose.runtime.setValue
 import androidx.lifecycle.viewmodel.compose.LocalViewModelStoreOwner
 import androidx.navigation.NavBackStackEntry
+import androidx.navigation.NavDestination.Companion.hasRoute
 import androidx.navigation.NavGraph.Companion.findStartDestination
 import androidx.navigation.NavHostController
 import com.vitorpamplona.amethyst.ui.navigation.BOTTOM_NAV_ROOT_KEY
@@ -102,6 +103,22 @@ class Nav(
     override fun navBottomBar(route: Route) {
         navigationScope.launch {
             ime.settle()
+
+            // A nav-bar tap asks for a tab, never for whatever the user pushed on top of one. Drop
+            // those pushes first, and without saving them, so the restoreState below can never hand
+            // a deep stack back. On phones this is always a no-op — AppBottomBar hides itself off
+            // tab roots, so the bar is only ever tapped from one — but the large-screen rail stays
+            // on screen the whole time and is routinely tapped from three screens deep.
+            popPushesAboveTabRoot()
+
+            // Dropping those pushes is often the whole job — re-tapping the tab the user is inside,
+            // and every tap of Home from somewhere inside Home, end here. Same already-there guard
+            // nav() uses, so a parameterized tab compares by its filled route.
+            if (getRouteWithArguments(route::class, controller) == route) {
+                controller.currentBackStackEntry?.savedStateHandle?.set(BOTTOM_NAV_ROOT_KEY, true)
+                return@launch
+            }
+
             controller.navigate(route) {
                 // Clear sibling bottom-nav entries but keep Home (the start
                 // destination) below, so back-swipe from any tab returns to
@@ -117,7 +134,15 @@ class Nav(
                     saveState = true
                 }
                 launchSingleTop = true
-                restoreState = true
+                // ...but never onto Home. A non-inclusive popUpTo files the popped entries under the
+                // popUpTo TARGET's own destination id as well as the popped tab's — the
+                // `if (!inclusive)` branch of NavControllerImpl.executePopOperations — so restoring
+                // here would hand Home the stack that was saved on the way out of some *other* tab.
+                // That is how tapping Home on the rail came back to a thread instead of the feed.
+                // Home is the anchor, so it is never popped and has no saved stack of its own to
+                // miss: launchSingleTop reuses the entry already sitting there, ViewModelStore and
+                // all.
+                restoreState = route != Route.Home
             }
             // Mark this entry as a tab root: hides the back arrow in canPop
             // and skips the horizontal slide in composableFromEnd.
@@ -146,6 +171,25 @@ class Nav(
         }
     }
 
+    /**
+     * Drops every entry the user pushed on top of the tab root they are currently in, discarding
+     * that state rather than saving it — a nav-bar tap is a request for a tab, and nothing above a
+     * tab root should survive to be replayed by a later `restoreState`.
+     *
+     * Stops at the first entry stamped [BOTTOM_NAV_ROOT_KEY] by [navBottomBar], or at Home, which
+     * is a tab root the user may never have tapped because the graph starts there. Terminates
+     * either way: every iteration that does not return removes one entry from a finite back stack,
+     * and [NavHostController.popBackStack] reports false once there is nothing left to pop.
+     */
+    private fun popPushesAboveTabRoot() {
+        while (true) {
+            val top = controller.currentBackStackEntry ?: return
+            if (top.isBottomNavRoot()) return
+            if (top.destination.hasRoute(Route.Home::class)) return
+            if (!controller.popBackStack()) return
+        }
+    }
+
     @Composable
     override fun canPop(): Boolean {
         // Decide the back arrow / bottom-bar visibility from THIS screen's own

amethyst/src/test/java/com/vitorpamplona/amethyst/ui/navigation/NavBottomBarStackTest.kt

@@ -0,0 +1,116 @@
+/*
+ * Copyright (c) 2025 Vitor Pamplona
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy of
+ * this software and associated documentation files (the "Software"), to deal in
+ * the Software without restriction, including without limitation the rights to use,
+ * copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the
+ * Software, and to permit persons to whom the Software is furnished to do so,
+ * subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in all
+ * copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS
+ * FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR
+ * COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN
+ * AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION
+ * WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
+ */
+package com.vitorpamplona.amethyst.ui.navigation
+
+import androidx.navigation.NavBackStackEntry
+import androidx.navigation.NavHostController
+import androidx.navigation.NavOptions
+import androidx.navigation.NavOptionsBuilder
+import androidx.navigation.navOptions
+import com.vitorpamplona.amethyst.ui.navigation.navs.Nav
+import com.vitorpamplona.amethyst.ui.navigation.routes.Route
+import io.mockk.every
+import io.mockk.mockk
+import io.mockk.slot
+import io.mockk.verify
+import kotlinx.coroutines.ExperimentalCoroutinesApi
+import kotlinx.coroutines.test.TestScope
+import kotlinx.coroutines.test.advanceUntilIdle
+import kotlinx.coroutines.test.runTest
+import org.junit.Assert.assertFalse
+import org.junit.Assert.assertTrue
+import org.junit.Test
+
+/**
+ * A nav-bar tap must land on the tab itself, never on whatever the user had pushed on top of one.
+ *
+ * The phone bottom bar gets this for free — it hides itself off tab roots, so it can only ever be
+ * tapped from one. The large-screen navigation rail stays on screen the whole time, which is where
+ * the gap showed: tapping Home from a thread three screens deep came back to that thread instead of
+ * the feed.
+ *
+ * Two rules keep that from happening, one per test below:
+ *
+ * 1. Pushes above a tab root are dropped, unsaved, on the way out — anything saved is eligible to be
+ *    replayed by a later `restoreState`.
+ * 2. Home is never restored onto. `popUpTo(Home) { inclusive = false; saveState = true }` files the
+ *    popped entries under the popUpTo target's own destination id as well as the popped tab's — see
+ *    the `if (!inclusive)` branch of `NavControllerImpl.executePopOperations` — so
+ *    `navigate(Home) { restoreState = true }` handed Home back the stack the user had left behind in
+ *    some *other* tab. Every other tab restores as before; that is what keeps its ViewModelStore.
+ */
+@OptIn(ExperimentalCoroutinesApi::class)
+class NavBottomBarStackTest {
+    /** A back-stack entry that is, or isn't, a tab root as far as [isBottomNavRoot] can tell. */
+    private fun entry(isTabRoot: Boolean): NavBackStackEntry =
+        mockk<NavBackStackEntry>(relaxed = true) {
+            every { savedStateHandle.get<Boolean>(BOTTOM_NAV_ROOT_KEY) } returns isTabRoot
+        }
+
+    /**
+     * Taps [route] on a controller whose back stack holds none of the nav-bar destinations, and
+     * returns the options the resulting navigate was built with.
+     */
+    private fun TestScope.optionsForTapping(route: Route): NavOptions {
+        val options = slot<NavOptionsBuilder.() -> Unit>()
+        val controller =
+            mockk<NavHostController>(relaxed = true) {
+                every { currentBackStackEntry } returns entry(isTabRoot = true)
+                every { navigate(route, capture(options)) } returns Unit
+            }
+
+        Nav(controller, this).navBottomBar(route)
+        advanceUntilIdle()
+
+        return navOptions(options.captured)
+    }
+
+    @Test
+    fun dropsWhatTheUserPushedOnTopOfTheTabBeforeSwitchingTabs() =
+        runTest {
+            // Home > Note > Profile: two pushes sitting on a tab root.
+            val controller =
+                mockk<NavHostController>(relaxed = true) {
+                    every { currentBackStackEntry } returnsMany
+                        listOf(entry(isTabRoot = false), entry(isTabRoot = false), entry(isTabRoot = true))
+                    every { popBackStack() } returns true
+                }
+
+            Nav(controller, this).navBottomBar(Route.Message)
+            advanceUntilIdle()
+
+            // Exactly the two pushes and no more: the tab root itself stays, so the navigate below
+            // saves a tab root rather than a branch that could be replayed later.
+            verify(exactly = 2) { controller.popBackStack() }
+        }
+
+    @Test
+    fun neverRestoresASavedStackOntoHome() =
+        runTest {
+            assertFalse(optionsForTapping(Route.Home).shouldRestoreState())
+        }
+
+    @Test
+    fun restoresTheSavedStackOfEveryOtherTab() =
+        runTest {
+            assertTrue(optionsForTapping(Route.Message).shouldRestoreState())
+        }
+}