From 02e5b56408d4bdb9c6c40928d9cae19eb6b6ef3b Mon Sep 17 00:00:00 2001 From: Mohamed Boudra Date: Tue, 5 May 2026 21:55:57 +0700 Subject: [PATCH] Fix duplicate workspace shell navigation --- .../workspace-navigation-regression.spec.ts | 32 ++++++++++++++++ .../src/app/h/[serverId]/agent/[agentId].tsx | 37 ++++++++++--------- packages/app/src/components/agent-list.tsx | 5 +-- .../app/src/components/agent-stream-view.tsx | 8 +--- packages/app/src/hooks/use-command-center.ts | 10 ++--- .../app/src/hooks/use-open-project.test.ts | 16 ++++---- packages/app/src/hooks/use-open-project.ts | 11 ++---- .../hooks/use-workspace-navigation.test.ts | 14 +++---- .../app/src/hooks/use-workspace-navigation.ts | 4 +- packages/app/src/screens/settings-screen.tsx | 6 +-- .../src/utils/workspace-navigation.test.ts | 3 ++ .../app/src/utils/workspace-navigation.ts | 12 ++++-- 12 files changed, 96 insertions(+), 62 deletions(-) diff --git a/packages/app/e2e/workspace-navigation-regression.spec.ts b/packages/app/e2e/workspace-navigation-regression.spec.ts index 1182e7234..38bd31761 100644 --- a/packages/app/e2e/workspace-navigation-regression.spec.ts +++ b/packages/app/e2e/workspace-navigation-regression.spec.ts @@ -36,6 +36,7 @@ import { workspaceDeckEntryLocator, expectWorkspaceDeckEntryCount, } from "./helpers/workspace-ui"; +import { clickSettingsBackToWorkspace } from "./helpers/settings"; const LOADING_WORKSPACE_TEXT_PATTERN = /Loading workspace/i; @@ -75,6 +76,18 @@ async function expectNoLoadingPane(page: Page): Promise { await expect(page.getByText(LOADING_WORKSPACE_TEXT_PATTERN)).toHaveCount(0); } +async function getVisibleDraftTabCount(page: Page): Promise { + return page.locator('[data-testid^="workspace-tab-draft"]').filter({ visible: true }).count(); +} + +async function closeFirstVisibleDraftTab(page: Page): Promise { + const closeButton = page.locator('[data-testid^="workspace-draft-close-"]').filter({ + visible: true, + }); + await expect(closeButton.first()).toBeVisible({ timeout: 30_000 }); + await closeButton.first().click(); +} + async function installDaemonWebSocketGate(page: Page, daemonPort: string) { let acceptingConnections = true; const activeSockets = new Set(); @@ -133,6 +146,25 @@ async function installDaemonWebSocketGate(page: Page, daemonPort: string) { test.describe("Workspace navigation regression", () => { test.describe.configure({ timeout: 240_000 }); + test("keeps one replacement draft after returning from settings and closing the last tab", async ({ + page, + withWorkspace, + }) => { + const workspace = await withWorkspace({ prefix: "workspace-settings-back-tab-" }); + + await workspace.navigateTo(); + await expect.poll(() => getVisibleDraftTabCount(page), { timeout: 30_000 }).toBe(1); + + await openSettings(page); + await clickSettingsBackToWorkspace(page); + await expect(page).toHaveURL(/\/workspace\//, { timeout: 30_000 }); + await expect.poll(() => getVisibleDraftTabCount(page), { timeout: 30_000 }).toBe(1); + + await closeFirstVisibleDraftTab(page); + + await expect.poll(() => getVisibleDraftTabCount(page), { timeout: 30_000 }).toBe(1); + }); + test("keeps the workspace rendered while reconnecting to the host", async ({ page }) => { const serverId = process.env.E2E_SERVER_ID; const daemonPort = process.env.E2E_DAEMON_PORT; diff --git a/packages/app/src/app/h/[serverId]/agent/[agentId].tsx b/packages/app/src/app/h/[serverId]/agent/[agentId].tsx index c32788dbc..2e5d2cd87 100644 --- a/packages/app/src/app/h/[serverId]/agent/[agentId].tsx +++ b/packages/app/src/app/h/[serverId]/agent/[agentId].tsx @@ -1,12 +1,12 @@ import { useEffect, useRef } from "react"; -import { useLocalSearchParams, useRouter, type Href } from "expo-router"; +import { useLocalSearchParams, usePathname, useRouter, type Href } from "expo-router"; import { HostRouteBootstrapBoundary } from "@/components/host-route-bootstrap-boundary"; import { useSessionStore } from "@/stores/session-store"; import { useResolveWorkspaceIdByCwd } from "@/stores/session-store-hooks"; import { useHostRuntimeClient, useHostRuntimeIsConnected } from "@/runtime/host-runtime"; import { buildHostRootRoute } from "@/utils/host-routes"; import { resolveWorkspaceIdByExecutionDirectory } from "@/utils/workspace-execution"; -import { prepareWorkspaceTab } from "@/utils/workspace-navigation"; +import { navigateToPreparedWorkspaceTab } from "@/utils/workspace-navigation"; export default function HostAgentReadyRoute() { return ( @@ -18,6 +18,7 @@ export default function HostAgentReadyRoute() { function HostAgentReadyRouteContent() { const router = useRouter(); + const pathname = usePathname(); const params = useLocalSearchParams<{ serverId?: string; agentId?: string; @@ -50,15 +51,15 @@ function HostAgentReadyRouteContent() { if (resolvedWorkspaceId) { redirectedRef.current = true; - router.replace( - prepareWorkspaceTab({ - serverId, - workspaceId: resolvedWorkspaceId, - target: { kind: "agent", agentId }, - }) as Href, - ); + navigateToPreparedWorkspaceTab({ + serverId, + workspaceId: resolvedWorkspaceId, + target: { kind: "agent", agentId }, + navigationMethod: "replace", + currentPathname: pathname, + }); } - }, [agentId, resolvedWorkspaceId, router, serverId]); + }, [agentId, pathname, resolvedWorkspaceId, router, serverId]); useEffect(() => { if (redirectedRef.current) { @@ -102,13 +103,13 @@ function HostAgentReadyRouteContent() { } redirectedRef.current = true; if (workspaceId) { - router.replace( - prepareWorkspaceTab({ - serverId, - workspaceId, - target: { kind: "agent", agentId }, - }) as Href, - ); + navigateToPreparedWorkspaceTab({ + serverId, + workspaceId, + target: { kind: "agent", agentId }, + navigationMethod: "replace", + currentPathname: pathname, + }); return; } router.replace(buildHostRootRoute(serverId)); @@ -125,7 +126,7 @@ function HostAgentReadyRouteContent() { return () => { cancelled = true; }; - }, [agentId, client, hasHydratedWorkspaces, isConnected, router, serverId]); + }, [agentId, client, hasHydratedWorkspaces, isConnected, pathname, router, serverId]); return null; } diff --git a/packages/app/src/components/agent-list.tsx b/packages/app/src/components/agent-list.tsx index 0668515f6..6539fe474 100644 --- a/packages/app/src/components/agent-list.tsx +++ b/packages/app/src/components/agent-list.tsx @@ -21,7 +21,7 @@ import { Archive } from "lucide-react-native"; import { getProviderIcon } from "@/components/provider-icons"; import { buildHostAgentDetailRoute } from "@/utils/host-routes"; import { resolveWorkspaceIdByExecutionDirectory } from "@/utils/workspace-execution"; -import { prepareWorkspaceTab } from "@/utils/workspace-navigation"; +import { navigateToPreparedWorkspaceTab } from "@/utils/workspace-navigation"; import type { Agent } from "@/stores/session-store"; import { useArchiveAgent } from "@/hooks/use-archive-agent"; @@ -324,13 +324,12 @@ export function AgentList({ } rememberArchivedAgentDetail(agent); - const route = prepareWorkspaceTab({ + navigateToPreparedWorkspaceTab({ serverId, workspaceId, target: { kind: "agent", agentId }, pin: Boolean(agent.archivedAt), }); - router.navigate(route); }, [isActionSheetVisible, onAgentSelect], ); diff --git a/packages/app/src/components/agent-stream-view.tsx b/packages/app/src/components/agent-stream-view.tsx index 59466ba91..8f7dfa4a5 100644 --- a/packages/app/src/components/agent-stream-view.tsx +++ b/packages/app/src/components/agent-stream-view.tsx @@ -21,7 +21,6 @@ import { import { StyleSheet, withUnistyles } from "react-native-unistyles"; import { useIsCompactFormFactor } from "@/constants/layout"; import { useMutation } from "@tanstack/react-query"; -import { useRouter } from "expo-router"; import Animated, { FadeIn, FadeOut, @@ -83,7 +82,7 @@ import { import { MAX_CONTENT_WIDTH } from "@/constants/layout"; import { normalizeInlinePathTarget } from "@/utils/inline-path"; import { resolveWorkspaceIdByExecutionDirectory } from "@/utils/workspace-execution"; -import { prepareWorkspaceTab } from "@/utils/workspace-navigation"; +import { navigateToPreparedWorkspaceTab } from "@/utils/workspace-navigation"; import { useStableEvent } from "@/hooks/use-stable-event"; import { getWorkingIndicatorDotStrength, @@ -135,7 +134,6 @@ const AgentStreamViewComponent = forwardRef(null); - const router = useRouter(); const isMobile = useIsCompactFormFactor(); const streamRenderStrategy = useMemo( () => @@ -213,12 +211,11 @@ const AgentStreamViewComponent = forwardRef s.commandCenterOpen); @@ -212,14 +212,14 @@ export function useCommandCenter() { router.navigate(buildHostAgentDetailRoute(agent.serverId, agent.id) as Href); return; } - const route = prepareWorkspaceTab({ + navigateToPreparedWorkspaceTab({ serverId: agent.serverId, workspaceId, target: { kind: "agent", agentId: agent.id }, + currentPathname: pathname, }); - router.navigate(route); }, - [setOpen], + [pathname, setOpen], ); const openProjectPicker = useOpenProjectPicker(activeServerId); diff --git a/packages/app/src/hooks/use-open-project.test.ts b/packages/app/src/hooks/use-open-project.test.ts index 252a14196..41073b136 100644 --- a/packages/app/src/hooks/use-open-project.test.ts +++ b/packages/app/src/hooks/use-open-project.test.ts @@ -1,5 +1,9 @@ import { beforeEach, describe, expect, it, vi } from "vitest"; +vi.hoisted(() => { + (globalThis as typeof globalThis & { __DEV__?: boolean }).__DEV__ = false; +}); + vi.mock("@react-native-async-storage/async-storage", () => { const storage = new Map(); return { @@ -19,10 +23,8 @@ const { replaceRoute } = vi.hoisted(() => ({ replaceRoute: vi.fn(), })); -vi.mock("expo-router", () => ({ - router: { - replace: replaceRoute, - }, +vi.mock("@/hooks/use-workspace-navigation", () => ({ + navigateToWorkspace: replaceRoute, })); import { openProjectDirectly } from "@/hooks/use-open-project"; @@ -89,7 +91,7 @@ describe("openProjectDirectly", () => { mergeWorkspaces: useSessionStore.getState().mergeWorkspaces, setHasHydratedWorkspaces: useSessionStore.getState().setHasHydratedWorkspaces, openDraftTab: createOpenDraftTab(), - replaceRoute, + navigateToWorkspace: replaceRoute, }); expect(result).toBe(true); @@ -115,7 +117,7 @@ describe("openProjectDirectly", () => { const tabs = collectAllTabs(layout.root); expect(tabs).toHaveLength(1); expect(tabs[0]?.target.kind).toBe("draft"); - expect(replaceRoute).toHaveBeenCalledWith("/h/server-1/workspace/1"); + expect(replaceRoute).toHaveBeenCalledWith("server-1", "1"); }); it("does not navigate or seed tabs when openProject fails", async () => { @@ -133,7 +135,7 @@ describe("openProjectDirectly", () => { mergeWorkspaces: useSessionStore.getState().mergeWorkspaces, setHasHydratedWorkspaces: useSessionStore.getState().setHasHydratedWorkspaces, openDraftTab: createOpenDraftTab(), - replaceRoute, + navigateToWorkspace: replaceRoute, }); expect(result).toBe(false); diff --git a/packages/app/src/hooks/use-open-project.ts b/packages/app/src/hooks/use-open-project.ts index 4e5e4d174..ee7f77a50 100644 --- a/packages/app/src/hooks/use-open-project.ts +++ b/packages/app/src/hooks/use-open-project.ts @@ -1,5 +1,4 @@ import { useCallback } from "react"; -import { router, type Href } from "expo-router"; import type { DaemonClient } from "@server/client/daemon-client"; import { useHostRuntimeClient, useHostRuntimeIsConnected } from "@/runtime/host-runtime"; import { @@ -12,7 +11,7 @@ import { useWorkspaceLayoutStore, } from "@/stores/workspace-layout-store"; import { generateDraftId } from "@/stores/draft-keys"; -import { buildHostWorkspaceRoute } from "@/utils/host-routes"; +import { navigateToWorkspace } from "@/hooks/use-workspace-navigation"; interface OpenProjectDirectlyInput { serverId: string; @@ -22,7 +21,7 @@ interface OpenProjectDirectlyInput { mergeWorkspaces: (serverId: string, workspaces: Iterable) => void; setHasHydratedWorkspaces: (serverId: string, hydrated: boolean) => void; openDraftTab: (workspaceKey: string) => string | null; - replaceRoute: (route: string) => void; + navigateToWorkspace: (serverId: string, workspaceId: string) => void; } export async function openProjectDirectly(input: OpenProjectDirectlyInput): Promise { @@ -50,7 +49,7 @@ export async function openProjectDirectly(input: OpenProjectDirectlyInput): Prom } input.openDraftTab(workspaceKey); - input.replaceRoute(buildHostWorkspaceRoute(normalizedServerId, workspace.id)); + input.navigateToWorkspace(normalizedServerId, workspace.id); return true; } @@ -75,9 +74,7 @@ export function useOpenProject(serverId: string | null): (path: string) => Promi kind: "draft", draftId: generateDraftId(), }), - replaceRoute: (route) => { - router.replace(route as Href); - }, + navigateToWorkspace, }); }, [client, isConnected, mergeWorkspaces, normalizedServerId, setHasHydratedWorkspaces], diff --git a/packages/app/src/hooks/use-workspace-navigation.test.ts b/packages/app/src/hooks/use-workspace-navigation.test.ts index 280c038ba..829555604 100644 --- a/packages/app/src/hooks/use-workspace-navigation.test.ts +++ b/packages/app/src/hooks/use-workspace-navigation.test.ts @@ -1,12 +1,12 @@ import { beforeEach, describe, expect, it, vi } from "vitest"; -const { navigateMock } = vi.hoisted(() => ({ - navigateMock: vi.fn(), +const { dismissToMock } = vi.hoisted(() => ({ + dismissToMock: vi.fn(), })); vi.mock("expo-router", () => ({ router: { - navigate: navigateMock, + dismissTo: dismissToMock, }, })); @@ -19,11 +19,11 @@ import { describe("navigateToWorkspace", () => { beforeEach(() => { - navigateMock.mockReset(); + dismissToMock.mockReset(); syncNavigationActiveWorkspace({ current: null }); }); - it("uses router navigation from a non-workspace route even when active selection is stale", () => { + it("dismisses to the workspace route from a non-workspace route even when active selection is stale", () => { activateNavigationWorkspaceSelection({ serverId: "server-1", workspaceId: "workspace-a", @@ -33,7 +33,7 @@ describe("navigateToWorkspace", () => { currentPathname: "/h/server-1/sessions", }); - expect(navigateMock).toHaveBeenCalledWith("/h/server-1/workspace/workspace-b"); + expect(dismissToMock).toHaveBeenCalledWith("/h/server-1/workspace/workspace-b"); expect(getNavigationActiveWorkspaceSelection()).toEqual({ serverId: "server-1", workspaceId: "workspace-a", @@ -50,7 +50,7 @@ describe("navigateToWorkspace", () => { currentPathname: "/h/server-1/workspace/workspace-a", }); - expect(navigateMock).not.toHaveBeenCalled(); + expect(dismissToMock).not.toHaveBeenCalled(); expect(getNavigationActiveWorkspaceSelection()).toEqual({ serverId: "server-1", workspaceId: "workspace-b", diff --git a/packages/app/src/hooks/use-workspace-navigation.ts b/packages/app/src/hooks/use-workspace-navigation.ts index f7483e52c..bd6459712 100644 --- a/packages/app/src/hooks/use-workspace-navigation.ts +++ b/packages/app/src/hooks/use-workspace-navigation.ts @@ -19,7 +19,7 @@ function shouldUseRetainedWorkspaceSwitch(input: { } if (input.currentPathname == null) { - return true; + return false; } return parseHostWorkspaceRouteFromPathname(input.currentPathname) !== null; @@ -49,7 +49,7 @@ export function navigateToWorkspace( } const href = buildHostWorkspaceRoute(serverId, workspaceId); - router.navigate(href); + router.dismissTo(href); } export function useWorkspaceNavigation() { diff --git a/packages/app/src/screens/settings-screen.tsx b/packages/app/src/screens/settings-screen.tsx index f8fef6dfe..0c9ad7ab0 100644 --- a/packages/app/src/screens/settings-screen.tsx +++ b/packages/app/src/screens/settings-screen.tsx @@ -78,13 +78,13 @@ import { useIsCompactFormFactor } from "@/constants/layout"; import { useLocalDaemonServerId } from "@/hooks/use-is-local-daemon"; import { buildHostOpenProjectRoute, - buildHostWorkspaceRoute, buildProjectsSettingsRoute, buildSettingsHostRoute, buildSettingsSectionRoute, type SettingsSectionSlug, } from "@/utils/host-routes"; import { getLastNavigationWorkspaceRouteSelection } from "@/stores/navigation-active-workspace-store"; +import { navigateToWorkspace } from "@/hooks/use-workspace-navigation"; // --------------------------------------------------------------------------- // View model @@ -963,9 +963,7 @@ export default function SettingsScreen({ view }: SettingsScreenProps) { const handleBackToWorkspace = useCallback(() => { const lastWorkspaceRoute = getLastNavigationWorkspaceRouteSelection(); if (lastWorkspaceRoute) { - router.replace( - buildHostWorkspaceRoute(lastWorkspaceRoute.serverId, lastWorkspaceRoute.workspaceId), - ); + navigateToWorkspace(lastWorkspaceRoute.serverId, lastWorkspaceRoute.workspaceId); return; } if (anyOnlineServerId) { diff --git a/packages/app/src/utils/workspace-navigation.test.ts b/packages/app/src/utils/workspace-navigation.test.ts index 8ed2f05a1..9a48b6c0b 100644 --- a/packages/app/src/utils/workspace-navigation.test.ts +++ b/packages/app/src/utils/workspace-navigation.test.ts @@ -8,6 +8,7 @@ const { platformState, routerMock } = vi.hoisted(() => ({ routerMock: { back: vi.fn(), canGoBack: vi.fn(() => false), + dismissTo: vi.fn(), navigate: vi.fn(), replace: vi.fn(), }, @@ -60,6 +61,7 @@ describe("prepareWorkspaceTab", () => { routerMock.back.mockReset(); routerMock.canGoBack.mockReset(); routerMock.canGoBack.mockReturnValue(false); + routerMock.dismissTo.mockReset(); routerMock.navigate.mockReset(); routerMock.replace.mockReset(); syncNavigationActiveWorkspace({ current: null }); @@ -106,6 +108,7 @@ describe("prepareWorkspaceTab", () => { expect(route).toBe("/h/server-1/workspace/b64_L3JlcG8vd29ya3RyZWU"); expect(routerMock.back).toHaveBeenCalledOnce(); + expect(routerMock.dismissTo).not.toHaveBeenCalled(); expect(routerMock.replace).not.toHaveBeenCalled(); vi.runAllTimers(); diff --git a/packages/app/src/utils/workspace-navigation.ts b/packages/app/src/utils/workspace-navigation.ts index 075042ff3..9cdc41384 100644 --- a/packages/app/src/utils/workspace-navigation.ts +++ b/packages/app/src/utils/workspace-navigation.ts @@ -1,5 +1,6 @@ -import { router, type Href } from "expo-router"; +import { router } from "expo-router"; import { isNative } from "@/constants/platform"; +import { navigateToWorkspace } from "@/hooks/use-workspace-navigation"; import { activateNavigationWorkspaceSelection, getLastNavigationWorkspaceRouteSelection, @@ -22,6 +23,7 @@ interface PrepareWorkspaceTabInput { interface NavigateToPreparedWorkspaceTabInput extends PrepareWorkspaceTabInput { navigationMethod?: "navigate" | "replace"; + currentPathname?: string | null; } function getPreparedTarget(target: WorkspaceTabTarget): WorkspaceTabTarget { @@ -65,9 +67,13 @@ export function navigateToPreparedWorkspaceTab(input: NavigateToPreparedWorkspac }, 0); return route; } - router.replace(route as Href); + navigateToWorkspace(input.serverId, input.workspaceId, { + currentPathname: input.currentPathname, + }); } else { - router.navigate(route as Href); + navigateToWorkspace(input.serverId, input.workspaceId, { + currentPathname: input.currentPathname, + }); } return route; }