fix: address Codex review on PR #2722

components/terminal/runtime/middleClickBehavior.test.ts

@@ -2,6 +2,7 @@ import assert from "node:assert/strict";
 import test from "node:test";
 
 import {
+  createRightClickMouseTrackingPressClaim,
   isMiddleClickContextMenuEvent,
   isShiftSelectionReplayMouseEvent,
   markMiddleClickContextMenuEvent,
@@ -10,6 +11,7 @@ import {
   resolveMiddleClickBehavior,
   shouldInterceptMouseTrackingContextMenu,
   shouldReplayShiftMouseSelectionAsMacOption,
+  shouldStopRightClickMouseTrackingMouseUp,
   shouldStopShiftRightClickMouseTrackingMouseDown,
 } from "./middleClickBehavior";
 
@@ -299,6 +301,118 @@ test("Shift right-click mousedown capture is limited to connected mouse tracking
   );
 });
 
+test("right-click mouseup reaches mouse-tracking apps when Netcatty did not claim the press", () => {
+  // Herdr / Terminal.app: button-down was delivered, so button-up must be too.
+  // Swallowing mouseup leaves the TUI stuck thinking the right button is held (#2721).
+  const claim = createRightClickMouseTrackingPressClaim();
+  assert.equal(
+    claim.noteMouseDown({
+      event: { button: 2, shiftKey: false } as MouseEvent,
+      mouseTracking: true,
+      status: "connected",
+      rightClickBehavior: "context-menu",
+      forceMenuInAlternateScreen: false,
+    }),
+    false,
+  );
+  assert.equal(
+    shouldStopRightClickMouseTrackingMouseUp({
+      event: { button: 2, shiftKey: false } as MouseEvent,
+      claimedMatchingMouseDown: claim.consumeMouseUpClaim({ button: 2 } as MouseEvent),
+    }),
+    false,
+  );
+});
+
+test("right-click mouseup is stopped only when Netcatty claimed the matching mousedown", () => {
+  const shiftClaim = createRightClickMouseTrackingPressClaim();
+  assert.equal(
+    shiftClaim.noteMouseDown({
+      event: { button: 2, shiftKey: true } as MouseEvent,
+      mouseTracking: true,
+      status: "connected",
+    }),
+    true,
+  );
+  assert.equal(
+    shouldStopRightClickMouseTrackingMouseUp({
+      event: { button: 2, shiftKey: true } as MouseEvent,
+      claimedMatchingMouseDown: shiftClaim.consumeMouseUpClaim({ button: 2 } as MouseEvent),
+    }),
+    true,
+  );
+
+  const forceMenuClaim = createRightClickMouseTrackingPressClaim();
+  assert.equal(
+    forceMenuClaim.noteMouseDown({
+      event: { button: 2, shiftKey: false } as MouseEvent,
+      mouseTracking: true,
+      status: "connected",
+      rightClickBehavior: "context-menu",
+      forceMenuInAlternateScreen: true,
+    }),
+    true,
+  );
+  assert.equal(
+    shouldStopRightClickMouseTrackingMouseUp({
+      event: { button: 2, shiftKey: false } as MouseEvent,
+      claimedMatchingMouseDown: forceMenuClaim.consumeMouseUpClaim({ button: 2 } as MouseEvent),
+    }),
+    true,
+  );
+
+  assert.equal(
+    shouldStopRightClickMouseTrackingMouseUp({
+      event: { button: 0, shiftKey: false } as MouseEvent,
+      claimedMatchingMouseDown: false,
+    }),
+    false,
+  );
+});
+
+test("right-click mouseup pairs with the claimed mousedown, not the release modifiers", () => {
+  // Shift+right press claimed by Netcatty, then Shift released before mouseup:
+  // still swallow the release so xterm never sees a lone button-up.
+  const claimedThenShiftReleased = createRightClickMouseTrackingPressClaim();
+  assert.equal(
+    claimedThenShiftReleased.noteMouseDown({
+      event: { button: 2, shiftKey: true } as MouseEvent,
+      mouseTracking: true,
+      status: "connected",
+    }),
+    true,
+  );
+  assert.equal(
+    shouldStopRightClickMouseTrackingMouseUp({
+      event: { button: 2, shiftKey: false } as MouseEvent,
+      claimedMatchingMouseDown: claimedThenShiftReleased.consumeMouseUpClaim(
+        { button: 2, shiftKey: false } as MouseEvent,
+      ),
+    }),
+    true,
+  );
+
+  // App-owned press, then Shift held on release: release must still reach xterm.
+  const appOwnedThenShiftAdded = createRightClickMouseTrackingPressClaim();
+  assert.equal(
+    appOwnedThenShiftAdded.noteMouseDown({
+      event: { button: 2, shiftKey: false } as MouseEvent,
+      mouseTracking: true,
+      status: "connected",
+    }),
+    false,
+  );
+  assert.equal(
+    shouldStopRightClickMouseTrackingMouseUp({
+      event: { button: 2, shiftKey: true } as MouseEvent,
+      claimedMatchingMouseDown: appOwnedThenShiftAdded.consumeMouseUpClaim(
+        { button: 2, shiftKey: true } as MouseEvent,
+      ),
+    }),
+    false,
+  );
+});
+
 test("middle-click terminal mouse down/up events are captured before xterm sees them", () => {
   const calls: string[] = [];
   const middleClickEvent = {

components/terminal/runtime/middleClickBehavior.ts

@@ -131,6 +131,48 @@ export const shouldStopShiftRightClickMouseTrackingMouseDown = ({
   && event.button === 2
   && (event.shiftKey || forcesMenuOverMouseTracking({ rightClickBehavior, forceMenuInAlternateScreen }));
 
+// Pair mouseup with mousedown ownership. When Netcatty claims the press
+// (Shift / fullscreen-apps menu), also swallow the release so xterm never
+// reports a lone button-up. When the TUI owns the press (Herdr, tmux menus,
+// vim, ...), the release must reach xterm too - otherwise the app stays stuck
+// with the right button held and mouse UI dies until restart (#2721).
+// Ownership is remembered from the actual mousedown claim — never re-derived
+// from mouseup modifiers (Shift may change between press and release).
+export interface RightClickMouseTrackingPressClaim {
+  /** Evaluate + record whether this right-button mousedown was claimed. */
+  noteMouseDown: (state: ShiftRightClickMouseDownCaptureState) => boolean;
+  /** Consume the pending claim for a right-button mouseup (clears state). */
+  consumeMouseUpClaim: (event: MouseEvent) => boolean;
+}
+
+export const createRightClickMouseTrackingPressClaim = (): RightClickMouseTrackingPressClaim => {
+  let claimedMatchingMouseDown = false;
+
+  return {
+    noteMouseDown(state) {
+      const shouldStop = shouldStopShiftRightClickMouseTrackingMouseDown(state);
+      if (state.event.button === 2) {
+        claimedMatchingMouseDown = shouldStop;
+      }
+      return shouldStop;
+    },
+    consumeMouseUpClaim(event) {
+      if (event.button !== 2) return false;
+      const claimed = claimedMatchingMouseDown;
+      claimedMatchingMouseDown = false;
+      return claimed;
+    },
+  };
+};
+
+export const shouldStopRightClickMouseTrackingMouseUp = ({
+  event,
+  claimedMatchingMouseDown,
+}: {
+  event: MouseEvent;
+  claimedMatchingMouseDown: boolean;
+}): boolean => event.button === 2 && claimedMatchingMouseDown;
+
 export const createMacOptionForcedSelectionMouseEvent = (event: MouseEvent): MouseEvent =>
   markShiftSelectionReplayMouseEvent(new MouseEvent(event.type, {
     bubbles: event.bubbles,

components/terminal/useTerminalEffects.ts

@@ -8,9 +8,10 @@ import { isMacPlatform } from '../../lib/utils';
 import { resolveXTermScrollback } from '../../infrastructure/config/xtermPerformance';
 import {
   createMacOptionForcedSelectionMouseEvent,
+  createRightClickMouseTrackingPressClaim,
   shouldInterceptMouseTrackingContextMenu,
   shouldReplayShiftMouseSelectionAsMacOption,
-  shouldStopShiftRightClickMouseTrackingMouseDown,
+  shouldStopRightClickMouseTrackingMouseUp,
 } from './runtime/middleClickBehavior';
 import {
   hasOpenAppDialog,
@@ -1553,21 +1554,24 @@ export function useTerminalEffects(ctx: TerminalEffectsContext) {
   }, [sessionId]);
 
 
-  // Prevent xterm.js's built-in rightClickHandler and right-button mouseup
-  // from interfering with tmux/vim popup menus when mouse tracking is active.
-  // On macOS, xterm forces selection with Option, while most terminal users
-  // expect Shift to bypass mouse reporting. Replay Shift+left-click as that
-  // native xterm force-selection gesture before xterm receives the original.
-  // - mousedown (button 2 + Shift): keep Shift+right-click local so the
-  //   terminal app does not also receive the right-button press
+  // Prevent xterm.js's built-in rightClickHandler from interfering with
+  // mouse-tracking TUIs. On macOS, xterm forces selection with Option, while
+  // most terminal users expect Shift to bypass mouse reporting. Replay
+  // Shift+left-click as that native xterm force-selection gesture before
+  // xterm receives the original.
+  // - mousedown (button 2 + Shift / fullscreen-apps menu): keep the press
+  //   local so the terminal app does not also receive it
   // - contextmenu: xterm.js calls textarea.select() which steals focus
-  // - mouseup (button 2): tmux interprets the right-button release as a
-  //   dismiss action, closing the popup menu immediately after it appears
-  // Both are intercepted at the capture phase before xterm.js's own listeners.
+  // - mouseup (button 2): only stop when we also claimed the matching
+  //   mousedown. App-owned right-clicks (Herdr, tmux, vim, ...) must receive
+  //   the release or the TUI stays stuck with the button held (#2721).
+  // Intercepted at the capture phase before xterm.js's own listeners.
   useEffect(() => {
     const el = containerRef.current;
     if (!el) return;
 
+    const rightClickPressClaim = createRightClickMouseTrackingPressClaim();
+
     const handleContextMenuCapture = (e: MouseEvent) => {
       if (!shouldInterceptMouseTrackingContextMenu({
         event: e,
@@ -1601,7 +1605,7 @@ export function useTerminalEffects(ctx: TerminalEffectsContext) {
     };
 
     const handleMouseDownCapture = (e: MouseEvent) => {
-      if (shouldStopShiftRightClickMouseTrackingMouseDown({
+      if (rightClickPressClaim.noteMouseDown({
         event: e,
         mouseTracking: mouseTrackingRef.current,
         status: statusRef.current,
@@ -1630,7 +1634,10 @@ export function useTerminalEffects(ctx: TerminalEffectsContext) {
     };
 
     const handleMouseUpCapture = (e: MouseEvent) => {
-      if (e.button === 2 && mouseTrackingRef.current && statusRef.current === 'connected') {
+      if (shouldStopRightClickMouseTrackingMouseUp({
+        event: e,
+        claimedMatchingMouseDown: rightClickPressClaim.consumeMouseUpClaim(e),
+      })) {
         e.stopImmediatePropagation();
       }
     };