From 6d40e01ccbc78a71b714810864d462a107ca42ec Mon Sep 17 00:00:00 2001 From: Jinwoo-H Date: Thu, 30 Apr 2026 16:50:17 -0400 Subject: [PATCH] Fix pane ID collision in mobile-fit overrides by using ptyId-based lookups MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Pane IDs are per-tab, so two tabs can both have pane 1. The old ptyIdByPaneId map keyed by bare pane ID caused the left initial terminal to stay stuck at phone dimensions (49 cols) even after the mobile-fit override was cleared — the override for a different tab's pane with the same ID would match. - Change override map key from bare paneId to tabId:paneId composite - Switch safeFit to look up overrides via data-pty-id attribute (globally unique) - Same ptyId-based lookup in onResize, terminal-appearance, and reattach - Fix test mocks to include container.dataset for data-pty-id writes Co-authored-by: Orca --- .../terminal-pane/pty-connection.test.ts | 1 + .../terminal-pane/pty-connection.ts | 17 +++++--- .../terminal-pane/terminal-appearance.ts | 5 ++- .../lib/pane-manager/mobile-fit-overrides.ts | 43 ++++++++++++------- .../lib/pane-manager/pane-tree-ops.test.ts | 2 +- .../src/lib/pane-manager/pane-tree-ops.ts | 8 ++-- 6 files changed, 49 insertions(+), 27 deletions(-) diff --git a/src/renderer/src/components/terminal-pane/pty-connection.test.ts b/src/renderer/src/components/terminal-pane/pty-connection.test.ts index 1a7d768fcc0..c51539cd048 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection.test.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection.test.ts @@ -141,6 +141,7 @@ function createPane(paneId: number) { onData: vi.fn(() => ({ dispose: vi.fn() })), onResize: vi.fn(() => ({ dispose: vi.fn() })) }, + container: { dataset: {} }, fitAddon: { fit: vi.fn() } diff --git a/src/renderer/src/components/terminal-pane/pty-connection.ts b/src/renderer/src/components/terminal-pane/pty-connection.ts index 4f43abc56a3..ce27e9a7b00 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection.ts @@ -10,7 +10,7 @@ import { createIpcPtyTransport } from './pty-transport' import { shouldSeedCacheTimerOnInitialTitle } from './cache-timer-seeding' import type { PtyConnectionDeps } from './pty-connection-types' import { safeFit } from '@/lib/pane-manager/pane-tree-ops' -import { getFitOverrideForPane, bindPanePtyId } from '@/lib/pane-manager/mobile-fit-overrides' +import { getFitOverrideForPty, bindPanePtyId } from '@/lib/pane-manager/mobile-fit-overrides' import { isPaneReplaying, replayIntoTerminal, replayIntoTerminalAsync } from './replay-guard' import { paneLeafId, @@ -212,7 +212,8 @@ export function connectPanePty( } const onPtySpawn = (ptyId: string): void => { - bindPanePtyId(pane.id, ptyId) + bindPanePtyId(pane.id, ptyId, deps.tabId) + pane.container.dataset.ptyId = ptyId deps.syncPanePtyLayoutBinding(pane.id, ptyId) deps.updateTabPtyId(deps.tabId, ptyId) // Spawn completion is when a pane gains a concrete PTY ID. The initial @@ -395,8 +396,10 @@ export function connectPanePty( const onResizeDisposable = pane.terminal.onResize(({ cols, rows }) => { // Why: when a mobile-fit override is active, the PTY is already at the // correct phone dimensions. Suppress resize forwarding to avoid spurious - // SIGWINCH signals that could cause TUI flicker. - if (getFitOverrideForPane(pane.id)) { + // SIGWINCH signals that could cause TUI flicker. Uses the transport's + // ptyId directly to avoid pane ID collisions across tabs. + const currentPtyId = transport.getPtyId() + if (currentPtyId && getFitOverrideForPty(currentPtyId)) { return } transport.resize(cols, rows) @@ -557,7 +560,8 @@ export function connectPanePty( startFreshSpawn() return } - bindPanePtyId(pane.id, ptyId) + bindPanePtyId(pane.id, ptyId, deps.tabId) + pane.container.dataset.ptyId = ptyId deps.syncPanePtyLayoutBinding(pane.id, ptyId) deps.updateTabPtyId(deps.tabId, ptyId) @@ -648,7 +652,8 @@ export function connectPanePty( // Why: when a mobile-fit override is active, skip sending desktop dims // to the PTY — the PTY is already at phone dimensions and must stay there. - if (!getFitOverrideForPane(pane.id)) { + const reattachPtyId = transport.getPtyId() + if (!reattachPtyId || !getFitOverrideForPty(reattachPtyId)) { transport.resize(cols, rows) } // Why: POSIX only delivers SIGWINCH when terminal dimensions actually diff --git a/src/renderer/src/components/terminal-pane/terminal-appearance.ts b/src/renderer/src/components/terminal-pane/terminal-appearance.ts index 861d5987f6c..b9b87976070 100644 --- a/src/renderer/src/components/terminal-pane/terminal-appearance.ts +++ b/src/renderer/src/components/terminal-pane/terminal-appearance.ts @@ -10,7 +10,7 @@ import { } from '@/lib/terminal-theme' import { buildFontFamily } from './layout-serialization' import { captureScrollState, restoreScrollState, safeFit } from '@/lib/pane-manager/pane-tree-ops' -import { getFitOverrideForPane } from '@/lib/pane-manager/mobile-fit-overrides' +import { getFitOverrideForPty } from '@/lib/pane-manager/mobile-fit-overrides' import type { PtyTransport } from './pty-transport' import type { EffectiveMacOptionAsAlt } from '@/lib/keyboard-layout/detect-option-as-alt' import { HEX_COLOR_RE } from '../../../../shared/color-validation' @@ -231,7 +231,8 @@ export function applyTerminalAppearance( // Why: skip PTY resize when a mobile-fit override is active — the PTY // is already at the correct phone dimensions and must not be resized // back to desktop dimensions by an appearance change. - if (transport?.isConnected() && !getFitOverrideForPane(pane.id)) { + const appearancePtyId = transport?.getPtyId() + if (transport?.isConnected() && (!appearancePtyId || !getFitOverrideForPty(appearancePtyId))) { transport.resize(pane.terminal.cols, pane.terminal.rows) maybePushMode2031Flip(pane.id, appearance.mode, transport, paneMode2031, paneLastThemeMode) } diff --git a/src/renderer/src/lib/pane-manager/mobile-fit-overrides.ts b/src/renderer/src/lib/pane-manager/mobile-fit-overrides.ts index 3c10f129e18..7e5a781a081 100644 --- a/src/renderer/src/lib/pane-manager/mobile-fit-overrides.ts +++ b/src/renderer/src/lib/pane-manager/mobile-fit-overrides.ts @@ -10,7 +10,9 @@ type FitOverride = { } const overridesByPtyId = new Map() -const ptyIdByPaneId = new Map() +// Why: keyed by 'tabId:paneId' composite to avoid collisions when different +// tabs have panes with the same numeric ID (pane IDs are per-tab, not global). +const ptyIdByPaneKey = new Map() // Why: the override maps are plain JS — React components that read them // (e.g. the desktop mobile-fit banner) have no way to know when entries @@ -52,9 +54,12 @@ export function setFitOverride( export function getPaneIdsForPty(ptyId: string): number[] { const result: number[] = [] - for (const [paneId, boundPtyId] of ptyIdByPaneId) { + for (const [key, boundPtyId] of ptyIdByPaneKey) { if (boundPtyId === ptyId) { - result.push(paneId) + const paneId = Number(key.split(':').pop()) + if (!Number.isNaN(paneId)) { + result.push(paneId) + } } } return result @@ -64,24 +69,32 @@ export function getFitOverrideForPty(ptyId: string): FitOverride | null { return overridesByPtyId.get(ptyId) ?? null } -export function getFitOverrideForPane(paneId: number): FitOverride | null { - const ptyId = ptyIdByPaneId.get(paneId) - if (!ptyId) { - return null +export function getFitOverrideForPane(paneId: number, tabId?: string): FitOverride | null { + if (tabId) { + const ptyId = ptyIdByPaneKey.get(`${tabId}:${paneId}`) + if (!ptyId) { + return null + } + return overridesByPtyId.get(ptyId) ?? null } - return overridesByPtyId.get(ptyId) ?? null + return null } -export function bindPanePtyId(paneId: number, ptyId: string | null): void { - if (ptyId) { - ptyIdByPaneId.set(paneId, ptyId) - } else { - ptyIdByPaneId.delete(paneId) +export function bindPanePtyId(paneId: number, ptyId: string | null, tabId?: string): void { + if (tabId) { + const key = `${tabId}:${paneId}` + if (ptyId) { + ptyIdByPaneKey.set(key, ptyId) + } else { + ptyIdByPaneKey.delete(key) + } } } -export function unbindPane(paneId: number): void { - ptyIdByPaneId.delete(paneId) +export function unbindPane(paneId: number, tabId?: string): void { + if (tabId) { + ptyIdByPaneKey.delete(`${tabId}:${paneId}`) + } } export function hydrateOverrides( diff --git a/src/renderer/src/lib/pane-manager/pane-tree-ops.test.ts b/src/renderer/src/lib/pane-manager/pane-tree-ops.test.ts index e52585fb05b..0a2450ba669 100644 --- a/src/renderer/src/lib/pane-manager/pane-tree-ops.test.ts +++ b/src/renderer/src/lib/pane-manager/pane-tree-ops.test.ts @@ -33,7 +33,7 @@ function createPane({ return { id: 1, terminal: terminal as never, - container: {} as never, + container: { dataset: {} } as never, xtermContainer: {} as never, linkTooltip: {} as never, gpuRenderingEnabled: true, diff --git a/src/renderer/src/lib/pane-manager/pane-tree-ops.ts b/src/renderer/src/lib/pane-manager/pane-tree-ops.ts index 7e7d05bb6c1..721479d32a8 100644 --- a/src/renderer/src/lib/pane-manager/pane-tree-ops.ts +++ b/src/renderer/src/lib/pane-manager/pane-tree-ops.ts @@ -5,7 +5,7 @@ import type { PaneStyleOptions } from './pane-manager-types' import { createDivider } from './pane-divider' -import { getFitOverrideForPane } from './mobile-fit-overrides' +import { getFitOverrideForPty } from './mobile-fit-overrides' export { findLineByContent, captureScrollState, restoreScrollState } from './pane-scroll' @@ -44,8 +44,10 @@ export function safeFit(pane: ManagedPane): void { // Why: when a mobile client has resized this PTY to phone dimensions, // the desktop must keep xterm at those dimensions instead of fitting to // the desktop pane geometry. This prevents desktop auto-fit from undoing - // the mobile resize. - const override = getFitOverrideForPane(pane.id) + // the mobile resize. Uses data-pty-id (set by bindPanePtyId) to look up + // the override by ptyId directly, avoiding pane ID collisions across tabs. + const ptyId = pane.container.dataset.ptyId + const override = ptyId ? getFitOverrideForPty(ptyId) : null if (override) { if (pane.terminal.cols !== override.cols || pane.terminal.rows !== override.rows) { pane.terminal.resize(override.cols, override.rows)