fix(mobile): the shell returns to a screen already on its stack (#32110)

* fix(mobile): the shell returns to a screen already on its stack

The page's own "back to hosts" chevron calls dismissTo('/') inside the page, which the page's
router hands to the shell as one navigate message. The shell pushed every such href, so leaving
a host from its page left [Home, Host, Home] with the old host page still mounted (WebView and
live subscriptions), and every round trip stacked another one. System Back then walked back
through stale host pages.

The shell now reads the navigation container's state first: an href that is already a screen
below this one (same path and query) is popped back to, and an href that is this screen is
dropped. Anything else is pushed as before. The pop names its navigator and source route
explicitly, because a screen's dispatch otherwise stamps its own key as the source and a parent
stack that cannot find it drops the pop.

The wide host-route dismissTo stays for the hrefs this rule does not cover: a host route whose
query differs from the one below (the missing-worktree bounce), and a cold deep link with no host
route below to return to. Overlay replace is unchanged, and so is the bridge message.

* fix(mobile): a param cleared to empty and a fragment still match a screen on the stack

The session screen clears paneKey with setParams({ paneKey: '' }), which leaves ?paneKey= on the
stack entry, so a later navigate to the plain session href found no match and pushed a duplicate.
Empty query values are now dropped when canonicalising, and the fragment is stripped since
expo-router keeps it as a param no href spells the same way.

mobile/src/mobile-web-shell/MobileWebShellScreen.tsx

@@ -1,6 +1,6 @@
 import { useEffect, useState, type ReactNode } from 'react'
 import { Linking, Pressable, StyleSheet, Text, View } from 'react-native'
-import { useNavigation, useRouter } from 'expo-router'
+import { useNavigation, useNavigationContainerRef, useRouter } from 'expo-router'
 import { useSafeAreaInsets } from 'react-native-safe-area-context'
 import {
   OrcaMobileWebShellView,
@@ -30,6 +30,7 @@ import { useMobileWebShellSession } from './use-mobile-web-shell-session'
 import { usePageHostSnapshot } from './use-page-host-snapshot'
 import { useReportedHostAreaServing } from './host-area-serving'
 import { isHostRouteOf } from './page-route-policy'
+import { nativeStackReturn } from './native-stack-return'
 import { PageRouteUnavailableScreen } from './PageRouteUnavailableScreen'
 import { useResponsiveLayout } from '../layout/responsive-layout'
 import { storageReadParamsSchema, type BridgeNativeVerb } from './bridge/bridge-native-verbs'
@@ -142,6 +143,7 @@ export function MobileWebShellScreen({
   const overlay = onOverlayLoad !== undefined
   const router = useRouter()
   const navigation = useNavigation()
+  const navigationContainer = useNavigationContainerRef()
   const popShellStack = useShellStackPop()
   const { isWideLayout } = useResponsiveLayout()
   const { droppedBinaryFrames, reportDroppedBinaryFrames } = useMobileWebShellDroppedFrames()
@@ -252,15 +254,32 @@ export function MobileWebShellScreen({
       console.warn('[web-shell] refused to open this screen', issue)
       reportShellFailure('document-load-failed')
     },
-    // Pushed, never replaced: the page stays mounted underneath, so Back reveals it with no
-    // download and no second `init`.
+    // A new screen is pushed, never replaced: the page stays mounted underneath, so Back reveals it
+    // with no download and no second `init`.
     onNavigate: (href: string) => {
       // An overlay hands its place to where the page went: Back there returns to the screen beneath.
       if (overlay) {
         return void router.replace(href)
       }
-      // Wide, this host's route is the area-owning session below: back to it, or in place of this
-      // one from a cold deep link, never a second on top. `dismissTo` takes the href's params.
+      // A screen already on the stack is returned to, as system Back would: the page's own exits
+      // ask for the screen beneath it, and a push would stack a second copy over a live one.
+      const rootState = navigationContainer.getRootState()
+      const onStack = rootState ? nativeStackReturn(rootState, href) : null
+      if (onStack === 'current') {
+        return
+      }
+      if (onStack) {
+        navigation.dispatch({
+          type: 'POP',
+          payload: { count: onStack.count },
+          target: onStack.target,
+          source: onStack.source
+        })
+        return
+      }
+      // Wide, this host's route is the area-owning session even when it is not on the stack as
+      // asked: `dismissTo` pops to it by name with the href's params, or replaces this one after a
+      // cold deep link, so it is never a second on top.
       if (isWideLayout && isHostRouteOf(href, hostId)) {
         router.dismissTo(href)
         return

mobile/src/mobile-web-shell/catch-all-page-route-states.test.tsx

@@ -102,6 +102,7 @@ vi.mock('expo-router', () => ({
   }),
   usePathname: () => '/h/host-1',
   useNavigation: () => ({ setOptions: vi.fn() }),
+  useNavigationContainerRef: () => ({ getRootState: () => undefined }),
   useFocusEffect: () => {}
 }))
 vi.mock('../../modules/orca-mobile-web-shell/src', async () => {

mobile/src/mobile-web-shell/host-area-leave-session.test.tsx

@@ -1,8 +1,8 @@
 /**
- * Leaving a detail session for the host route on a wide layout: the page's back chevron
- * (`leaveSession`, which replaces to `/h/<host>` when nothing is below it) and the missing-worktree
- * bounce (`/h/<host>?notice=…`). Under a page that owns the host area that hop reaches the shell,
- * which must land on one host route rather than stack a second host-area session on the session.
+ * Leaving a detail session for the host route: the page's back chevron (`leaveSession`, which
+ * replaces to `/h/<host>` when nothing is below it) and the missing-worktree bounce
+ * (`/h/<host>?notice=…`). That hop reaches the shell, which must land on one host route rather than
+ * stack a second host-area session on the session.
  */
 import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
 
@@ -44,6 +44,7 @@ import { createElement } from 'react'
 import { act, create, type ReactTestRenderer } from 'react-test-renderer'
 import { clientFrame, createFakeRpcClient } from './bridge-host-test-fakes'
 import { MobileWebShellScreen } from './MobileWebShellScreen'
+import type { NativeStackState } from './native-stack-return'
 
 const HOST = '/h/host-1'
 const DETAIL = '/h/host-1/session/wt-1'
@@ -54,16 +55,38 @@ const PHONE = { width: 390, height: 844 }
 /** A stack entry: the route's pathname and its search, as the native stack holds them. */
 type Entry = string
 
+/** The entries as the navigation container holds them: the host stack inside the root one. */
+function rootStateOf(stack: readonly Entry[]): NativeStackState {
+  const routes = stack.map((entry, index) => {
+    const [path = '', search = ''] = entry.split('?')
+    const [, , hostId = '', area, worktreeId] = path.split('/')
+    const query = Object.fromEntries(new URLSearchParams(search))
+    return area === undefined
+      ? { key: `entry-${index}`, name: '[hostId]/index', params: { hostId, ...query } }
+      : {
+          key: `entry-${index}`,
+          name: `[hostId]/${area}/[worktreeId]`,
+          params: { hostId, worktreeId: worktreeId ?? '', ...query }
+        }
+  })
+  const hostStack = { key: 'host-stack', index: routes.length - 1, routes }
+  return { key: 'root-stack', index: 0, routes: [{ key: 'h', name: 'h', state: hostStack }] }
+}
+
 /**
- * The native stack the screen's calls act on, with expo-router 55's semantics: `push` adds,
- * `dismissTo` is react-navigation's POP_TO — pop back to the nearest entry of that route and take
- * the new params, or replace the current entry when there is none below it.
+ * The native stack the screen's calls act on, with expo-router 55's semantics: `push` adds, a
+ * dispatched `POP` removes that many, and `dismissTo` is react-navigation's POP_TO — pop back to
+ * the nearest entry of that route and take the new params, or replace the current entry when there
+ * is none below it.
  */
 function applied(stack: readonly Entry[]): Entry[] {
   const next = [...stack]
   for (const [href] of dependencies.push.mock.calls) {
     next.push(String(href))
   }
+  for (const [action] of dependencies.dispatch.mock.calls) {
+    next.splice(next.length - action.payload.count)
+  }
   for (const [href] of device.dismissTo.mock.calls) {
     const target = String(href)
     const route = target.split('?')[0]
@@ -78,9 +101,14 @@ function applied(stack: readonly Entry[]): Entry[] {
 }
 
 /** The session page on screen, asking the shell for `href`. */
-async function hopFromSession(viewport: { width: number; height: number }, href: string) {
+async function hopFromSession(
+  viewport: { width: number; height: number },
+  stack: readonly Entry[],
+  href: string
+) {
   device.width = viewport.width
   device.height = viewport.height
+  dependencies.rootState = rootStateOf(stack)
   dependencies.client = createFakeRpcClient()
   dependencies.state = harness.readyState('session-one')
   dependencies.pageRoutes = ['/h/[hostId]', '/h/[hostId]/session/[worktreeId]']
@@ -114,25 +142,31 @@ afterEach(() => {
 
 describe('leaving a wide detail session for its host route', () => {
   it('returns to the host-area session below it, so Back does not reopen the session', async () => {
-    await hopFromSession(IPAD, HOST)
+    await hopFromSession(IPAD, [HOST, DETAIL], HOST)
     expect(applied([HOST, DETAIL])).toEqual([HOST])
   })
 
   it('carries the bounce notice to that host route, and still stacks nothing', async () => {
-    await hopFromSession(IPAD, BOUNCE)
+    await hopFromSession(IPAD, [HOST, DETAIL], BOUNCE)
     expect(applied([HOST, DETAIL])).toEqual([BOUNCE])
   })
 
   it('lands on one host route from a cold deep link, with nothing below the session', async () => {
-    await hopFromSession(IPAD, BOUNCE)
+    await hopFromSession(IPAD, [DETAIL], BOUNCE)
     expect(applied([DETAIL])).toEqual([BOUNCE])
   })
 })
 
 describe('the same hop on a phone', () => {
-  it('pushes, as it always has', async () => {
-    await hopFromSession(PHONE, HOST)
+  it('returns to the host route below rather than stacking a second one', async () => {
+    await hopFromSession(PHONE, [HOST, DETAIL], HOST)
     expect(device.dismissTo).not.toHaveBeenCalled()
-    expect(applied([HOST, DETAIL])).toEqual([HOST, DETAIL, HOST])
+    expect(applied([HOST, DETAIL])).toEqual([HOST])
+  })
+
+  it('pushes a route the stack does not hold, such as another worktree', async () => {
+    const other = '/h/host-1/session/wt-2'
+    await hopFromSession(PHONE, [HOST, DETAIL], other)
+    expect(applied([HOST, DETAIL])).toEqual([HOST, DETAIL, other])
   })
 })

mobile/src/mobile-web-shell/mobile-web-shell-screen-test-harness.tsx

@@ -12,6 +12,7 @@ import type {
   MobileWebShellUpdateNotice
 } from './mobile-web-shell-session-contract'
 import type { ShellPageFrame } from './shell-page-frame'
+import type { NativeStackState } from './native-stack-return'
 
 /**
  * Everything the screen's cases mock away, as one mutable record, plus the helpers that mount it.
@@ -33,6 +34,10 @@ export type ScreenDependencies = {
   back: Mock
   /** What the native stack answers: false is a page opened as the first screen on it. */
   canGoBack: boolean
+  /** The navigation container's whole state; undefined before it is ready. */
+  rootState: NativeStackState | undefined
+  /** Actions the screen dispatched onto its own place on the stack. */
+  dispatch: Mock
   pathname: string
   pageRoutes: readonly string[]
   routeGrants: readonly string[]
@@ -103,6 +108,8 @@ export function createScreenDependencies(): ScreenDependencies {
     push: vi.fn(),
     back: vi.fn(),
     canGoBack: true,
+    rootState: undefined,
+    dispatch: vi.fn(),
     pathname: '/h/host-1',
     pageRoutes: ['/h/[hostId]'],
     routeGrants: DEFAULT_ROUTE_GRANTS,
@@ -154,6 +161,8 @@ export function resetScreenDependencies(dependencies: ScreenDependencies): void
   dependencies.openUrl.mockReset()
   dependencies.openUrl.mockImplementation(() => Promise.resolve(true))
   dependencies.canGoBack = true
+  dependencies.rootState = undefined
+  dependencies.dispatch.mockReset()
   dependencies.pathname = '/h/host-1'
   dependencies.updateNotice = null
   dependencies.backHandlers.clear()

mobile/src/mobile-web-shell/mobile-web-shell-screen-test-mocks.tsx

@@ -117,7 +117,11 @@ export function screenModuleMocks(dependencies: ScreenDependencies) {
       // Read by the pop latch, which clears on the route this shell is mounted at changing.
       usePathname: () => dependencies.pathname,
       // The screen's own place on the stack, which is where the iOS swipe-back is taken away.
-      useNavigation: () => ({ setOptions: dependencies.setScreenOptions }),
+      useNavigation: () => ({
+        setOptions: dependencies.setScreenOptions,
+        dispatch: dependencies.dispatch
+      }),
+      useNavigationContainerRef: () => ({ getRootState: () => dependencies.rootState }),
       useFocusEffect: (effect: () => undefined | (() => void)) => React.useEffect(effect, [effect])
     }),
     // A component rather than a host string: the React key is what makes a retry a rebuilt WebView,

mobile/src/mobile-web-shell/native-stack-return.test.ts

@@ -0,0 +1,70 @@
+import { describe, expect, it } from 'vitest'
+import { nativeStackReturn, type NativeStackState } from './native-stack-return'
+
+type Route = NativeStackState['routes'][number]
+
+function stack(key: string, routes: readonly Route[]): NativeStackState {
+  return {
+    key,
+    index: routes.length - 1,
+    routes: routes.map((route, index) => ({ ...route, key: `${key}/${index}` }))
+  }
+}
+
+function hostStack(...routes: Route[]): Route {
+  return { name: 'h', state: stack('host-stack', routes) }
+}
+
+// The root layout is a screen of expo-router's own navigator, so the walk starts one level up.
+function rootStack(...routes: Route[]): NativeStackState {
+  return stack('expo-root', [{ name: '__root', state: stack('root-stack', routes) }])
+}
+
+const HOME = { name: 'index' }
+const HOST = { name: '[hostId]/index', params: { hostId: 'mac/1' } }
+const SESSION = {
+  name: '[hostId]/session/[worktreeId]',
+  params: { hostId: 'mac/1', worktreeId: 'wt-a' }
+}
+
+describe('nativeStackReturn', () => {
+  it('pops back to Home under the host instead of stacking a second Home', () => {
+    expect(nativeStackReturn(rootStack(HOME, hostStack(HOST)), '/')).toEqual({
+      target: 'root-stack',
+      source: 'root-stack/1',
+      count: 1
+    })
+  })
+
+  it('finds nothing when the host is the root, as after a cold deep link', () => {
+    expect(nativeStackReturn(rootStack(hostStack(HOST)), '/')).toBeNull()
+  })
+
+  it('returns to an exact path and query, however the href spells it', () => {
+    const files = { name: '[hostId]/files/[worktreeId]', params: SESSION.params }
+    const tabbed = { ...SESSION, params: { ...SESSION.params, tab: 'b', pane: '1' } }
+    const state = rootStack(HOME, hostStack(HOST, tabbed, files))
+    expect(nativeStackReturn(state, '/h/mac%2F1/session/wt-a?pane=1&tab=b')).toEqual({
+      target: 'host-stack',
+      source: 'host-stack/2',
+      count: 1
+    })
+    expect(nativeStackReturn(state, '/h/mac%2F1')).toMatchObject({ target: 'host-stack', count: 2 })
+    expect(nativeStackReturn(state, '/h/mac%2F1/session/wt-a?pane=1&tab=c')).toBeNull()
+    expect(nativeStackReturn(state, '/h/mac%2F1/session/wt-b?pane=1&tab=b')).toBeNull()
+  })
+
+  it('treats a param cleared to empty as absent', () => {
+    const cleared = { ...SESSION, params: { ...SESSION.params, paneKey: '' } }
+    const state = rootStack(
+      HOME,
+      hostStack(HOST, cleared, { name: '[hostId]/files/[worktreeId]', params: SESSION.params })
+    )
+    expect(nativeStackReturn(state, '/h/mac%2F1/session/wt-a')).toMatchObject({ count: 1 })
+  })
+
+  it('names the focused screen as current rather than a pop', () => {
+    const state = rootStack(HOME, hostStack(HOST, SESSION))
+    expect(nativeStackReturn(state, '/h/mac%2F1/session/wt-a')).toBe('current')
+  })
+})

mobile/src/mobile-web-shell/native-stack-return.ts

@@ -0,0 +1,116 @@
+import { stringifyRouteHref } from '../navigation/route-href'
+
+export type NativeStackState = Readonly<{
+  key?: string
+  index?: number
+  routes: readonly NativeStackRoute[]
+}>
+
+type NativeStackRoute = Readonly<{
+  key?: string
+  name: string
+  params?: object
+  state?: NativeStackState
+}>
+
+/** A pop of `count` screens off the navigator keyed `target`, counted from its route `source`.
+ *  `source` is explicit because a screen's `dispatch` stamps its own key there otherwise, and a parent
+ *  navigator that cannot find that key among its routes drops the pop. */
+export type NativeStackReturn = Readonly<{ target: string; source: string; count: number }>
+
+/** Expo-router file segments that name no part of the URL: `index`, `(group)` and its own `__root`. */
+function isUrlSegment(segment: string): boolean {
+  return segment !== 'index' && !/^\(.*\)$/.test(segment) && !segment.startsWith('__')
+}
+
+/** A state not yet rehydrated may omit `index`; React Navigation reads that as its last route. */
+function focusedIndex(state: NativeStackState): number {
+  return state.index ?? state.routes.length - 1
+}
+
+function decodeSegment(segment: string): string {
+  try {
+    return decodeURIComponent(segment)
+  } catch {
+    return segment
+  }
+}
+
+/** One spelling per URL: decoded path segments and the query sorted, as expo-router parses both.
+ *  An empty value is dropped because screens clear a param with `''` (session `paneKey`), and the
+ *  fragment because expo-router keeps it as a param no href spells the same way. */
+function canonicalHref(href: string): string {
+  const [path = '', search = ''] = href.split('#')[0].split('?')
+  const segments = path.split('/').filter(Boolean).map(decodeSegment)
+  const query = [...new URLSearchParams(search)]
+    .filter(([, value]) => value !== '')
+    .sort(([a], [b]) => (a < b ? -1 : a > b ? 1 : 0))
+  return `/${segments.join('/')}?${new URLSearchParams(query).toString()}`
+}
+
+/** The URL a route on the stack shows, or null when its params hold something no URL carries. */
+function routeHref(route: NativeStackRoute, parentNames: readonly string[]): string | null {
+  const names = [...parentNames, route.name]
+  const nested = route.state?.routes[focusedIndex(route.state)]
+  if (nested) {
+    return routeHref(nested, names)
+  }
+  const params: Record<string, string | string[]> = {}
+  for (const [key, value] of Object.entries(route.params ?? {})) {
+    if (typeof value === 'string') {
+      params[key] = value
+    } else if (Array.isArray(value) && value.every((one) => typeof one === 'string')) {
+      params[key] = value
+    } else if (value !== undefined) {
+      return null
+    }
+  }
+  const pathname = `/${names
+    .flatMap((name) => name.split('/'))
+    .filter(isUrlSegment)
+    .join('/')}`
+  return canonicalHref(stringifyRouteHref({ pathname, params }))
+}
+
+function returnWithin(
+  level: NativeStackState,
+  parentNames: readonly string[],
+  wanted: string
+): NativeStackReturn | 'current' | null {
+  const current = focusedIndex(level)
+  const focused = level.routes[current]
+  if (!focused) {
+    return null
+  }
+  if (focused.state) {
+    // The deeper navigator first: its screens sit nearer the top than anything at this level.
+    const deeper = returnWithin(focused.state, [...parentNames, focused.name], wanted)
+    if (deeper) {
+      return deeper
+    }
+  } else if (routeHref(focused, parentNames) === wanted) {
+    return 'current'
+  }
+  if (!level.key || !focused.key) {
+    return null
+  }
+  for (let index = current - 1; index >= 0; index--) {
+    if (routeHref(level.routes[index], parentNames) === wanted) {
+      return { target: level.key, source: focused.key, count: current - index }
+    }
+  }
+  return null
+}
+
+/**
+ * Where `href` already sits on the native stack: `'current'` when it is the focused screen, the pop
+ * that returns to the nearest screen below showing it, or null when it is nowhere on the stack.
+ *
+ * Path and query both count, so another worktree or tab under the same file is a different screen.
+ */
+export function nativeStackReturn(
+  state: NativeStackState,
+  href: string
+): NativeStackReturn | 'current' | null {
+  return returnWithin(state, [], canonicalHref(href))
+}