fix(desktop): route organization members to their organization and settings

Author: waleedlatif1

apps/desktop/src/main/app-routes.test.ts

@@ -0,0 +1,25 @@
+import { describe, expect, it, vi } from 'vitest'
+
+vi.mock('electron', () => import('@/test/electron-mock'))
+
+import { settingsRoute } from '@/main/app-routes'
+
+describe('settingsRoute', () => {
+  it('opens the settings of the organization the user was last in', () => {
+    expect(settingsRoute('/o/org-1/search?q=x')).toBe('/o/org-1/settings/general')
+  })
+
+  it('keeps the desktop settings of the workspace the user was last in', () => {
+    expect(settingsRoute('/workspace/ws-1/home')).toBe('/workspace/ws-1/settings/desktop')
+  })
+
+  it.each([
+    ['the workspace picker', '/workspace'],
+    ['the app entry', '/home'],
+    ['no saved route', undefined],
+    ['a route that only starts like an organization', '/oauth/o/org-1'],
+    ['an unsafe route', '//evil.example/o/org-1'],
+  ])('resolves settings through the web app from %s', (_label, lastRoute) => {
+    expect(settingsRoute(lastRoute)).toBe('/workspace?redirect=settings')
+  })
+})

apps/desktop/src/main/app-routes.ts

@@ -16,6 +16,12 @@ import { isSafeInternalPath } from '@/main/config'
  */
 export const APP_ENTRY_ROUTE = '/home'
 
