fix: pr-bug-scan validated finding from #2504 (#2548)

* fix: address pr-bug-scan validated finding from #2504

Narrowed App.tsx early return so it only suppresses chords the floating panel actually claims (Cmd+T/W, Cmd+Shift+B/M); B/L/Shift+E/F/G now reach app branches.

* Let floating terminals receive shell control chords

- Track floating xterm focus in the main process so Ctrl+B/Cmd+B and
  sidebar-adjacent chords can pass through to SSH and tmux
- Share renderer shortcut targeting logic for floating panel shortcuts
- Cover focus routing and shortcut ownership with tests

---------

Co-authored-by: orca-bug-scan-bot <orca-bug-scan-bot@stably.ai>
Co-authored-by: Jinjing <6427696+AmethystLiang@users.noreply.github.com>
This commit is contained in:
buf0-bot[bot] 2026-05-21 17:16:52 -07:00 committed by GitHub
parent ea63c32b52
commit bc1ba6dc72
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
10 changed files with 383 additions and 20 deletions

View File

@ -1061,6 +1061,83 @@ describe('createMainWindow', () => {
expect(webContents.send).not.toHaveBeenCalledWith('ui:toggleLeftSidebar')
})
it('skips Cmd+B interception when floating terminal input is focused', () => {
const windowHandlers: Record<string, (...args: any[]) => void> = {}
const webContents = {
on: vi.fn((event, handler) => {
windowHandlers[event] = handler
}),
setZoomLevel: vi.fn(),
setBackgroundThrottling: vi.fn(),
invalidate: vi.fn(),
setWindowOpenHandler: vi.fn(),
send: vi.fn(),
isDevToolsOpened: vi.fn(),
openDevTools: vi.fn(),
closeDevTools: vi.fn()
}
const browserWindowInstance = {
webContents,
on: vi.fn(),
isDestroyed: vi.fn(() => false),
isMaximized: vi.fn(() => true),
isFullScreen: vi.fn(() => false),
getSize: vi.fn(() => [1200, 800]),
setSize: vi.fn(),
maximize: vi.fn(),
show: vi.fn(),
loadFile: vi.fn(),
loadURL: vi.fn()
}
browserWindowMock.mockImplementation(function () {
return browserWindowInstance
})
createMainWindow(null)
const setFocusedListener = vi
.mocked(ipcMain.on)
.mock.calls.find(([channel]) => channel === 'ui:setFloatingTerminalInputFocused')?.[1]
expect(setFocusedListener).toBeTypeOf('function')
setFocusedListener?.({ sender: webContents } as never, true)
const preventDefault = vi.fn()
const isDarwin = process.platform === 'darwin'
windowHandlers['before-input-event'](
{ preventDefault } as never,
{
type: 'keyDown',
code: 'KeyB',
key: 'b',
meta: isDarwin,
control: !isDarwin,
alt: false,
shift: false
} as never
)
expect(preventDefault).not.toHaveBeenCalled()
expect(webContents.send).not.toHaveBeenCalledWith('ui:toggleLeftSidebar')
webContents.send.mockClear()
const newWorkspacePreventDefault = vi.fn()
windowHandlers['before-input-event'](
{ preventDefault: newWorkspacePreventDefault } as never,
{
type: 'keyDown',
code: 'KeyN',
key: 'n',
meta: isDarwin,
control: !isDarwin,
alt: false,
shift: false
} as never
)
expect(newWorkspacePreventDefault).toHaveBeenCalledTimes(1)
expect(webContents.send).toHaveBeenCalledWith('ui:openNewWorkspace')
})
it('still intercepts Cmd+Shift+B and Cmd+Alt+B when the markdown editor is focused', () => {
const windowHandlers: Record<string, (...args: any[]) => void> = {}
const webContents = {

View File

@ -466,6 +466,7 @@ export function createMainWindow(
// preserves the ^B-to-PTY leak protection rationale in
// shared/window-shortcut-policy.ts:74-77.
let markdownEditorFocused = false
let floatingTerminalInputFocused = false
const markdownFocusChannel = 'ui:setMarkdownEditorFocused'
// Why: coerce to strict boolean and verify the sender. A renderer bug or
@ -481,6 +482,19 @@ export function createMainWindow(
markdownEditorFocused = focused === true
}
ipcMain.on(markdownFocusChannel, onMarkdownEditorFocused)
const floatingTerminalInputFocusChannel = 'ui:setFloatingTerminalInputFocused'
// Why: main before-input-event runs before renderer keydown handlers. Mirror
// floating xterm focus so Ctrl+B/L and related shell chords can reach SSH/tmux.
const onFloatingTerminalInputFocused = (
event: Electron.IpcMainEvent,
focused: unknown
): void => {
if (event.sender !== mainWindow.webContents) {
return
}
floatingTerminalInputFocused = focused === true
}
ipcMain.on(floatingTerminalInputFocusChannel, onFloatingTerminalInputFocused)
const onMainContextMenu = (_event: Electron.Event, params: Electron.ContextMenuParams): void => {
const template = buildEditableContextMenuTemplate(params, mainWindow.webContents)
@ -501,6 +515,9 @@ export function createMainWindow(
const resetMarkdownEditorFocus = (): void => {
markdownEditorFocused = false
}
const resetFloatingTerminalInputFocus = (): void => {
floatingTerminalInputFocused = false
}
let rendererProcessGone = false
let rendererRecoveryTimer: ReturnType<typeof setTimeout> | null = null
const clearRendererRecoveryTimer = (): void => {
@ -540,16 +557,21 @@ export function createMainWindow(
mainWindow.webContents.on('render-process-gone', (_event, details) => {
rendererProcessGone = true
resetMarkdownEditorFocus()
resetFloatingTerminalInputFocus()
if (opts?.shouldRecordRendererCrash?.(details, rendererWebContentsId) !== false) {
opts?.onRendererProcessGone?.(details, rendererWebContentsId)
}
console.error('[window] Renderer process gone; close confirmation will be bypassed', details)
scheduleRendererRecovery(details)
})
mainWindow.webContents.on('destroyed', resetMarkdownEditorFocus)
mainWindow.webContents.on('destroyed', () => {
resetMarkdownEditorFocus()
resetFloatingTerminalInputFocus()
})
mainWindow.webContents.on('did-start-navigation', (_e, _url, _isInPlace, isMainFrame) => {
if (isMainFrame) {
resetMarkdownEditorFocus()
resetFloatingTerminalInputFocus()
}
})
mainWindow.webContents.on('did-finish-load', () => {
@ -606,14 +628,20 @@ export function createMainWindow(
return
}
// Why: keep the main-process interception surface as an explicit allowlist.
// Anything outside this helper must continue to the renderer/PTTY so
// readline control chords are not silently stolen above the terminal.
const action = resolveWindowShortcutAction(input, process.platform)
if (!action) {
return
}
// Why: keep global app routing for non-terminal actions, but let floating
// xterm own shell control chars that overlap sidebar chrome shortcuts.
if (
floatingTerminalInputFocused &&
(action.type === 'toggleLeftSidebar' || action.type === 'toggleRightSidebar')
) {
return
}
if (input.type !== 'keyDown') {
return
}
@ -830,6 +858,7 @@ export function createMainWindow(
// stale-true flag can't leak past subsequent state transitions. Paired
// with the webContents lifecycle resets above.
markdownEditorFocused = false
floatingTerminalInputFocused = false
clearRendererRecoveryTimer()
ipcMain.removeListener(trafficLightChannel, onSyncTrafficLights)
ipcMain.removeListener(minimizeChannel, onMinimize)
@ -840,6 +869,7 @@ export function createMainWindow(
ipcMain.removeHandler(isMaximizedChannel)
ipcMain.removeListener(confirmCloseChannel, onConfirmClose)
ipcMain.removeListener(markdownFocusChannel, onMarkdownEditorFocused)
ipcMain.removeListener(floatingTerminalInputFocusChannel, onFloatingTerminalInputFocused)
// Why: on updater-triggered shutdown, BrowserWindow can emit `closed`
// after its webContents has already been destroyed. The destroyed
// webContents owns its listeners, so do not touch `mainWindow.webContents`

View File

@ -1827,6 +1827,7 @@ export type PreloadApi = {
setZoomLevel: (level: number) => void
syncTrafficLights: (zoomFactor: number) => void
setMarkdownEditorFocused: (focused: boolean) => void
setFloatingTerminalInputFocused: (focused: boolean) => void
onRichMarkdownContextCommand: (
callback: (payload: RichMarkdownContextMenuCommandPayload) => void
) => () => void

View File

@ -2632,6 +2632,9 @@ const api = {
setMarkdownEditorFocused: (focused: boolean): void => {
ipcRenderer.send('ui:setMarkdownEditorFocused', focused)
},
setFloatingTerminalInputFocused: (focused: boolean): void => {
ipcRenderer.send('ui:setFloatingTerminalInputFocused', focused)
},
onRichMarkdownContextCommand: (
callback: (payload: RichMarkdownContextMenuCommandPayload) => void
): (() => void) => {

View File

@ -58,6 +58,8 @@ import {
import { TOGGLE_FLOATING_TERMINAL_EVENT } from '@/lib/floating-terminal'
import {
isFloatingWorkspacePanelFocused,
isFloatingWorkspacePanelShortcut,
isFloatingWorkspaceTerminalInputTarget,
shouldMinimizeFloatingWorkspacePanelOnCloseShortcut
} from '@/lib/floating-workspace-terminal-actions'
import { DictationController } from './components/dictation/DictationController'
@ -1074,6 +1076,13 @@ function App(): React.JSX.Element {
return
}
// Why: xterm's helper textarea is intentionally not a generic editable
// target, but floating-terminal SSH/tmux control chords must still reach
// the terminal instead of app-level chrome shortcuts.
if (isFloatingWorkspaceTerminalInputTarget(e.target)) {
return
}
// Cmd/Ctrl+Alt+Arrow — worktree history back/forward. Handled before the
// `mod && !alt` branch below since this is the one renderer-side shortcut
// that intentionally requires Alt.
@ -1103,8 +1112,15 @@ function App(): React.JSX.Element {
return
}
// Why: only short-circuit chords the floating panel's own keydown
// handler claims (Cmd/Ctrl+T, Cmd/Ctrl+W, Cmd/Ctrl+Shift+B/M). Other
// app-level mod shortcuts (B, L, Shift+E/F/G) have no panel-level
// counterpart, so suppressing them here would silently no-op when
// focus lives inside the floating panel.
if (isFloatingWorkspacePanelFocused()) {
return
if (isFloatingWorkspacePanelShortcut(e, isMac)) {
return
}
}
// Why: after the last floating tab is closed, the empty overlay has no

View File

@ -503,7 +503,8 @@ describe('FloatingTerminalPanel close behavior', () => {
pickFloatingMarkdownDocument: mocks.pickFloatingMarkdownDocument
},
browser: { notifyActiveTabChanged: vi.fn() },
cli: { getInstallStatus: mocks.getInstallStatus }
cli: { getInstallStatus: mocks.getInstallStatus },
ui: { setFloatingTerminalInputFocused: vi.fn() }
},
innerWidth: 1200,
removeEventListener: vi.fn()
@ -576,7 +577,10 @@ describe('FloatingTerminalPanel close behavior', () => {
setFloatingTabs([makeTab({ id: 'tab-1' })])
const element = await renderPanel(true)
const panel = findByProp(element, 'data-floating-terminal-panel')
const titlebarTarget = { closest: vi.fn().mockReturnValue({}) }
const titlebarTarget = {
closest: vi.fn().mockReturnValue({}),
getAttribute: vi.fn().mockReturnValue(null)
}
Object.setPrototypeOf(titlebarTarget, HTMLElement.prototype)
const preventDefault = vi.fn()
@ -657,7 +661,10 @@ describe('FloatingTerminalPanel close behavior', () => {
const onOpenChange = vi.fn()
const element = await renderPanel(true, onOpenChange)
const panel = findByProp(element, 'data-floating-terminal-panel')
const emptyStateTarget = { closest: vi.fn().mockReturnValue({}) }
const emptyStateTarget = {
closest: vi.fn().mockReturnValue({}),
getAttribute: vi.fn().mockReturnValue(null)
}
Object.setPrototypeOf(emptyStateTarget, HTMLElement.prototype)
;(panel.props.onKeyDownCapture as (event: unknown) => void)({

View File

@ -24,6 +24,10 @@ import { getConnectionId } from '@/lib/connection-context'
import { createUntitledMarkdownFile } from '@/lib/create-untitled-markdown'
import { detectLanguage } from '@/lib/language-detect'
import { focusTerminalTabSurface } from '@/lib/focus-terminal-tab-surface'
import {
isFloatingWorkspacePanelShortcut,
isFloatingWorkspaceTerminalInputTarget
} from '@/lib/floating-workspace-terminal-actions'
import { extractIpcErrorMessage } from '@/lib/ipc-error'
import {
ORCHESTRATION_SETUP_DISMISSED_STORAGE_KEY,
@ -654,6 +658,10 @@ export function FloatingTerminalPanel({
panelRef.current?.focus({ preventScroll: true })
}, [])
const setFloatingTerminalInputFocused = useCallback((target: EventTarget | null): void => {
window.api.ui.setFloatingTerminalInputFocused(isFloatingWorkspaceTerminalInputTarget(target))
}, [])
const handleShortcutSurfaceKeyDown = useCallback(
(event: React.KeyboardEvent<HTMLDivElement>) => {
if (!open || event.defaultPrevented || event.repeat) {
@ -668,9 +676,13 @@ export function FloatingTerminalPanel({
return
}
const isMac = navigator.userAgent.includes('Mac')
const mod = isMac ? event.metaKey && !event.ctrlKey : event.ctrlKey && !event.metaKey
if (!mod || event.altKey) {
if (
!isFloatingWorkspacePanelShortcut(
event,
navigator.userAgent.includes('Mac'),
panelRef.current
)
) {
return
}
@ -710,6 +722,13 @@ export function FloatingTerminalPanel({
]
)
useEffect(() => {
if (!open) {
window.api.ui.setFloatingTerminalInputFocused(false)
}
return () => window.api.ui.setFloatingTerminalInputFocused(false)
}, [open])
const toggleMaximized = useCallback(() => {
setMaximized((current) => {
if (current) {
@ -804,6 +823,8 @@ export function FloatingTerminalPanel({
clampFloatingTerminalBounds({ ...prev, width: rect.width, height: rect.height })
)
}}
onFocusCapture={(event) => setFloatingTerminalInputFocused(event.target)}
onBlurCapture={(event) => setFloatingTerminalInputFocused(event.relatedTarget)}
onKeyDownCapture={handleShortcutSurfaceKeyDown}
>
<div className="flex min-h-0 flex-1 flex-col">

View File

@ -1,9 +1,12 @@
import { beforeEach, describe, expect, it, vi } from 'vitest'
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
import { FLOATING_TERMINAL_WORKTREE_ID } from '../../../shared/constants'
import type { TerminalTab } from '../../../shared/types'
import {
createFloatingWorkspaceTerminalTab,
isFloatingWorkspacePanelFocused,
isFloatingWorkspacePanelShortcut,
isFloatingWorkspacePanelShortcutTarget,
isFloatingWorkspaceTerminalInputTarget,
isFloatingWorkspacePanelVisible,
shouldMinimizeFloatingWorkspacePanelOnCloseShortcut
} from './floating-workspace-terminal-actions'
@ -19,6 +22,50 @@ vi.mock('./focus-terminal-tab-surface', () => ({
focusTerminalTabSurface: focusTerminalTabSurfaceMock
}))
function shortcutEvent(overrides: Partial<KeyboardEvent>): KeyboardEvent {
return {
altKey: false,
ctrlKey: false,
key: 't',
metaKey: false,
shiftKey: false,
...overrides
} as KeyboardEvent
}
function shortcutSurfaceEvent(overrides: Partial<KeyboardEvent>): KeyboardEvent {
return shortcutEvent({
target: makeElement({
closestSelectors: ['[data-floating-terminal-shortcut-surface]']
}),
...overrides
})
}
function installFakeHTMLElement(): void {
vi.stubGlobal('HTMLElement', class {})
}
function makeElement({
attributes = [],
classNames = [],
closestSelectors = []
}: {
attributes?: string[]
classNames?: string[]
closestSelectors?: string[]
}): HTMLElement {
const element = {
classList: {
contains: vi.fn((token: string) => classNames.includes(token))
},
getAttribute: vi.fn((attribute: string) => (attributes.includes(attribute) ? '' : null)),
closest: vi.fn((selector: string) => (closestSelectors.includes(selector) ? {} : null))
}
Object.setPrototypeOf(element, HTMLElement.prototype)
return element as unknown as HTMLElement
}
function makeTab(id: string): TerminalTab {
return {
id,
@ -32,6 +79,10 @@ function makeTab(id: string): TerminalTab {
}
}
afterEach(() => {
vi.unstubAllGlobals()
})
describe('isFloatingWorkspacePanelVisible', () => {
it('detects the visible floating workspace panel', () => {
const doc = {
@ -53,17 +104,122 @@ describe('isFloatingWorkspacePanelVisible', () => {
describe('isFloatingWorkspacePanelFocused', () => {
it('detects focus inside the floating workspace panel', () => {
const activeElement = {
closest: vi.fn().mockReturnValue({})
}
vi.stubGlobal('HTMLElement', class {})
Object.setPrototypeOf(activeElement, HTMLElement.prototype)
installFakeHTMLElement()
const activeElement = makeElement({
closestSelectors: ['[data-floating-terminal-panel]']
})
expect(isFloatingWorkspacePanelFocused({ activeElement } as never)).toBe(true)
expect(activeElement.closest).toHaveBeenCalledWith('[data-floating-terminal-panel]')
})
})
vi.unstubAllGlobals()
describe('isFloatingWorkspaceTerminalInputTarget', () => {
it('detects the xterm helper textarea inside the floating panel', () => {
installFakeHTMLElement()
const target = makeElement({
classNames: ['xterm-helper-textarea'],
closestSelectors: ['[data-floating-terminal-panel]']
})
expect(isFloatingWorkspaceTerminalInputTarget(target)).toBe(true)
})
it('detects targets inside xterm DOM inside the floating panel', () => {
installFakeHTMLElement()
const target = makeElement({
closestSelectors: ['[data-floating-terminal-panel]', '.xterm']
})
expect(isFloatingWorkspaceTerminalInputTarget(target)).toBe(true)
})
it('ignores terminal input outside the floating panel', () => {
installFakeHTMLElement()
const target = makeElement({
classNames: ['xterm-helper-textarea']
})
expect(isFloatingWorkspaceTerminalInputTarget(target)).toBe(false)
})
it('ignores non-terminal targets inside the floating panel', () => {
installFakeHTMLElement()
const target = makeElement({
closestSelectors: ['[data-floating-terminal-panel]']
})
expect(isFloatingWorkspaceTerminalInputTarget(target)).toBe(false)
})
})
describe('isFloatingWorkspacePanelShortcut', () => {
beforeEach(() => {
installFakeHTMLElement()
})
it.each([
['Cmd+T', true, { key: 't', metaKey: true }],
['Ctrl+T', false, { key: 't', ctrlKey: true }],
['Cmd+W', true, { key: 'w', metaKey: true }],
['Ctrl+W', false, { key: 'w', ctrlKey: true }],
['Cmd+Shift+B', true, { key: 'b', metaKey: true, shiftKey: true }],
['Ctrl+Shift+B', false, { key: 'b', ctrlKey: true, shiftKey: true }],
['Cmd+Shift+M', true, { key: 'm', metaKey: true, shiftKey: true }],
['Ctrl+Shift+M', false, { key: 'm', ctrlKey: true, shiftKey: true }]
])('claims %s', (_label, isMacPlatform, overrides) => {
expect(isFloatingWorkspacePanelShortcut(shortcutSurfaceEvent(overrides), isMacPlatform)).toBe(
true
)
})
it.each([
['Cmd+B', true, { key: 'b', metaKey: true }],
['Ctrl+B', false, { key: 'b', ctrlKey: true }]
])('does not claim bare %s', (_label, isMacPlatform, overrides) => {
expect(isFloatingWorkspacePanelShortcut(shortcutSurfaceEvent(overrides), isMacPlatform)).toBe(
false
)
})
it('does not claim shortcuts with Alt or the wrong platform modifier', () => {
expect(
isFloatingWorkspacePanelShortcut(shortcutSurfaceEvent({ key: 't', metaKey: true }), false)
).toBe(false)
expect(
isFloatingWorkspacePanelShortcut(shortcutSurfaceEvent({ key: 't', ctrlKey: true }), true)
).toBe(false)
expect(
isFloatingWorkspacePanelShortcut(
shortcutSurfaceEvent({ key: 't', ctrlKey: true, altKey: true }),
false
)
).toBe(false)
})
it('only claims shortcuts from the panel root or shortcut surface', () => {
const panelRoot = makeElement({
attributes: ['data-floating-terminal-panel'],
closestSelectors: ['[data-floating-terminal-panel]']
})
const panelContent = makeElement({
closestSelectors: ['[data-floating-terminal-panel]']
})
expect(isFloatingWorkspacePanelShortcutTarget(panelRoot, panelRoot)).toBe(true)
expect(
isFloatingWorkspacePanelShortcut(
shortcutEvent({ key: 't', ctrlKey: true, target: panelRoot }),
false
)
).toBe(true)
expect(
isFloatingWorkspacePanelShortcut(
shortcutEvent({ key: 't', ctrlKey: true, target: panelContent }),
false,
panelRoot
)
).toBe(false)
})
})

View File

@ -9,6 +9,18 @@ type FloatingWorkspaceTerminalStore = Pick<
'activeGroupIdByWorktree' | 'createTab' | 'activateTab' | 'settings'
>
type FloatingWorkspaceShortcutEvent = Pick<
KeyboardEvent,
'altKey' | 'ctrlKey' | 'key' | 'metaKey' | 'shiftKey' | 'target'
>
const FLOATING_WORKSPACE_PANEL_SELECTOR = '[data-floating-terminal-panel]'
const FLOATING_WORKSPACE_SHORTCUT_SURFACE_SELECTOR = '[data-floating-terminal-shortcut-surface]'
function defaultIsMacPlatform(): boolean {
return typeof navigator !== 'undefined' && navigator.userAgent.includes('Mac')
}
export function isFloatingWorkspacePanelVisible(
doc: Pick<Document, 'querySelector'> = document
): boolean {
@ -19,7 +31,46 @@ export function isFloatingWorkspacePanelFocused(
doc: Pick<Document, 'activeElement'> = document
): boolean {
const active = doc.activeElement
return active instanceof HTMLElement && active.closest('[data-floating-terminal-panel]') !== null
return active instanceof HTMLElement && active.closest(FLOATING_WORKSPACE_PANEL_SELECTOR) !== null
}
export function isFloatingWorkspaceTerminalInputTarget(target: EventTarget | null): boolean {
if (!(target instanceof HTMLElement)) {
return false
}
if (target.closest(FLOATING_WORKSPACE_PANEL_SELECTOR) === null) {
return false
}
return target.classList.contains('xterm-helper-textarea') || target.closest('.xterm') !== null
}
export function isFloatingWorkspacePanelShortcutTarget(
target: EventTarget | null,
panelRoot: HTMLElement | null = null
): boolean {
if (!(target instanceof HTMLElement)) {
return false
}
return (
target === panelRoot ||
target.getAttribute('data-floating-terminal-panel') !== null ||
target.closest(FLOATING_WORKSPACE_SHORTCUT_SURFACE_SELECTOR) !== null
)
}
export function isFloatingWorkspacePanelShortcut(
event: FloatingWorkspaceShortcutEvent,
isMacPlatform = defaultIsMacPlatform(),
panelRoot: HTMLElement | null = null
): boolean {
const mod = isMacPlatform ? event.metaKey && !event.ctrlKey : event.ctrlKey && !event.metaKey
if (!mod || event.altKey) {
return false
}
const key = event.key.toLowerCase()
const claimedChord = event.shiftKey ? key === 'b' || key === 'm' : key === 't' || key === 'w'
return claimedChord && isFloatingWorkspacePanelShortcutTarget(event.target, panelRoot)
}
export function shouldMinimizeFloatingWorkspacePanelOnCloseShortcut({

View File

@ -901,6 +901,7 @@ function createWebUiApi(): NonNullable<Partial<PreloadApi>['ui']> {
onFileDrop: () => noopUnsubscribe,
syncTrafficLights: () => {},
setMarkdownEditorFocused: () => {},
setFloatingTerminalInputFocused: () => {},
onRichMarkdownContextCommand: () => noopUnsubscribe,
onFullscreenChanged: () => noopUnsubscribe,
minimize: () => {},