fix(links): route devices and /phone-app through the linking config

shared/constants/deeplinks.test.ts

@@ -0,0 +1,69 @@
+/// <reference types="jest" />
+jest.mock('./router', () => ({
+  navUpToScreen: jest.fn(),
+  navigateAppend: jest.fn(),
+  navigateToThread: jest.fn(),
+  navToProfile: jest.fn(),
+  previewConversation: jest.fn(),
+  switchTab: jest.fn(),
+}))
+jest.mock('@/teams/team-page-actions', () => ({showTeamByName: jest.fn()}))
+import * as Router from './router'
+import * as Tabs from './tabs'
+import {settingsDevicesTab} from './settings'
+import {handleAppLink} from './deeplinks'
+
+const withIsMobile = (isMobile: boolean, f: () => void) => {
+  const was = global.isMobile
+  global.isMobile = isMobile
+  try {
+    f()
+  } finally {
+    global.isMobile = was
+  }
+}
+
+beforeEach(() => {
+  jest.clearAllMocks()
+})
+
+// On desktop handleAppLink IS the linking subscription's listener (router.tsx passes it as
+// both listener and fallback), so this case is the whole implementation there.
+test('a devices link opens the devices tab on desktop', () => {
+  withIsMobile(false, () => {
+    handleAppLink('keybase://devices')
+
+    expect(Router.switchTab).toHaveBeenCalledWith(Tabs.devicesTab)
+    expect(Router.navUpToScreen).toHaveBeenCalledWith('devicesRoot')
+    expect(Router.navigateAppend).not.toHaveBeenCalled()
+  })
+})
+
+// The mobile half of this switch is the fallback only: isHandledByLinkingConfig now claims
+// keybase://devices, so on mobile every producer routes it to the linking config instead (the
+// phone shape it builds is covered by router-v2/linking-phone.test.ts). It still has to be
+// phone-correct, because the config is the thing that can stop claiming a URL.
+// One call covers phone and tablet: the push carries no target, so it is handled by whichever
+// navigator registers the route -- the root stack above the tabs on a phone, the Settings tab
+// stack on a tablet. navUpToScreen cannot do that; it pins its popTo to the active stack, which
+// at a tab root on a phone is the Settings tab stack, where the route does not exist.
+test('a devices link pushes the devices screen without pinning it to the settings tab stack', () => {
+  withIsMobile(true, () => {
+    handleAppLink('keybase://devices')
+
+    expect(Router.switchTab).toHaveBeenCalledWith(Tabs.settingsTab)
+    expect(Router.navigateAppend).toHaveBeenCalledWith({name: settingsDevicesTab, params: {}})
+    expect(Router.navUpToScreen).not.toHaveBeenCalled()
+  })
+})
+
+// The invite install link normalizes to this; the linking config handles it on mobile, but
+// desktop routes every URL through here, so both have to agree on where it goes.
+test('an add-phone link opens the add-phone modal over settings', () => {
+  withIsMobile(false, () => {
+    handleAppLink('keybase://settingsAddPhone')
+
+    expect(Router.switchTab).toHaveBeenCalledWith(Tabs.settingsTab)
+    expect(Router.navigateAppend).toHaveBeenCalledWith({name: 'settingsAddPhone', params: {}})
+  })
+})

shared/constants/deeplinks.tsx

@@ -1,7 +1,15 @@
 import logger from '@/logger'
 import * as T from '@/constants/types'
-import {navigateAppend, navigateToThread, navToProfile, previewConversation, switchTab} from './router'
+import {
+  navigateAppend,
+  navigateToThread,
+  navToProfile,
+  navUpToScreen,
+  previewConversation,
+  switchTab,
+} from './router'
 import * as Tabs from './tabs'
+import {settingsDevicesTab} from './settings'
 import {showTeamByName} from '@/teams/team-page-actions'
 
 const prefix = 'keybase://'
@@ -75,6 +83,27 @@ const handleKeybaseLink = (link: string) => {
         return
       }
       break
