From 6db261f91142b76c57588d82eadacca7cf94cedf Mon Sep 17 00:00:00 2001 From: Mohamed Boudra Date: Tue, 18 Aug 2026 14:04:02 +0200 Subject: [PATCH] fix(app): keep Cmd+T working after switching workspaces MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The new-tab menu handler registered itself as `workspace-new-tab-menu::`, leaving out the workspace. Pane ids are namespaced per workspace — every workspace's default pane is called "main" — so all workspaces on a host produced the same handler id. The deck keeps several workspaces mounted at once, the dispatcher stores one live handler per id, and useKeyboardActionHandler registers once per id and never re-registers on focus change. So the second workspace to mount evicted the first, and switching back left a mounted, focused workspace with no handler at all: Cmd+T and Cmd+Shift+A/T/C/E silently did nothing until the newer entry expired from the deck. Handler ids are now built from the surface's full identity rather than assembled at the call site, so the workspace can't be dropped again. The three workspace-wide handlers move onto the same constructor; they were already correct, but the shape belongs in one place. --- packages/app/e2e/browser/launcher-tab.spec.ts | 48 ++++++++++++++ packages/app/src/keyboard/handler-id.test.ts | 62 +++++++++++++++++++ packages/app/src/keyboard/handler-id.ts | 33 ++++++++++ .../workspace/workspace-desktop-tabs-row.tsx | 8 ++- .../screens/workspace/workspace-screen.tsx | 19 +++++- 5 files changed, 166 insertions(+), 4 deletions(-) create mode 100644 packages/app/src/keyboard/handler-id.test.ts create mode 100644 packages/app/src/keyboard/handler-id.ts diff --git a/packages/app/e2e/browser/launcher-tab.spec.ts b/packages/app/e2e/browser/launcher-tab.spec.ts index 70a269ba8..59bf8d71b 100644 --- a/packages/app/e2e/browser/launcher-tab.spec.ts +++ b/packages/app/e2e/browser/launcher-tab.spec.ts @@ -9,6 +9,8 @@ import { clickNewChat, clickNewTerminal, countTabsOfKind, + waitForTabBar, + pressNewTabShortcut, getTabTestIds, waitForTabWithTitle, measureTileTransition, @@ -27,10 +29,14 @@ import { seedTerminalProfiles, type TerminalProfile, } from "../support/helpers/new-workspace-launch"; +import { gotoAppShell } from "../support/helpers/app"; +import { waitForSidebarHydration } from "../support/helpers/workspace-ui"; +import { getServerId } from "../support/helpers/server-id"; // ─── Shared state ────────────────────────────────────────────────────────── let workspace: SeededWorkspace; +let secondWorkspaceId: string | null = null; const EMPTY_PROMPT_PROFILE: TerminalProfile = { id: "e2e-empty-prompt", @@ -41,6 +47,14 @@ const EMPTY_PROMPT_PROFILE: TerminalProfile = { test.beforeAll(async () => { workspace = await seedWorkspace({ repoPrefix: "launcher-e2e-" }); + const created = await workspace.client.createWorkspace({ + source: { kind: "directory", path: workspace.repoPath, projectId: workspace.projectId }, + title: "launcher-e2e-cmd-t-switch-target", + }); + if (!created.workspace) { + throw new Error(created.error ?? "Failed to create secondary workspace for cmd+t switch test"); + } + secondWorkspaceId = created.workspace.id; }); test.afterAll(async () => { @@ -52,6 +66,40 @@ test.afterAll(async () => { // ═══════════════════════════════════════════════════════════════════════════ test.describe("Tab creation", () => { + test("Cmd+T keeps opening the New tab menu after workspace switches", async ({ page }) => { + if (!secondWorkspaceId) { + throw new Error("Secondary workspace was not created"); + } + + const serverId = getServerId(); + const newTabMenuItem = page.getByRole("menuitem", { name: /^Agent/ }); + + const switchWorkspaceRow = async (workspaceId: string) => { + const row = page.getByTestId(`sidebar-workspace-row-${serverId}:${workspaceId}`).first(); + await expect(row).toBeVisible({ timeout: 30_000 }); + await row.click(); + // The shortcut must follow the route, so prove the route actually moved first. + await expect(page).toHaveURL(new RegExp(`workspace/${workspaceId}(\\b|/|$)`), { + timeout: 15_000, + }); + await waitForTabBar(page); + }; + + await gotoAppShell(page); + await waitForSidebarHydration(page); + + const sequence = [workspace.workspaceId, secondWorkspaceId]; + for (let i = 0; i < 8; i++) { + await switchWorkspaceRow(sequence[i % sequence.length]); + await pressNewTabShortcut(page); + await expect(newTabMenuItem, `New tab menu did not open after switch ${i}`).toBeVisible({ + timeout: 5_000, + }); + await page.keyboard.press("Escape"); + await expect(newTabMenuItem).toBeHidden({ timeout: 5_000 }); + } + }); + test("retained inactive workspaces cannot own New tab shortcuts", async ({ page }) => { await gotoWorkspace(page, workspace.workspaceId); const workspaceUrl = page.url(); diff --git a/packages/app/src/keyboard/handler-id.test.ts b/packages/app/src/keyboard/handler-id.test.ts new file mode 100644 index 000000000..27eac22a5 --- /dev/null +++ b/packages/app/src/keyboard/handler-id.test.ts @@ -0,0 +1,62 @@ +import { describe, expect, it } from "vitest"; +import { buildWorkspaceKeyboardHandlerId } from "@/keyboard/handler-id"; + +describe("buildWorkspaceKeyboardHandlerId", () => { + it("separates workspaces that share a pane id", () => { + // Every workspace's default pane is called "main", and the deck keeps several + // workspaces mounted at once. Dropping the workspace id here left the evicted + // workspace without a keyboard. + const left = buildWorkspaceKeyboardHandlerId({ + name: "workspace-new-tab-menu", + serverId: "srv", + workspaceId: "wks_a", + paneId: "main", + }); + const right = buildWorkspaceKeyboardHandlerId({ + name: "workspace-new-tab-menu", + serverId: "srv", + workspaceId: "wks_b", + paneId: "main", + }); + + expect(left).not.toBe(right); + }); + + it("separates panes within a workspace", () => { + const main = buildWorkspaceKeyboardHandlerId({ + name: "workspace-new-tab-menu", + serverId: "srv", + workspaceId: "wks_a", + paneId: "main", + }); + const explorer = buildWorkspaceKeyboardHandlerId({ + name: "workspace-new-tab-menu", + serverId: "srv", + workspaceId: "wks_a", + paneId: "explorer", + }); + + expect(main).not.toBe(explorer); + }); + + it("omits the pane segment for workspace-wide surfaces", () => { + expect( + buildWorkspaceKeyboardHandlerId({ + name: "workspace-tab-actions", + serverId: "srv", + workspaceId: "wks_a", + }), + ).toBe("workspace-tab-actions:srv:wks_a"); + }); + + it("treats a blank pane id as workspace-wide", () => { + expect( + buildWorkspaceKeyboardHandlerId({ + name: "workspace-new-tab-menu", + serverId: "srv", + workspaceId: "wks_a", + paneId: " ", + }), + ).toBe("workspace-new-tab-menu:srv:wks_a"); + }); +}); diff --git a/packages/app/src/keyboard/handler-id.ts b/packages/app/src/keyboard/handler-id.ts new file mode 100644 index 000000000..5de6b9215 --- /dev/null +++ b/packages/app/src/keyboard/handler-id.ts @@ -0,0 +1,33 @@ +/** + * A keyboard handler id is an identity, not a label. + * + * The dispatcher keeps one live handler per id and useKeyboardActionHandler + * registers once per id, never re-registering when focus changes. So when two + * mounted surfaces produce the same id, the second one evicts the first and the + * first is left without a keyboard for the rest of its mount — silently, with no + * symptom until someone presses the key. + * + * The workspace deck keeps several workspaces mounted at once, so a workspace + * surface is only identified by its server, its workspace, and its pane. Pane + * ids are namespaced per workspace ("main" is every workspace's default pane), + * which is why they cannot carry the identity on their own. + */ +export interface WorkspaceKeyboardSurface { + /** Stable name for the surface, e.g. "workspace-new-tab-menu". */ + name: string; + serverId: string; + workspaceId: string; + /** Omit for surfaces that own the whole workspace rather than one pane. */ + paneId?: string | null; +} + +export function buildWorkspaceKeyboardHandlerId({ + name, + serverId, + workspaceId, + paneId, +}: WorkspaceKeyboardSurface): string { + const trimmedPaneId = paneId?.trim(); + const paneSegment = trimmedPaneId ? `:${trimmedPaneId}` : ""; + return `${name}:${serverId}:${workspaceId}${paneSegment}`; +} diff --git a/packages/app/src/screens/workspace/workspace-desktop-tabs-row.tsx b/packages/app/src/screens/workspace/workspace-desktop-tabs-row.tsx index 5e463ef90..540f6b4a9 100644 --- a/packages/app/src/screens/workspace/workspace-desktop-tabs-row.tsx +++ b/packages/app/src/screens/workspace/workspace-desktop-tabs-row.tsx @@ -104,6 +104,7 @@ import { useWorkspaceDirectory } from "@/stores/session-store-hooks"; import { useCheckoutStatusQuery } from "@/git/use-status-query"; import { TrailingActionScrim } from "@/components/ui/trailing-action-scrim"; import { useKeyboardActionHandler } from "@/hooks/use-keyboard-action-handler"; +import { buildWorkspaceKeyboardHandlerId } from "@/keyboard/handler-id"; import type { KeyboardActionDefinition } from "@/keyboard/keyboard-action-dispatcher"; const DROPDOWN_WIDTH = 220; @@ -1489,7 +1490,12 @@ function ResolvedWorkspaceDesktopTabsRow({ ); useKeyboardActionHandler({ - handlerId: `workspace-new-tab-menu:${normalizedServerId}:${paneId ?? "main"}`, + handlerId: buildWorkspaceKeyboardHandlerId({ + name: "workspace-new-tab-menu", + serverId: normalizedServerId, + workspaceId: normalizedWorkspaceId, + paneId, + }), actions: [ "workspace.tab.menu.open", "workspace.tab.target.agent", diff --git a/packages/app/src/screens/workspace/workspace-screen.tsx b/packages/app/src/screens/workspace/workspace-screen.tsx index ab5278904..a2b3c1395 100644 --- a/packages/app/src/screens/workspace/workspace-screen.tsx +++ b/packages/app/src/screens/workspace/workspace-screen.tsx @@ -67,6 +67,7 @@ import { type WorkspaceTabTarget, } from "@/workspace-tabs/model"; import { useKeyboardActionHandler } from "@/hooks/use-keyboard-action-handler"; +import { buildWorkspaceKeyboardHandlerId } from "@/keyboard/handler-id"; import type { KeyboardActionDefinition } from "@/keyboard/keyboard-action-dispatcher"; import { useCreateFlowStore } from "@/stores/create-flow-store"; import { @@ -3042,7 +3043,11 @@ function WorkspaceScreenContent({ ); useKeyboardActionHandler({ - handlerId: `workspace-tab-actions:${normalizedServerId}:${normalizedWorkspaceId}`, + handlerId: buildWorkspaceKeyboardHandlerId({ + name: "workspace-tab-actions", + serverId: normalizedServerId, + workspaceId: normalizedWorkspaceId, + }), actions: [ "workspace.agent.new", "workspace.tab.close-current", @@ -3058,7 +3063,11 @@ function WorkspaceScreenContent({ }); useKeyboardActionHandler({ - handlerId: `workspace-pane-actions:${normalizedServerId}:${normalizedWorkspaceId}`, + handlerId: buildWorkspaceKeyboardHandlerId({ + name: "workspace-pane-actions", + serverId: normalizedServerId, + workspaceId: normalizedWorkspaceId, + }), actions: [ "workspace.pane.split.right", "workspace.pane.split.down", @@ -3080,7 +3089,11 @@ function WorkspaceScreenContent({ }); useKeyboardActionHandler({ - handlerId: `workspace-sidebar-actions:${normalizedServerId}:${normalizedWorkspaceId}`, + handlerId: buildWorkspaceKeyboardHandlerId({ + name: "workspace-sidebar-actions", + serverId: normalizedServerId, + workspaceId: normalizedWorkspaceId, + }), actions: ["sidebar.toggle.right"] as const, enabled: Boolean(isRouteFocused && normalizedServerId && normalizedWorkspaceId), priority: 100,