fix: prevent Shift+Tab from changing a backgrounded agent's mode (#1848)

* fix: prevent Shift+Tab from changing a backgrounded agent's mode

The New Workspace mode-cycle shortcut (Shift+Tab) could reach a
still-mounted background agent's mode control and silently change that
running agent's execution mode — including into a permissive/bypass
mode — with no feedback in the New Workspace UI.

Root cause is in useKeyboardActionHandler: it re-registered its handler
on every render (a fresh inline `actions` array plus a changing `handle`
identity). The dispatcher runs the most-recently-registered matching
handler first, so a frequently re-rendering control climbed ahead of the
active one and consumed the key. The registered `handle`/`enabled` were
also captured per registration and read stale by the native keydown
listener.

Fix: register once per (handlerId, priority, actions); read `handle` and
`isActive` live from a ref; fold `enabled` into a fresh `isActive` so
the dispatcher re-checks it at dispatch time. Registration order stays
stable and callbacks stay current for all six consumers of the hook.

Add an e2e regression test that opens a live agent, opens New
Workspace, and presses Shift+Tab. It asserts the running agent's mode
is unchanged: its daemon-committed mode, plus no set_agent_mode_request
on the wire. It fails on the previous code (the mode was flipped to a
more permissive mode) and passes with this change.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(app): restore workspace pin shortcut build

The shortcut change landed with an import path removed by the workspace tabs reshape, breaking app typecheck and web builds. Import the canonical workspace tab model.

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-authored-by: Mohamed Boudra <boudra.moha@gmail.com>

packages/app/e2e/new-workspace-mode-cycle-safety.spec.ts