+    case 'devices':
+      // Devices live under Settings on phone/tablet and in their own tab on desktop.
+      if (!isMobile) {
+        switchTab(Tabs.devicesTab)
+        navUpToScreen('devicesRoot')
+        return
+      }
+      switchTab(Tabs.settingsTab)
+      // navUpToScreen pins its popTo to the deepest active stack, which at a tab root on a
+      // phone is the Settings tab stack -- and that stack knows only settingsRoot there, so
+      // StackRouter returned null and the action was dropped. An untargeted push lands
+      // wherever the route is registered: the root stack above the tabs on a phone, the
+      // Settings tab stack on a tablet. Same call the phone settings list itself makes.
+      navigateAppend({name: settingsDevicesTab, params: {}})
+      return
+    case 'settingsAddPhone':
+      // Where the invite install link (https://keybase.io/phone-app) lands. The linking config
+      // also handles it; desktop routes every URL here, so this must agree with it.
+      switchTab(Tabs.settingsTab)
+      navigateAppend({name: 'settingsAddPhone', params: {}})
+      return
     case 'private':
     case 'public':
       try {

shared/constants/init/index.tsx

@@ -158,19 +158,17 @@ const loadStartupDetails = async () => {
   let conversation: T.Chat.ConversationIDKey | undefined
   let conversationUid = ''
   let followUser = ''
-  let link = ''
   let tab = ''
 
   // Top priority, push
   if (push) {
     logger.info('initialState: push', push.startupConversation, push.startupFollowUser)
     conversation = push.startupConversation
     followUser = push.startupFollowUser ?? ''
-  } else if (initialUrl) {
-    // Second priority, deep link
-    link = initialUrl
-  } else if (routeState) {
-    // Last priority, saved from last session
+  } else if (!initialUrl && routeState) {
+    // Last priority, saved from last session. The linking config reads the launch URL
+    // itself; this read only decides whether the saved route may be restored, since a
+    // launch URL outranks it.
     try {
       const item = JSON.parse(routeState) as
         | undefined
@@ -203,7 +201,6 @@ const loadStartupDetails = async () => {
     conversation: conversation ?? noConversationIDKey,
     conversationUid,
     followUser,
-    link,
     tab: tab as Tabs.Tab,
   })
 
@@ -592,7 +589,6 @@ const _initDesktopPlatformListener = () => {
       useConfigState.getState().dispatch.setStartupDetails({
         conversation: Chat.noConversationIDKey,
         followUser: '',
-        link: '',
         tab: undefined,
       })
     }

shared/router-v2/deep-link-emitter.tsx

@@ -40,6 +40,15 @@ const normalizeHttpUrl = (url: string): string | undefined => {
       : `keybase://team-page/${teamName}`
   }
 
+  // /phone-app — the install link our own chat invite banner texts to an unresolved @phone
+  // participant (chat/conversation/bottom-banner.tsx). It is not a username, so it has to be
+  // carved out ahead of the single-segment rule below, which would otherwise open a profile
+  // for a user that does not exist. It always opens Add Phone Number: the invitee's inviter
+  // wrote to a number, and nothing here knows (or waits to learn) whether they have one.
+  if (pathname === '/phone-app' || pathname === '/phone-app/') {
+    return 'keybase://settingsAddPhone'
+  }
+
   // /username (single path segment)
   const userMatch = pathname.match(/^\/((?:[a-zA-Z0-9][a-zA-Z0-9_-]?)+)\/?$/)
   if (userMatch?.[1]) {

shared/router-v2/linking-initial-url.test.ts

@@ -21,7 +21,6 @@ type Startup = {
   conversation: T.Chat.ConversationIDKey
   conversationUid?: string
   followUser: string
-  link: string
   tab?: Tabs.Tab
 }
 
@@ -32,7 +31,6 @@ const setStartup = (st: Partial<Startup>) => {
     startup: {
       conversation: T.Chat.noConversationIDKey,
       followUser: '',
-      link: '',
       loaded: true,
       ...st,
     },

shared/router-v2/linking-phone.test.ts

@@ -0,0 +1,56 @@
+/// <reference types="jest" />
+// Phone shapes. isSplit is computed at module load and this suite loads as desktop, so
+// global.isMobile alone yields the tablet shapes (see linking.test.ts / linking-state.test.ts);
+// only mocking the module gets the phone ones. On a phone each tab stack holds just its root
+// screen -- every other route is registered on the root stack, above the tabs -- so a nested
+// route is silently dropped on rehydrate and the tap lands on the tab root.
+jest.mock('@/constants/chat/layout', () => ({isSplit: false, threadRouteName: 'chatConversation'}))
+import * as Settings from '@/constants/settings'
+import * as Tabs from '@/constants/tabs'
+import {createLinkingConfig} from './linking'
+
+const getStateFromPath = (path: string) =>
+  (createLinkingConfig(jest.fn()).getStateFromPath as (p: string) => unknown)(path)
+
+const wasMobile = global.isMobile
+beforeAll(() => {
+  global.isMobile = true
+})
+afterAll(() => {
+  global.isMobile = wasMobile
+})
+
+test('a devices link opens devices on the root stack above the tabs on a phone', () => {
+  expect(getStateFromPath('devices')).toEqual({
+    index: 1,
+    routes: [
+      {
+        name: 'loggedIn',
+        state: {
+          index: 0,
+          routes: [{name: Tabs.settingsTab, state: {index: 0, routes: [{name: 'settingsRoot'}]}}],
+        },
+      },
+      {name: Settings.settingsDevicesTab},
+    ],
+  })
+})
+
+// A control: if the isSplit mock ever stopped taking effect, this would produce the split
+// (chatRoot-with-params) shape instead, and the devices expectation above would be testing
+// the tablet path while claiming to test the phone one.
+test('the phone shapes are in force -- a conversation opens above the tabs, not in the chat tab', () => {
+  expect(getStateFromPath('convid/conv-1')).toEqual({
+    index: 1,
+    routes: [
+      {
+        name: 'loggedIn',
+        state: {
+          index: 0,
+          routes: [{name: Tabs.chatTab, state: {index: 0, routes: [{name: 'chatRoot', params: {}}]}}],
+        },
+      },
+      {name: 'chatConversation', params: {conversationIDKey: 'conv-1'}},
+    ],
+  })
+})

shared/router-v2/linking-state.test.ts

@@ -144,6 +144,17 @@ test('the push prompt is a modal with no tab parked underneath', () => {
   })
 })
 
+test('add-phone is a modal over the settings tab', () => {
+  expect(isHandledByLinkingConfig('keybase://settingsAddPhone')).toBe(true)
+  expect(getStateFromPath('settingsAddPhone')).toEqual({
+    index: 1,
+    routes: [
+      {name: 'loggedIn', state: {index: 0, routes: [{name: Tabs.settingsTab}]}},
+      {name: 'settingsAddPhone'},
+    ],
+  })
+})
+
 test('every app tab name is a bare tab switch', () => {
   for (const tab of [
     Tabs.chatTab,

shared/router-v2/linking.test.ts

@@ -3,7 +3,9 @@ import {useConfigState} from '@/stores/config'
 import {useCurrentUserState} from '@/stores/current-user'
 import {useNavigationIntentsState} from '@/stores/navigation-intents'
 import {emitDeepLink} from './deep-link-emitter'
-import {subscribeNavigationIntents} from './linking'
+import * as Settings from '@/constants/settings'
+import * as Tabs from '@/constants/tabs'
+import {createLinkingConfig, isHandledByLinkingConfig, subscribeNavigationIntents} from './linking'
 
 const setCurrentUser = (uid: string) => {
   useCurrentUserState.getState().dispatch.setBootstrap({
@@ -148,3 +150,58 @@ test('consumes an intent after bootstrap fills in the uid the router readied wit
   expect(listener).toHaveBeenCalledWith('keybase://convid/post-bootstrap-conversation')
   unsubscribe()
 })
+
+const getStateFromPath = (path: string) =>
+  (createLinkingConfig(jest.fn()).getStateFromPath as (p: string) => unknown)(path)
+
+test('a devices link is consumed by the linking config, not by handleAppLink', () => {
+  useNavigationIntentsState.getState().dispatch.setNavigationReady(true, 'current-uid')
+  const listener = jest.fn()
+  const handleAppLink = jest.fn()
+  const unsubscribe = subscribeNavigationIntents(listener, handleAppLink)
+
+  emitDeepLink('keybase://devices')
+
+  expect(isHandledByLinkingConfig('keybase://devices')).toBe(true)
+  expect(listener).toHaveBeenCalledWith('keybase://devices')
+  expect(handleAppLink).not.toHaveBeenCalled()
+  unsubscribe()
+})
+
+// isSplit is baked in at module load and this suite loads as desktop, so global.isMobile alone
+// gets the tablet shape, not the phone one. Phone coverage lives in linking-phone.test.ts.
+test('a devices link opens the devices screen inside the settings tab on tablet', () => {
+  const wasMobile = global.isMobile
+  global.isMobile = true
+  try {
+    expect(getStateFromPath('devices')).toEqual({
+      index: 0,
+      routes: [
+        {
+          name: 'loggedIn',
+          state: {
+            index: 0,
+            routes: [
+              {
+                name: Tabs.settingsTab,
+                state: {
+                  index: 1,
+                  routes: [{name: 'settingsRoot'}, {name: Settings.settingsDevicesTab}],
+                },
+              },
+            ],
+          },
+        },
+      ],
+    })
+  } finally {
+    global.isMobile = wasMobile
+  }
+})
+
+test('a devices link opens the devices tab on desktop', () => {
+  expect(getStateFromPath('devices')).toEqual({
+    index: 0,
+    routes: [{name: 'loggedIn', state: {index: 0, routes: [{name: Tabs.devicesTab}]}}],
+  })
+})