+/**
+ * Settings for a viewer whose last route names no workspace or organization. The web
+ * app resolves it to their most recent workspace, or to their organization's settings.
+ */
+const SETTINGS_ENTRY_ROUTE = '/workspace?redirect=settings'
+
 /** Workspace id from the last visited route, or null when it carries none. */
 function workspaceIdFromRoute(lastRoute: string | undefined): string | null {
   if (isSafeInternalPath(lastRoute)) {
@@ -27,6 +33,16 @@ function workspaceIdFromRoute(lastRoute: string | undefined): string | null {
   return null
 }
 
+function organizationIdFromRoute(lastRoute: string | undefined): string | null {
+  if (isSafeInternalPath(lastRoute)) {
+    const match = /^\/o\/([^/?#]+)/.exec(lastRoute)
+    if (match) {
+      return match[1]
+    }
+  }
+  return null
+}
+
 /**
  * Route for "New Chat": the home (chat) surface of the workspace the user was
  * last in, falling back to the app entry when the last route carries no
@@ -38,10 +54,14 @@ export function newChatRoute(lastRoute: string | undefined): string {
 }
 
 /**
- * Route for "Settings…": the Sim app's settings surface for the workspace the
- * user was last in, falling back to the app entry.
+ * Route for "Settings…": the settings of the workspace or organization the user
+ * was last in. Never the app entry, which lands on a page without settings.
  */
 export function settingsRoute(lastRoute: string | undefined): string {
   const workspaceId = workspaceIdFromRoute(lastRoute)
-  return workspaceId ? `/workspace/${workspaceId}/settings/desktop` : APP_ENTRY_ROUTE
+  if (workspaceId) {
+    return `/workspace/${workspaceId}/settings/desktop`
+  }
+  const organizationId = organizationIdFromRoute(lastRoute)
+  return organizationId ? `/o/${organizationId}/settings/general` : SETTINGS_ENTRY_ROUTE
 }

apps/desktop/src/main/handoff.test.ts

@@ -1,9 +1,11 @@
 import { get as httpGet } from 'node:http'
+import type { BrowserWindow } from 'electron'
 import { describe, expect, it, vi } from 'vitest'
 
 vi.mock('electron', () => import('@/test/electron-mock'))
 
 import {
+  createAuthFlow,
   createHandoffManager,
   type HandoffCallback,
   type HandoffCallbacks,
@@ -161,6 +163,31 @@ describe('createHandoffManager', () => {
   })
 })
 
+describe('login handoff landing', () => {
+  it('lands on the app entry after redeeming the login token', async () => {
+    const win = {
+      isDestroyed: () => false,
+      loadURL: vi.fn(async () => {}),
+      show: vi.fn(),
+      focus: vi.fn(),
+      webContents: {
+        getURL: () => 'https://sim.ai/login',
+        executeJavaScript: vi.fn(async () => 200),
+      },
+    }
+    const flow = createAuthFlow({
+      handoff: { consume: () => true } as unknown as ReturnType<typeof createHandoffManager>,
+      origin: () => 'https://sim.ai',
+      events: makeEvents(),
+      ensureMainWindow: async () => win as unknown as BrowserWindow,
+    })
+
+    await flow.handleCallback({ token: VALID_TOKEN, state: _VALID_STATE })
+
+    expect(win.loadURL).toHaveBeenCalledExactlyOnceWith('https://sim.ai/home')
+  })
+})
+
 describe('connect handoff account pinning', () => {
   it('keeps a source request correlated across the browser and native completion', async () => {
     const deps = makeDeps()

apps/desktop/src/main/handoff.ts

@@ -6,6 +6,7 @@ import { getErrorMessage } from '@sim/utils/errors'
 import { generateShortId } from '@sim/utils/id'
 import type { BrowserWindow } from 'electron'
 import { app } from 'electron'
+import { APP_ENTRY_ROUTE } from '@/main/app-routes'
 import { showShellDialog } from '@/main/dialogs'
 import type { EventRecorder } from '@/main/observability'
 
@@ -451,7 +452,7 @@ export interface AuthFlow {
 
 /**
  * Orchestrates the login handoff: opening the system browser, consuming the
- * loopback callback, redeeming the token, and navigating to the workspace. A
+ * loopback callback, redeeming the token, and navigating to the app entry. A
  * failed or expired callback never leaves a partial session — the window lands
  * back on /login.
  */
@@ -521,7 +522,7 @@ export function createAuthFlow(deps: AuthFlowDeps): AuthFlow {
       }
       deps.events.record('handoff_redeem_ok')
       try {
-        await win.loadURL(`${origin}/workspace`)
+        await win.loadURL(`${origin}${APP_ENTRY_ROUTE}`)
       } catch {}
       win.show()
       win.focus()

apps/sim/app/workspace/page.test.tsx

@@ -0,0 +1,62 @@
+/**
+ * @vitest-environment node
+ */
+import { authMockFns } from '@sim/testing/mocks/auth.mock'
+import { nextNavigationMock } from '@sim/testing/mocks/next-navigation.mock'
+import { beforeEach, describe, expect, it, vi } from 'vitest'
+
+const { mockResolveWorkspacelessMemberPath } = vi.hoisted(() => ({
+  mockResolveWorkspacelessMemberPath: vi.fn(),
+}))
+
+vi.mock('next/navigation', () => nextNavigationMock)
+
+vi.mock('@/lib/navigation/resolve-app-entry', () => ({
+  resolveWorkspacelessMemberPath: mockResolveWorkspacelessMemberPath,
+}))
+
+vi.mock('@/app/workspace/workspace-entry', () => ({ WorkspaceEntry: () => null }))
+
+import WorkspacePage from '@/app/workspace/page'
+
+const mockGetSession = authMockFns.mockGetSession
+const session = { user: { id: 'viewer' } }
+
+function visit(search: { redirect?: string } = {}) {
+  return WorkspacePage({ searchParams: Promise.resolve(search) })
+}
+
+describe('WorkspacePage', () => {
+  beforeEach(() => {
+    mockGetSession.mockResolvedValue(session)
+    mockResolveWorkspacelessMemberPath.mockResolvedValue(null)
+  })
+
+  it('forwards a workspaceless member before the picker renders', async () => {
+    mockResolveWorkspacelessMemberPath.mockResolvedValue('/o/org-1/home')
+    await expect(visit()).rejects.toThrow('NEXT_REDIRECT:/o/org-1/home')
+    expect(mockResolveWorkspacelessMemberPath).toHaveBeenCalledWith(session, 'home')
+  })
+
+  it('carries a settings request through to the destination', async () => {
+    mockResolveWorkspacelessMemberPath.mockResolvedValue('/o/org-1/settings/general')
+    await expect(visit({ redirect: 'settings' })).rejects.toThrow(
+      'NEXT_REDIRECT:/o/org-1/settings/general'
+    )
+    expect(mockResolveWorkspacelessMemberPath).toHaveBeenCalledWith(session, 'settings')
+  })
+
+  it('leaves a stale session on the recovery surface', async () => {
+    mockGetSession.mockResolvedValue(null)
+    await expect(visit()).resolves.toBeTruthy()
+    expect(mockResolveWorkspacelessMemberPath).not.toHaveBeenCalled()
+  })
+
+  it.each([
+    ['session', () => mockGetSession.mockRejectedValue(new Error('down'))],
+    ['destination', () => mockResolveWorkspacelessMemberPath.mockRejectedValue(new Error('down'))],
+  ])('keeps the recovery surface when the %s lookup fails', async (_label, fail) => {
+    fail()
+    await expect(visit()).resolves.toBeTruthy()
+  })
+})