diff --git a/docs/development.md b/docs/development.md index 6375a63d2..8a20c1a99 100644 --- a/docs/development.md +++ b/docs/development.md @@ -76,6 +76,15 @@ workspace for that host after the workspace-selection store hydrates unless the host's hydrated workspace list proves that workspace is gone; hosts without a remembered workspace go to `open-project`. +When app-wide routes such as `/new` navigate back into a host workspace, use +`navigateToHostWorkspaceRoute()` instead of calling `router.dismissTo()` with the +leaf workspace URL. The root stack owns `h/[serverId]`; the host stack owns +`workspace/[workspaceId]/index`. Repeated global-route hops must `POP_TO` the +root host route and pass the nested workspace screen, or Expo Router can append +extra hidden workspace deck entries. Those hidden entries are not harmless: +composer floating panels can measure against the wrong deck and disappear +offscreen. + Keep workspace identity and retention outside native-stack `getId`/ `dangerouslySingular`. Expo Router maps `dangerouslySingular` to React Navigation `getId`, and `getId` has broken Android native-stack/Fabric by diff --git a/packages/app/e2e/composer-autocomplete.spec.ts b/packages/app/e2e/composer-autocomplete.spec.ts index c7d80e857..b813c605a 100644 --- a/packages/app/e2e/composer-autocomplete.spec.ts +++ b/packages/app/e2e/composer-autocomplete.spec.ts @@ -1,8 +1,14 @@ import { expect, test, type Page } from "./fixtures"; import { composerLocator, expectComposerVisible } from "./helpers/composer"; -import { openAgentRoute, seedMockAgentWorkspace } from "./helpers/mock-agent"; +import { + openAgentRoute, + seedMockAgentWorkspace, + type MockAgentWorkspace, +} from "./helpers/mock-agent"; import { expectWorkspaceTabVisible } from "./helpers/archive-tab"; import { daemonWsRoutePattern } from "./helpers/daemon-port"; +import { getServerId } from "./helpers/server-id"; +import { switchWorkspaceViaSidebar } from "./helpers/workspace-ui"; const TEST_COMMANDS = [ { @@ -142,6 +148,43 @@ async function installListCommandsStub(page: Page): Promise { }); } +async function openAppWideNewWorkspace(page: Page): Promise { + await page.getByTestId("sidebar-global-new-workspace").first().click(); + await page.waitForURL((url) => url.pathname === "/new", { timeout: 30_000 }); +} + +async function expectSingleCurrentWorkspaceDeckEntry( + page: Page, + input: { expectedDeckEntryCount: number; serverId: string; workspaceId: string }, +): Promise { + const summary = await page + .locator('[data-testid^="workspace-deck-entry-"]') + .evaluateAll((elements, target) => { + const currentTestId = `workspace-deck-entry-${target.serverId}:${target.workspaceId}`; + const entries = elements.map((element) => { + const rect = element.getBoundingClientRect(); + return { + testId: element.getAttribute("data-testid"), + hasLayout: rect.width > 0 && rect.height > 0, + }; + }); + const currentEntries = entries.filter((entry) => entry.testId === currentTestId); + return { + totalDeckEntryCount: entries.length, + currentWorkspaceEntryCount: currentEntries.length, + visibleCurrentWorkspaceEntryCount: currentEntries.filter((entry) => entry.hasLayout).length, + hiddenCurrentWorkspaceEntryCount: currentEntries.filter((entry) => !entry.hasLayout).length, + }; + }, input); + + expect(summary).toEqual({ + totalDeckEntryCount: input.expectedDeckEntryCount, + currentWorkspaceEntryCount: 1, + visibleCurrentWorkspaceEntryCount: 1, + hiddenCurrentWorkspaceEntryCount: 0, + }); +} + async function openReadyMockAgent( page: Page, options?: { expectWorkspaceTab?: boolean }, @@ -308,6 +351,64 @@ function expectPopoverDoesNotDisappearAfterFirstVisible(frames: PopoverFrame[]): } test.describe("Composer autocomplete", () => { + test("stays visible after returning from the app-wide new workspace route", async ({ page }) => { + await installListCommandsStub(page); + const serverId = getServerId(); + const sessions: MockAgentWorkspace[] = []; + + try { + sessions.push( + await seedMockAgentWorkspace({ + repoPrefix: "autocomplete-new-route-a-", + title: "Autocomplete new route A", + }), + ); + sessions.push( + await seedMockAgentWorkspace({ + repoPrefix: "autocomplete-new-route-b-", + title: "Autocomplete new route B", + }), + ); + sessions.push( + await seedMockAgentWorkspace({ + repoPrefix: "autocomplete-new-route-c-", + title: "Autocomplete new route C", + }), + ); + + const [first, second, third] = sessions; + + await openAgentRoute(page, first); + await expectComposerVisible(page, { timeout: 30_000 }); + + await openAppWideNewWorkspace(page); + await switchWorkspaceViaSidebar({ page, serverId, workspaceId: second.workspaceId }); + await expectComposerVisible(page, { timeout: 30_000 }); + + await openAppWideNewWorkspace(page); + await switchWorkspaceViaSidebar({ page, serverId, workspaceId: third.workspaceId }); + await expectComposerVisible(page, { timeout: 30_000 }); + + await openAppWideNewWorkspace(page); + await switchWorkspaceViaSidebar({ page, serverId, workspaceId: first.workspaceId }); + await expectComposerVisible(page, { timeout: 30_000 }); + await expectSingleCurrentWorkspaceDeckEntry(page, { + expectedDeckEntryCount: sessions.length, + serverId, + workspaceId: first.workspaceId, + }); + + await composerLocator(page).fill("/"); + const popover = page + .getByTestId("composer-autocomplete-popover") + .filter({ hasText: "/help", visible: true }) + .first(); + await expect(popover).toBeInViewport({ timeout: 30_000 }); + } finally { + await Promise.allSettled(sessions.map((session) => session.cleanup())); + } + }); + test("does not flash at the wrong position on the first slash command paint", async ({ page, }) => { diff --git a/packages/app/src/app/_layout.tsx b/packages/app/src/app/_layout.tsx index 33f58c560..ab3370ec1 100644 --- a/packages/app/src/app/_layout.tsx +++ b/packages/app/src/app/_layout.tsx @@ -4,7 +4,13 @@ import { PortalProvider } from "@gorhom/portal"; import { QueryClientProvider } from "@tanstack/react-query"; import * as Linking from "expo-linking"; import * as Notifications from "expo-notifications"; -import { Stack, useGlobalSearchParams, usePathname, useRouter } from "expo-router"; +import { + Stack, + useGlobalSearchParams, + useNavigationContainerRef, + usePathname, + useRouter, +} from "expo-router"; import { createContext, type ReactNode, @@ -58,6 +64,7 @@ import { startHostRuntimeBootstrap, type StartupBlocker, } from "@/navigation/host-runtime-bootstrap"; +import { registerWorkspaceRouteNavigationRef } from "@/navigation/workspace-route-navigation"; import { shouldUseDesktopDaemon } from "@/desktop/daemon/desktop-daemon"; import { listenToDesktopEvent } from "@/desktop/electron/events"; import { updateDesktopWindowControls } from "@/desktop/electron/window"; @@ -949,12 +956,23 @@ function RootStack() { ); } +function WorkspaceRouteNavigationBridge() { + const navigationRef = useNavigationContainerRef(); + + useEffect(() => { + return registerWorkspaceRouteNavigationRef(navigationRef); + }, [navigationRef]); + + return null; +} + function AppShell() { return ( + diff --git a/packages/app/src/components/sidebar-workspace-list.tsx b/packages/app/src/components/sidebar-workspace-list.tsx index 4086cd384..99f4c369e 100644 --- a/packages/app/src/components/sidebar-workspace-list.tsx +++ b/packages/app/src/components/sidebar-workspace-list.tsx @@ -1749,13 +1749,14 @@ function WorkspaceRowItem({ isDragging = false, dragHandleProps, }: WorkspaceRowItemProps) { + const currentPathname = usePathname(); const handlePress = useCallback(() => { if (!workspace.serverId) { return; } onWorkspacePress?.(); - navigateToWorkspace(workspace.serverId, workspace.workspaceId); - }, [onWorkspacePress, workspace.serverId, workspace.workspaceId]); + navigateToWorkspace(workspace.serverId, workspace.workspaceId, { currentPathname }); + }, [currentPathname, onWorkspacePress, workspace.serverId, workspace.workspaceId]); return ( { if (!workspace.serverId) return; onWorkspacePress?.(); - navigateToWorkspace(workspace.serverId, workspace.workspaceId); - }, [onWorkspacePress, workspace.serverId, workspace.workspaceId]); + navigateToWorkspace(workspace.serverId, workspace.workspaceId, { currentPathname }); + }, [currentPathname, onWorkspacePress, workspace.serverId, workspace.workspaceId]); if (!workspaceEntry) return null; diff --git a/packages/app/src/navigation/workspace-route-navigation.ts b/packages/app/src/navigation/workspace-route-navigation.ts new file mode 100644 index 000000000..fc72a577e --- /dev/null +++ b/packages/app/src/navigation/workspace-route-navigation.ts @@ -0,0 +1,109 @@ +import type { NavigationAction, NavigationContainerRefWithCurrent } from "@react-navigation/native"; +import { router, type Href } from "expo-router"; +import { + encodeWorkspaceIdForPathSegment, + parseHostWorkspaceRouteFromPathname, +} from "@/utils/host-routes"; + +const ROOT_HOST_ROUTE_NAME = "h/[serverId]"; +const HOST_WORKSPACE_ROUTE_NAME = "workspace/[workspaceId]/index"; + +let rootNavigationRef: NavigationContainerRefWithCurrent | null = + null; + +interface NavigateToHostWorkspaceRouteOptions { + popToExistingHostRoute?: boolean; +} + +export function registerWorkspaceRouteNavigationRef( + ref: NavigationContainerRefWithCurrent, +): () => void { + rootNavigationRef = ref; + return () => { + if (rootNavigationRef === ref) { + rootNavigationRef = null; + } + }; +} + +function findStackKeyWithRouteName(state: unknown, routeName: string): string | null { + if (!state || typeof state !== "object") { + return null; + } + + const candidate = state as { + key?: unknown; + routeNames?: unknown; + routes?: unknown; + }; + if ( + typeof candidate.key === "string" && + Array.isArray(candidate.routeNames) && + candidate.routeNames.includes(routeName) + ) { + return candidate.key; + } + + if (!Array.isArray(candidate.routes)) { + return null; + } + + for (const route of candidate.routes) { + if (!route || typeof route !== "object") { + continue; + } + const childKey = findStackKeyWithRouteName((route as { state?: unknown }).state, routeName); + if (childKey) { + return childKey; + } + } + + return null; +} + +function dispatchHostWorkspacePopTo(route: string): boolean { + const selection = parseHostWorkspaceRouteFromPathname(route); + const navigation = rootNavigationRef?.current; + if (!selection || !navigation?.isReady()) { + return false; + } + + const rootState = navigation.getRootState(); + const target = findStackKeyWithRouteName(rootState, ROOT_HOST_ROUTE_NAME); + if (!target) { + return false; + } + + const action: NavigationAction = { + type: "POP_TO", + target, + payload: { + name: ROOT_HOST_ROUTE_NAME, + params: { + serverId: selection.serverId, + screen: HOST_WORKSPACE_ROUTE_NAME, + params: { + serverId: selection.serverId, + workspaceId: encodeWorkspaceIdForPathSegment(selection.workspaceId), + }, + // React Navigation consumes this nested hint when resolving the host child screen. + // The browser-route canonicalizer strips the resulting ?pop=true URL artifact. + // Removing it lets repeated /new -> workspace hops append hidden deck entries. + pop: true, + }, + }, + }; + navigation.dispatch(action); + return true; +} + +export function navigateToHostWorkspaceRoute( + route: string, + options: NavigateToHostWorkspaceRouteOptions = {}, +): void { + if (options.popToExistingHostRoute && dispatchHostWorkspacePopTo(route)) { + return; + } + + router.dismissTo(route as Href); +} diff --git a/packages/app/src/screens/new-workspace-screen.tsx b/packages/app/src/screens/new-workspace-screen.tsx index 931725e39..93defa90c 100644 --- a/packages/app/src/screens/new-workspace-screen.tsx +++ b/packages/app/src/screens/new-workspace-screen.tsx @@ -967,6 +967,7 @@ function submitWorkspaceDraft(input: SubmitDraftInput): void { navigateToPreparedWorkspaceTab({ serverId, workspaceId, + currentPathname: "/new", target: { kind: "draft", draftId }, }); useDraftStore.getState().clearDraftInput({ draftKey, lifecycle: "sent" }); @@ -1781,7 +1782,8 @@ export function NewWorkspaceScreen({ payload, ensureWorkspace, serverId: selectedServerId, - navigate: navigateToWorkspace, + navigate: (targetServerId, workspaceId) => + navigateToWorkspace(targetServerId, workspaceId, { currentPathname: "/new" }), }); return; } diff --git a/packages/app/src/stores/navigation-active-workspace-store/index.ts b/packages/app/src/stores/navigation-active-workspace-store/index.ts index 94f2300d0..d0655e747 100644 --- a/packages/app/src/stores/navigation-active-workspace-store/index.ts +++ b/packages/app/src/stores/navigation-active-workspace-store/index.ts @@ -1,5 +1,5 @@ import AsyncStorage from "@react-native-async-storage/async-storage"; -import { router, useLocalSearchParams, usePathname, type Href } from "expo-router"; +import { useLocalSearchParams, usePathname } from "expo-router"; import { useEffect, useSyncExternalStore } from "react"; import { createLastWorkspaceSelectionStore, @@ -15,6 +15,7 @@ import { import { useSessionStore } from "@/stores/session-store"; import { useWorkspaceLayoutStore } from "@/stores/workspace-layout-store"; import { stripHostWorkspaceRouteEchoSearchFromBrowserUrlAfterCommit } from "@/utils/host-route-browser"; +import { navigateToHostWorkspaceRoute } from "@/navigation/workspace-route-navigation"; export type { ActiveWorkspaceSelection } from "@/stores/last-workspace-selection"; @@ -33,7 +34,13 @@ const lastWorkspaceSelectionStore = createLastWorkspaceSelectionStore( lastWorkspaceSelectionStorage, ); -function navigateDeps(): NavigateToWorkspaceDeps { +function shouldPopToExistingHostRoute(options: NavigateToWorkspaceOptions): boolean { + // Only /new needs POP_TO to avoid hidden deck entries; regular workspace switches + // should use router navigation so route observers like the sidebar selection update. + return options.currentPathname === "/new"; +} + +function navigateDeps(options: NavigateToWorkspaceOptions): NavigateToWorkspaceDeps { return { getSessionWorkspaces: (serverId) => useSessionStore.getState().sessions[serverId]?.workspaces, getSessionAgents: (serverId) => @@ -43,7 +50,9 @@ function navigateDeps(): NavigateToWorkspaceDeps { }, rememberLastWorkspace: (selection) => lastWorkspaceSelectionStore.remember(selection), navigateToRoute: (route) => { - router.dismissTo(route as Href); + navigateToHostWorkspaceRoute(route, { + popToExistingHostRoute: shouldPopToExistingHostRoute(options), + }); stripHostWorkspaceRouteEchoSearchFromBrowserUrlAfterCommit(); }, }; @@ -64,14 +73,14 @@ export function getIsLastWorkspaceSelectionHydrated(): boolean { export function navigateToWorkspace( serverId: string, workspaceId: string, - _options: NavigateToWorkspaceOptions = {}, + options: NavigateToWorkspaceOptions = {}, ) { - navigateToWorkspacePure(serverId, workspaceId, navigateDeps()); + navigateToWorkspacePure(serverId, workspaceId, navigateDeps(options)); } export function navigateToLastWorkspace(): boolean { return navigateToLastWorkspacePure({ - ...navigateDeps(), + ...navigateDeps({}), getLastWorkspaceSelection: () => lastWorkspaceSelectionStore.getSelection(), }); } diff --git a/packages/app/src/utils/host-routes.test.ts b/packages/app/src/utils/host-routes.test.ts index ca5ec3ae7..c2db2032d 100644 --- a/packages/app/src/utils/host-routes.test.ts +++ b/packages/app/src/utils/host-routes.test.ts @@ -141,6 +141,22 @@ describe("workspace route parsing", () => { ); }); + it("strips the React Navigation nested pop hint from workspace route search params", () => { + expect(stripHostWorkspaceRouteEchoSearch("/h/local/workspace/164?pop=true")).toBe( + "/h/local/workspace/164", + ); + expect( + stripHostWorkspaceRouteEchoSearch("/h/local/workspace/164?pop=true&open=agent%3Aagent-1"), + ).toBe("/h/local/workspace/164?open=agent%3Aagent-1"); + }); + + it("keeps non-navigation pop search params", () => { + expect(stripHostWorkspaceRouteEchoSearch("/h/local/workspace/164?pop=false")).toBe( + "/h/local/workspace/164?pop=false", + ); + expect(stripHostWorkspaceRouteEchoSearch("/new?pop=true")).toBe("/new?pop=true"); + }); + it("strips encoded workspace route echoes", () => { expect( stripHostWorkspaceRouteEchoSearch( diff --git a/packages/app/src/utils/host-routes.ts b/packages/app/src/utils/host-routes.ts index 999ef60d5..0b8dfcbb6 100644 --- a/packages/app/src/utils/host-routes.ts +++ b/packages/app/src/utils/host-routes.ts @@ -326,6 +326,11 @@ export function stripHostWorkspaceRouteEchoSearch(route: string): string { didStrip = true; } + if (params.get("pop") === "true") { + params.delete("pop"); + didStrip = true; + } + if (!didStrip) { return route; }