diff --git a/packages/app/e2e/sidebar-workspace-pin-shortcut.spec.ts b/packages/app/e2e/sidebar-workspace-pin-shortcut.spec.ts new file mode 100644 index 000000000..b9a0bb89e --- /dev/null +++ b/packages/app/e2e/sidebar-workspace-pin-shortcut.spec.ts @@ -0,0 +1,266 @@ +import { test, expect, type Page } from "./fixtures"; +import { gotoAppShell } from "./helpers/app"; +import { daemonWsRoutePattern } from "./helpers/daemon-port"; +import { seedWorkspace, type SeededWorkspace } from "./helpers/seed-client"; +import { getServerId } from "./helpers/server-id"; + +// The pin shortcut used to be registered by the sidebar row itself, so it silently did nothing +// whenever the row was unmounted — a collapsed project section being the common case. It now +// lives in a single always-mounted handler keyed on the active route selection. +const PIN_SHORTCUT = "ControlOrMeta+Shift+P"; + +function workspaceRow(page: Page, workspaceId: string) { + return page.getByTestId(`sidebar-workspace-row-${getServerId()}:${workspaceId}`); +} + +function pinnedSection(page: Page) { + return page.getByTestId("sidebar-pinned-section"); +} + +// Opens the workspace so it becomes the active route selection, which is what the shortcut acts on. +async function openWorkspace(page: Page, workspaceId: string) { + const row = workspaceRow(page, workspaceId); + await expect(row).toBeVisible({ timeout: 30_000 }); + await row.click(); + await expect(page).toHaveURL(/\/workspace\//, { timeout: 30_000 }); +} + +// The project key is host-scoped and not exposed by the seed helper, so the header is addressed by +// its display name, scoped to project rows so a workspace row can never match. Pressing the header +// toggles the section, which unmounts every workspace row under it. +async function collapseProjectSection(page: Page, project: SeededWorkspace): Promise { + const header = page + .locator('[data-testid^="sidebar-project-row-"]') + .filter({ hasText: project.projectDisplayName }); + await expect(header).toHaveCount(1, { timeout: 30_000 }); + + await header.click(); + await expect(workspaceRow(page, project.workspaceId)).toHaveCount(0, { timeout: 10_000 }); +} + +async function switchToStatusGrouping(page: Page): Promise { + await page.getByTestId("sidebar-display-preferences-menu").click(); + await page.getByTestId("sidebar-grouping-status").click(); + await expect(page.getByTestId("sidebar-status-list-scroll")).toBeVisible({ timeout: 10_000 }); +} + +// Status mode buckets workspaces by state rather than project, so the group holding this workspace +// is discovered from the rows container it sits in rather than assumed. +async function collapseStatusGroupContaining(page: Page, workspaceId: string): Promise { + const rows = page + .locator('[data-testid^="sidebar-status-group-rows-"]') + .filter({ has: workspaceRow(page, workspaceId) }); + await expect(rows).toHaveCount(1, { timeout: 30_000 }); + + const rowsTestId = await rows.getAttribute("data-testid"); + const bucket = rowsTestId?.replace("sidebar-status-group-rows-", ""); + expect(bucket).toBeTruthy(); + + await page.getByTestId(`sidebar-status-group-${bucket}`).click(); + await expect(workspaceRow(page, workspaceId)).toHaveCount(0, { timeout: 10_000 }); +} + +function readSessionMessage( + message: string | Buffer, +): { type?: unknown; requestId?: unknown } | null { + const raw = typeof message === "string" ? message : message.toString("utf8"); + try { + const envelope = JSON.parse(raw) as { type?: unknown; message?: unknown }; + if (envelope.type !== "session" || typeof envelope.message !== "object") { + return null; + } + return envelope.message as { type?: unknown; requestId?: unknown }; + } catch { + return null; + } +} + +const PIN_REJECTION_MESSAGE = "Pin rejected by test."; + +interface PinRpcGate { + /** Pin requests the client has sent so far. */ + sentCount(): number; +} + +// Proxies everything so the app boots against the real daemon, counting pin RPCs and optionally +// rejecting the first `rejectFirst` of them. The count asserts how many pins one keypress actually +// dispatched, which the rendered pin state cannot show — a toggle that fired twice lands back +// where it started. +async function installPinRpcGate( + page: Page, + options: { rejectFirst?: number } = {}, +): Promise { + const rejectFirst = options.rejectFirst ?? 0; + let sent = 0; + + await page.routeWebSocket(daemonWsRoutePattern(), (ws) => { + const server = ws.connectToServer(); + + ws.onMessage((message) => { + const sessionMessage = readSessionMessage(message); + if ( + sessionMessage?.type === "workspace.pin.set.request" && + typeof sessionMessage.requestId === "string" + ) { + sent += 1; + if (sent <= rejectFirst) { + ws.send( + JSON.stringify({ + type: "session", + message: { + type: "rpc_error", + payload: { + requestId: sessionMessage.requestId, + requestType: "workspace.pin.set.request", + error: PIN_REJECTION_MESSAGE, + code: "transport", + }, + }, + }), + ); + return; + } + } + + try { + server.send(message); + } catch { + // server socket already closed + } + }); + + server.onMessage((message) => { + try { + ws.send(message); + } catch { + // client socket already closed + } + }); + }); + + return { sentCount: () => sent }; +} + +test.describe("Pin workspace shortcut", () => { + test("pins the active workspace while its project section is collapsed", async ({ page }) => { + const workspace = await seedWorkspace({ repoPrefix: "pin-shortcut-collapsed-" }); + + try { + await gotoAppShell(page); + await openWorkspace(page, workspace.workspaceId); + await collapseProjectSection(page, workspace); + + await page.keyboard.press(PIN_SHORTCUT); + + await expect(pinnedSection(page)).toBeVisible({ timeout: 10_000 }); + await expect( + pinnedSection(page).getByTestId( + `sidebar-workspace-row-${getServerId()}:${workspace.workspaceId}`, + ), + ).toBeVisible(); + } finally { + await workspace.cleanup(); + } + }); + + test("unpins the active workspace while the Pinned section is collapsed", async ({ page }) => { + const workspace = await seedWorkspace({ repoPrefix: "pin-shortcut-unpin-" }); + + try { + await gotoAppShell(page); + await openWorkspace(page, workspace.workspaceId); + + await page.keyboard.press(PIN_SHORTCUT); + await expect(pinnedSection(page)).toBeVisible({ timeout: 10_000 }); + + await page.getByTestId("sidebar-pinned-section-header").click(); + await expect(workspaceRow(page, workspace.workspaceId)).toHaveCount(0, { timeout: 10_000 }); + + await page.keyboard.press(PIN_SHORTCUT); + + await expect(pinnedSection(page)).toHaveCount(0, { timeout: 10_000 }); + } finally { + await workspace.cleanup(); + } + }); + + test("sends exactly one pin RPC per press when the row is rendered and selected", async ({ + page, + }) => { + const workspace = await seedWorkspace({ repoPrefix: "pin-shortcut-expanded-" }); + + try { + const gate = await installPinRpcGate(page); + + await gotoAppShell(page); + await openWorkspace(page, workspace.workspaceId); + + await page.keyboard.press(PIN_SHORTCUT); + await expect(pinnedSection(page)).toBeVisible({ timeout: 10_000 }); + // Counting frames catches a press that produces zero or two RPCs — a misfiring in-flight + // guard, or a second dispatch path. It cannot detect a duplicate handler registration: + // `keyboardActionDispatcher.dispatch` returns at the first handler that returns true, so a + // shadowed second handler is unobservable from outside by design. + expect(gate.sentCount()).toBe(1); + + await page.keyboard.press(PIN_SHORTCUT); + await expect(pinnedSection(page)).toHaveCount(0, { timeout: 10_000 }); + expect(gate.sentCount()).toBe(2); + await expect(workspaceRow(page, workspace.workspaceId)).toHaveCount(1); + } finally { + await workspace.cleanup(); + } + }); + + test("pins the active workspace while its status group is collapsed", async ({ page }) => { + const workspace = await seedWorkspace({ repoPrefix: "pin-shortcut-status-" }); + + try { + await gotoAppShell(page); + await openWorkspace(page, workspace.workspaceId); + await switchToStatusGrouping(page); + await collapseStatusGroupContaining(page, workspace.workspaceId); + + await page.keyboard.press(PIN_SHORTCUT); + + await expect(pinnedSection(page)).toBeVisible({ timeout: 10_000 }); + await expect( + pinnedSection(page).getByTestId( + `sidebar-workspace-row-${getServerId()}:${workspace.workspaceId}`, + ), + ).toBeVisible(); + } finally { + await workspace.cleanup(); + } + }); + + test("shows an error toast when the host rejects the pin, and the next press succeeds", async ({ + page, + }) => { + const workspace = await seedWorkspace({ repoPrefix: "pin-shortcut-failure-" }); + + try { + const gate = await installPinRpcGate(page, { rejectFirst: 1 }); + + await gotoAppShell(page); + await openWorkspace(page, workspace.workspaceId); + + await page.keyboard.press(PIN_SHORTCUT); + + await expect(page.getByTestId("app-toast-message")).toContainText(PIN_REJECTION_MESSAGE, { + timeout: 10_000, + }); + await expect(pinnedSection(page)).toHaveCount(0); + await expect(workspaceRow(page, workspace.workspaceId)).toHaveCount(1); + + // The failure must leave the action usable: the in-flight guard has to release the key so a + // retry is not swallowed. Without that release the workspace is unpinnable for the session. + await page.keyboard.press(PIN_SHORTCUT); + + await expect(pinnedSection(page)).toBeVisible({ timeout: 10_000 }); + expect(gate.sentCount()).toBe(2); + } finally { + await workspace.cleanup(); + } + }); +}); diff --git a/packages/app/src/app/_layout.tsx b/packages/app/src/app/_layout.tsx index 5591949f0..febae56d9 100644 --- a/packages/app/src/app/_layout.tsx +++ b/packages/app/src/app/_layout.tsx @@ -32,6 +32,7 @@ import { AppDiagnosticHost } from "@/components/app-diagnostic-host"; import { LeftSidebar } from "@/components/left-sidebar"; import { WindowSidebarMenuToggle } from "@/components/headers/menu-header"; import { SidebarModelProvider } from "@/components/sidebar/sidebar-model"; +import { WorkspacePinShortcutHandler } from "@/components/workspace-pin-shortcut-handler"; import { CompactExplorerSidebarHost } from "@/components/compact-explorer-sidebar-host"; import { ProviderSettingsHost } from "@/components/provider-settings-host"; import { RootErrorBoundary } from "@/components/root-error-boundary"; @@ -551,6 +552,7 @@ function AppContainer({ children, chromeEnabled: chromeEnabledOverride }: AppCon + diff --git a/packages/app/src/components/sidebar-workspace-list.tsx b/packages/app/src/components/sidebar-workspace-list.tsx index 9acba693e..36ffc8951 100644 --- a/packages/app/src/components/sidebar-workspace-list.tsx +++ b/packages/app/src/components/sidebar-workspace-list.tsx @@ -1368,17 +1368,6 @@ function WorkspaceRowWithMenu({ }, }); - useKeyboardActionHandler({ - handlerId: `workspace-pin-${workspace.workspaceKey}`, - actions: ["workspace.pin"], - enabled: selected && canPin, - priority: 0, - handle: () => { - onTogglePin?.(); - return true; - }, - }); - return ( <> { - onTogglePin?.(); - return true; - }, - }); - return ( <> ({ + id: workspace.id, + pinnedAt: workspace.pinnedAt ?? null, + })); + const canPin = useHostFeature(serverId, "workspacePinning"); + const togglePin = useSidebarWorkspacePinController(); + + const handle = useCallback(() => { + if (!serverId || !fields || !canPin) { + return false; + } + const workspaceKey = buildWorkspaceTabPersistenceKey({ + serverId, + workspaceId: fields.id, + }); + if (!workspaceKey) { + return false; + } + togglePin({ + serverId, + workspaceId: fields.id, + workspaceKey, + pinnedAt: fields.pinnedAt, + }); + return true; + }, [canPin, fields, serverId, togglePin]); + + useKeyboardActionHandler({ + handlerId: "workspace-pin-global", + actions: WORKSPACE_PIN_ACTIONS, + enabled: serverId !== null && fields !== null && canPin, + priority: 0, + handle, + }); +} diff --git a/packages/app/src/hooks/use-sidebar-workspace-pin.ts b/packages/app/src/hooks/use-sidebar-workspace-pin.ts index 2019442fb..9edb09f8b 100644 --- a/packages/app/src/hooks/use-sidebar-workspace-pin.ts +++ b/packages/app/src/hooks/use-sidebar-workspace-pin.ts @@ -1,22 +1,33 @@ -import { useCallback, useRef } from "react"; +import { useCallback } from "react"; import { useTranslation } from "react-i18next"; import { useMutation } from "@tanstack/react-query"; import { useToast } from "@/contexts/toast-context"; import type { SidebarWorkspaceEntry } from "@/hooks/use-sidebar-workspaces-list"; import { getHostRuntimeStore } from "@/runtime/host-runtime"; -export type ToggleSidebarWorkspacePin = (workspace: SidebarWorkspaceEntry) => void; +// Everything the pin toggle actually needs. Kept narrower than SidebarWorkspaceEntry so the +// global keyboard handler can build one from the active route selection without a sidebar row. +export type PinnableWorkspace = Pick< + SidebarWorkspaceEntry, + "serverId" | "workspaceId" | "workspaceKey" | "pinnedAt" +>; + +export type ToggleSidebarWorkspacePin = (workspace: PinnableWorkspace) => void; + +// Module scope, not a per-hook ref: the sidebar row menus and the global keyboard shortcut each +// hold their own controller instance, and a per-instance guard would let a keypress and a menu +// click fire two concurrent, opposite setWorkspacePinned calls for the same workspace. +const pendingWorkspaceKeys = new Set(); export function useSidebarWorkspacePinController(): ToggleSidebarWorkspacePin { const { t } = useTranslation(); const toast = useToast(); - const pendingWorkspaceKeysRef = useRef(new Set()); const mutation = useMutation({ mutationFn: async ({ workspace, pinned, }: { - workspace: SidebarWorkspaceEntry; + workspace: PinnableWorkspace; pinned: boolean; }) => { const client = getHostRuntimeStore().getClient(workspace.serverId); @@ -31,17 +42,17 @@ export function useSidebarWorkspacePinController(): ToggleSidebarWorkspacePin { ); }, onSettled: (_data, _error, { workspace }) => { - pendingWorkspaceKeysRef.current.delete(workspace.workspaceKey); + pendingWorkspaceKeys.delete(workspace.workspaceKey); }, }); const mutate = mutation.mutate; return useCallback( - (workspace: SidebarWorkspaceEntry) => { - if (pendingWorkspaceKeysRef.current.has(workspace.workspaceKey)) { + (workspace: PinnableWorkspace) => { + if (pendingWorkspaceKeys.has(workspace.workspaceKey)) { return; } - pendingWorkspaceKeysRef.current.add(workspace.workspaceKey); + pendingWorkspaceKeys.add(workspace.workspaceKey); mutate({ workspace, pinned: workspace.pinnedAt == null }); }, [mutate],