@@ -0,0 +1,146 @@
+import { expect, test, type Page } from "./fixtures";
+import { daemonWsRoutePattern } from "./helpers/daemon-port";
+import { openAgentRoute } from "./helpers/mock-agent";
+import { openGlobalNewWorkspaceComposer, selectNewWorkspaceProject } from "./helpers/new-workspace";
+import { seedWorkspace } from "./helpers/seed-client";
+import { getServerId } from "./helpers/server-id";
+
+const CREATE_AGENT_PREFERENCES_KEY = "@paseo:create-agent-preferences";
+
+type WebSocketMessage = string | Buffer;
+
+function parseWebSocketJson(message: WebSocketMessage): unknown {
+  const rawMessage = typeof message === "string" ? message : message.toString("utf8");
+  try {
+    return JSON.parse(rawMessage);
+  } catch {
+    return null;
+  }
+}
+
+function getSessionMessage(message: WebSocketMessage): Record<string, unknown> | null {
+  const envelope = parseWebSocketJson(message);
+  if (!envelope || typeof envelope !== "object") {
+    return null;
+  }
+  const maybeEnvelope = envelope as { type?: unknown; message?: unknown };
+  if (maybeEnvelope.type !== "session" || !maybeEnvelope.message) {
+    return null;
+  }
+  if (typeof maybeEnvelope.message !== "object") {
+    return null;
+  }
+  return maybeEnvelope.message as Record<string, unknown>;
+}
+
+// The draft mode control in New Workspace only mutates local form state; it never sends
+// set_agent_mode_request. So the only source of such a request while the New Workspace
+// composer is focused is a *live* agent's mode control. Recording those, keyed by agentId,
+// gives a direct signal that Shift+Tab leaked into a backgrounded agent.
+async function recordSetAgentModeRequests(page: Page): Promise<{
+  requestsForAgent(agentId: string): Array<{ agentId: string; modeId: string }>;
+}> {
+  const seen: Array<{ agentId: string; modeId: string }> = [];
+  await page.routeWebSocket(daemonWsRoutePattern(), (ws) => {
+    const server = ws.connectToServer();
+    ws.onMessage((message) => {
+      const sessionMessage = getSessionMessage(message);
+      if (sessionMessage?.type === "set_agent_mode_request") {
+        const agentId = typeof sessionMessage.agentId === "string" ? sessionMessage.agentId : "";
+        const modeId = typeof sessionMessage.modeId === "string" ? sessionMessage.modeId : "";
+        seen.push({ agentId, modeId });
+      }
+      server.send(message);
+    });
+    server.onMessage((message) => ws.send(message));
+  });
+  return {
+    requestsForAgent: (agentId: string) => seen.filter((request) => request.agentId === agentId),
+  };
+}
+
+async function seedCodexDefaultPreferences(page: Page, serverId: string): Promise<void> {
+  await page.addInitScript(
+    ({ preferencesKey, serverId: seededServerId }) => {
+      localStorage.setItem(
+        preferencesKey,
+        JSON.stringify({
+          serverId: seededServerId,
+          provider: "codex",
+          providerPreferences: {
+            codex: {
+              model: "gpt-5.4-mini",
+              mode: "auto",
+              thinkingByModel: { "gpt-5.4-mini": "low" },
+            },
+            mock: { model: "ten-second-stream" },
+          },
+        }),
+      );
+    },
+    { preferencesKey: CREATE_AGENT_PREFERENCES_KEY, serverId },
+  );
+}
+
+// Focus the New Workspace composer and cycle the execution mode with the keyboard.
+// Kept out of the test body so the test reads as intent rather than key mechanics.
+async function cycleNewWorkspaceMode(page: Page, presses: number): Promise<void> {
+  const composer = page.getByRole("textbox", { name: "Message agent..." });
+  await expect(composer).toBeVisible({ timeout: 30_000 });
+  await composer.click();
+  for (let i = 0; i < presses; i++) {
+    await page.keyboard.press("Shift+Tab");
+  }
+}
+
+test.describe("New Workspace mode cycle safety", () => {
+  test.describe.configure({ timeout: 240_000 });
+
+  // Regression guard for the P1 safety bug: cycling the execution mode with Shift+Tab in
+  // the New Workspace composer must never reach a backgrounded, still-mounted agent's mode
+  // control and silently change that (possibly running) agent's mode — e.g. into a
+  // permissive/bypass mode. See use-keyboard-action-handler.ts.
+  test("Shift+Tab in New Workspace never changes a backgrounded agent's mode", async ({ page }) => {
+    const serverId = getServerId();
+    const seeded = await seedWorkspace({ repoPrefix: "mode-cycle-safety-" });
+    await seedCodexDefaultPreferences(page, serverId);
+    const modeRequests = await recordSetAgentModeRequests(page);
+
+    try {
+      const agent = await seeded.client.createAgent({
+        provider: "codex",
+        cwd: seeded.repoPath,
+        workspaceId: seeded.workspaceId,
+        title: "mode cycle safety e2e",
+        modeId: "auto",
+        model: "gpt-5.4-mini",
+      });
+
+      // Mount the live agent tab: its mode control registers a mode-cycle keyboard handler.
+      await openAgentRoute(page, { workspaceId: seeded.workspaceId, agentId: agent.id });
+      await expect(page.getByTestId("mode-control").first()).toContainText("Default permissions", {
+        timeout: 30_000,
+      });
+
+      // Move to the New Workspace composer. The agent tab stays mounted in the background,
+      // so its handler is still registered when we cycle here.
+      await openGlobalNewWorkspaceComposer(page);
+      await selectNewWorkspaceProject(page, {
+        projectKey: seeded.projectId,
+        projectDisplayName: seeded.projectDisplayName,
+      });
+
+      await cycleNewWorkspaceMode(page, 6);
+
+      // fetchAgents is a real daemon round-trip; once it resolves, any mode change the
+      // presses would have triggered has already landed. Assert the running agent is
+      // untouched — both its committed mode and on the wire — with no fixed sleep.
+      const agents = await seeded.client.fetchAgents();
+      const backgroundAgent = agents.entries.find((entry) => entry.agent.id === agent.id)?.agent;
+      expect(backgroundAgent?.currentModeId).toBe("auto");
+      expect(modeRequests.requestsForAgent(agent.id)).toEqual([]);
+    } finally {
+      await seeded.cleanup();
+    }
+  });
+});

