From 96573d0bcc87a9abe04e3deb0cea5c8e9816416d Mon Sep 17 00:00:00 2001 From: Mohamed Boudra Date: Sun, 28 Jun 2026 10:01:01 +0700 Subject: [PATCH] Keep slash commands visible after New Workspace (#1760) * fix(app): keep slash commands visible after New Workspace Repeated returns from the app-wide New Workspace route were using dismissTo with the workspace leaf URL. That updated the root URL without popping the nested host stack, so hidden duplicate workspace deck entries could remain mounted and steal composer popover measurements. Dispatch a root-stack POP_TO into the host workspace route instead, with a Playwright regression for the slash-command popover. * test(app): tighten workspace autocomplete regression Derive the expected deck count from the seeded workspaces and make the duplicate-deck assertion hard. Keep the Expo Router pop hint documented because removing it reproduces the hidden deck entries. --- docs/development.md | 9 ++ .../app/e2e/composer-autocomplete.spec.ts | 103 ++++++++++++++++- packages/app/src/app/_layout.tsx | 20 +++- .../src/components/sidebar-workspace-list.tsx | 5 +- .../sidebar/sidebar-status-list.tsx | 6 +- .../navigation/workspace-route-navigation.ts | 109 ++++++++++++++++++ .../app/src/screens/new-workspace-screen.tsx | 4 +- .../index.ts | 21 +++- packages/app/src/utils/host-routes.test.ts | 16 +++ packages/app/src/utils/host-routes.ts | 5 + 10 files changed, 285 insertions(+), 13 deletions(-) create mode 100644 packages/app/src/navigation/workspace-route-navigation.ts 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; }