mirror of
https://github.com/stablyai/orca.git
synced 2026-10-21 16:03:26 +00:00
Fix pane ID collision in mobile-fit overrides by using ptyId-based lookups
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 <help@stably.ai>
This commit is contained in:
@@ -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()
|
||||
}
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
|
||||
@@ -10,7 +10,9 @@ type FitOverride = {
|
||||
}
|
||||
|
||||
const overridesByPtyId = new Map<string, FitOverride>()
|
||||
const ptyIdByPaneId = new Map<number, string>()
|
||||
// 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<string, string>()
|
||||
|
||||
// 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(
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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)
|
||||
|
||||
Reference in New Issue
Block a user