packages/app/src/hooks/use-global-workspace-pin-action.ts

@@ -5,7 +5,7 @@ import type { KeyboardActionId } from "@/keyboard/keyboard-action-dispatcher";
 import { useHostFeature } from "@/runtime/host-features";
 import { useActiveWorkspaceSelection } from "@/stores/navigation-active-workspace-store";
 import { useWorkspaceFields } from "@/stores/session-store-hooks";
-import { buildWorkspaceTabPersistenceKey } from "@/stores/workspace-tabs-store";
+import { buildWorkspaceTabPersistenceKey } from "@/workspace-tabs/model";
 
 const WORKSPACE_PIN_ACTIONS: readonly KeyboardActionId[] = ["workspace.pin"];
 

packages/app/src/hooks/use-keyboard-action-handler.ts

@@ -1,4 +1,4 @@
-import { useEffect } from "react";
+import { useEffect, useRef } from "react";
 
 import {
   keyboardActionDispatcher,
@@ -15,15 +15,48 @@ interface UseKeyboardActionHandlerInput {
   handle: (action: KeyboardActionDefinition) => boolean;
 }
 
+/**
+ * Registers a keyboard action handler with the global dispatcher.
+ *
+ * The dispatcher is driven by a native window keydown listener and calls the
+ * most-recently-registered matching handler first (see keyboard-action-dispatcher.ts).
+ * Two properties must hold or handlers bound to the same action step on each other:
+ *
+ *  1. Stable registration order. registerHandler bumps a registration counter, so
+ *     re-registering on every render reshuffles which handler the dispatcher reaches
+ *     first. A frequently re-rendering control (e.g. a running agent's mode control)
+ *     would then jump ahead of another (e.g. the New Workspace draft) and consume the
+ *     key, silently acting on the wrong surface. So we register once per
+ *     (handlerId, priority, actions) and never re-register just because handle,
+ *     isActive, or enabled changed.
+ *
+ *  2. Fresh callbacks. Because we do not re-register on every render, the registered
+ *     entry must read the latest props at dispatch time. handle and isActive are read
+ *     live from a ref, and enabled is folded into isActive so the dispatcher
+ *     re-evaluates it fresh (the plain enabled field it filters on is captured at
+ *     registration and would otherwise go stale).
+ */
 export function useKeyboardActionHandler(input: UseKeyboardActionHandlerInput) {
+  const inputRef = useRef(input);
+  inputRef.current = input;
+
+  // Only these identity-affecting fields trigger a re-register. actions is compared by
+  // content, not array identity, so inline literals do not churn the registration.
+  const actionsKey = input.actions.join(" ");
   useEffect(() => {
     return keyboardActionDispatcher.registerHandler({
-      handlerId: input.handlerId,
-      actions: input.actions,
-      enabled: input.enabled,
-      priority: input.priority,
-      isActive: input.isActive,
-      handle: input.handle,
+      handlerId: inputRef.current.handlerId,
+      actions: inputRef.current.actions,
+      // Always-on at the coarse filter; the real enable/active gate is re-checked fresh
+      // inside isActive below so it can never be stale in the registry entry.
+      enabled: true,
+      priority: inputRef.current.priority,
+      isActive: () => {
+        const current = inputRef.current;
+        if (!current.enabled) return false;
+        return current.isActive ? current.isActive() : true;
+      },
+      handle: (action) => inputRef.current.handle(action),
     });
-  }, [input.actions, input.enabled, input.handle, input.handlerId, input.isActive, input.priority]);
+  }, [input.handlerId, input.priority, actionsKey]);
 }