fix(terminal): restore the floating workspace open and maximized (#13258)

* fix(terminal): restore the floating workspace open and maximized

The panel's open and maximized flags were never persisted, so every restart
dropped the user into a closed, default-sized panel that they reopened and
re-maximized by hand. On a typical window that is 113 columns to 211, and
113/211 is the ~55% left band in the bug report.

That column jump reflows the xterm buffer. Rows written at the narrow width
carry wrapped continuations that unwrap into the wide grid, leaving
interleaved tails and stacked status lines under a live relative-cursor TUI.
The live region recovers on the next repaint; the reflowed scrollback never
does. Correct PTY sizing cannot undo it, so the fix is to not make the jump.

Persist both flags, restore maximized geometry in the bounds initializer so
the first paint is already final, and hold the panel's terminals until the
viewport settles - the window restores its saved bounds and only then
maximizes, so mounting earlier fits terminals to a grid it is about to leave.

* fix(terminal): stop a boot-time flag read from wiping the restored open state

Settings hydrate asynchronously, so floatingTerminalEnabled reads false on
every boot before it resolves. The feature-off effect force-closes the panel,
and that close was being persisted - overwriting the user's restored open
preference with a value they never chose.

Measured on a real restart: storage ended as {"maximized":true,"open":false}
after a session where both were true, so the panel came back closed and the
restore did nothing.

Persist only while the feature is enabled, which is the only state in which
the value reflects a real user choice.

* fix(terminal): only force-close the floating workspace on a hydrated flag-off

The feature-off effect fires on every boot while settings are still
undefined, so it closed the restored panel before the real flag value
arrived. The previous commit stopped that close from being persisted, but
the React state was still discarded, so the panel never actually reopened.

Measured on a real restart: storage kept open:true and maximized:true, yet
the workspace stayed closed. Gate the close on settings having hydrated -
only a real flag-off is a disable.

* fix(terminal): treat null settings as unhydrated in the floating-panel gate

The settings slice initializes to null, not undefined, so the hydration
selector read "hydrated" on the very first render and the feature-off
close still fired at boot - measured again as storage keeping open:true
while the panel stayed closed. Check for null.

* fix(terminal): preserve floating restore bounds after restart
This commit is contained in:
Brennan Benson 2026-08-09 14:09:29 -07:00 committed by GitHub
parent 850342a3e0
commit c19bac80d6
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
7 changed files with 352 additions and 16 deletions

View File

@ -69,6 +69,10 @@ import { onOnboardingReopened } from './components/onboarding/show-onboarding-ev
import { shouldShowOnboarding } from './components/onboarding/should-show-onboarding'
import { MarkdownTemplatePicker } from './components/editor/MarkdownTemplatePicker'
import { FloatingTerminalToggleButton } from './components/floating-terminal/FloatingTerminalToggleButton'
import {
persistFloatingTerminalPanelOpen,
readPersistedFloatingTerminalPanelViewState
} from './components/floating-terminal/floating-terminal-panel-view-state'
import {
TOGGLE_FLOATING_TERMINAL_EVENT,
requestFloatingTerminalOpenMaximized
@ -440,7 +444,11 @@ function App(): React.JSX.Element {
const clearUnreadDockBadge = useUnreadDockBadge()
useRadixBodyPointerEventsRecovery()
useWebSessionTabsSync()
const [floatingTerminalOpen, setFloatingTerminalOpen] = useState(false)
// Why restored: leaving the panel closed forces the user to reopen and re-maximize it,
// and that size jump reflows a live TUI's buffer (see floating-terminal-panel-view-state).
const [floatingTerminalOpen, setFloatingTerminalOpen] = useState(
() => readPersistedFloatingTerminalPanelViewState()?.open === true
)
const floatingWorkspaceTourInteractionSnapshotRef = useRef<{
wasPreviouslyInteracted?: boolean
persisted?: Promise<void>
@ -535,6 +543,10 @@ function App(): React.JSX.Element {
const historyBackShortcutLabel = useShortcutLabel('worktree.history.back')
const historyForwardShortcutLabel = useShortcutLabel('worktree.history.forward')
const floatingTerminalEnabled = useAppStore((s) => s.settings?.floatingTerminalEnabled === true)
// Why tracked separately: the flag reads false while settings are still loading, and a
// false read at boot must not be treated as the user disabling the feature. The store
// initializes `settings` to null (not undefined) - fetchSettings replaces it atomically.
const floatingTerminalSettingsHydrated = useAppStore((s) => s.settings != null)
const floatingTerminalTriggerLocation = useAppStore(
(s) => s.settings?.floatingTerminalTriggerLocation ?? 'floating-button'
)
@ -632,8 +644,19 @@ function App(): React.JSX.Element {
restoreFloatingTerminalReturnFocus()
}
setFloatingTerminalOpen(resolvedOpen)
// Why gated on the flag: `settings` is undefined until it hydrates, so the
// feature-off effect force-closes the panel on every boot. Persisting there would
// overwrite the user's restored `open` with a value they never chose.
if (floatingTerminalEnabled) {
persistFloatingTerminalPanelOpen(resolvedOpen)
}
},
[floatingTerminalOpen, rememberFloatingTerminalReturnFocus, restoreFloatingTerminalReturnFocus]
[
floatingTerminalEnabled,
floatingTerminalOpen,
rememberFloatingTerminalReturnFocus,
restoreFloatingTerminalReturnFocus
]
)
useEffect(() => {
@ -647,10 +670,13 @@ function App(): React.JSX.Element {
}, [floatingTerminalEnabled, setFloatingTerminalOpenWithFocus])
useEffect(() => {
if (!floatingTerminalEnabled) {
// Why the hydration gate: this effect fires on every boot while settings are still
// undefined, and closing there discards the restored open state before the real flag
// value arrives. Only a hydrated flag-off is an actual disable.
if (floatingTerminalSettingsHydrated && !floatingTerminalEnabled) {
setFloatingTerminalOpenWithFocus(false)
}
}, [floatingTerminalEnabled, setFloatingTerminalOpenWithFocus])
}, [floatingTerminalSettingsHydrated, floatingTerminalEnabled, setFloatingTerminalOpenWithFocus])
const sidebarWidth = useAppStore((s) => s.sidebarWidth)
const sidebarOpen = useAppStore((s) => s.sidebarOpen)

View File

@ -18,6 +18,7 @@ import {
consumeFloatingTerminalOpenMaximizedIntent,
requestFloatingTerminalOpenMaximized
} from '@/lib/floating-terminal'
import { FLOATING_TERMINAL_PANEL_VIEW_STATE_STORAGE_KEY } from './floating-terminal-panel-view-state'
import {
clearFloatingPanelReclaimIntent,
consumeFloatingPanelReclaimIntent
@ -1197,14 +1198,45 @@ describe('FloatingTerminalPanel close behavior', () => {
element = await renderPanel(true)
expect(getPanelStyleBounds(element)).toEqual(getMaximizedFloatingTerminalBounds())
expect(getMockedLocalStorage().setItem).not.toHaveBeenCalled()
// Why key-scoped rather than "no writes": maximize now persists panel view state under
// its own key. The invariant here is that the saved NORMAL bounds are never clobbered.
expect(getMockedLocalStorage().setItem).not.toHaveBeenCalledWith(
FLOATING_TERMINAL_PANEL_BOUNDS_STORAGE_KEY,
expect.anything()
)
const restoredControls = findByTypeName(element, 'FloatingTerminalWindowControls')
;(restoredControls.props.onToggleMaximized as () => void)()
element = await renderPanel(true)
expect(getPanelStyleBounds(element)).toEqual(savedBounds)
expect(getMockedLocalStorage().setItem).not.toHaveBeenCalled()
// Why key-scoped rather than "no writes": maximize now persists panel view state under
// its own key. The invariant here is that the saved NORMAL bounds are never clobbered.
expect(getMockedLocalStorage().setItem).not.toHaveBeenCalledWith(
FLOATING_TERMINAL_PANEL_BOUNDS_STORAGE_KEY,
expect.anything()
)
})
it('restores saved normal bounds after starting maximized', async () => {
const savedBounds = { left: 120, top: 96, width: 760, height: 420 }
getMockedLocalStorage().getItem.mockImplementation((key: string) => {
if (key === FLOATING_TERMINAL_PANEL_BOUNDS_STORAGE_KEY) {
return JSON.stringify(savedBounds)
}
return key === FLOATING_TERMINAL_PANEL_VIEW_STATE_STORAGE_KEY
? JSON.stringify({ open: true, maximized: true })
: null
})
let element = await renderPanel(true)
expect(getPanelStyleBounds(element)).toEqual(getMaximizedFloatingTerminalBounds())
const controls = findByTypeName(element, 'FloatingTerminalWindowControls')
;(controls.props.onToggleMaximized as () => void)()
element = await renderPanel(true)
expect(getPanelStyleBounds(element)).toEqual(savedBounds)
})
it('restores committed normal bounds after maximizing from a skinny clamp', async () => {
@ -1239,7 +1271,12 @@ describe('FloatingTerminalPanel close behavior', () => {
width: 920,
height: 560
})
expect(getMockedLocalStorage().setItem).not.toHaveBeenCalled()
// Why key-scoped rather than "no writes": maximize now persists panel view state under
// its own key. The invariant here is that the saved NORMAL bounds are never clobbered.
expect(getMockedLocalStorage().setItem).not.toHaveBeenCalledWith(
FLOATING_TERMINAL_PANEL_BOUNDS_STORAGE_KEY,
expect.anything()
)
})
it('does not bootstrap a terminal tab when the panel opens empty', async () => {

View File

@ -95,18 +95,24 @@ export { FloatingTerminalToggleButton } from './FloatingTerminalToggleButton'
import {
anchorFloatingTerminalPanelBounds,
clampFloatingTerminalBounds,
getDefaultFloatingTerminalCommittedBounds,
getDefaultFloatingTerminalBounds,
getDefaultFloatingTerminalCommittedBounds,
getMaximizedFloatingTerminalBounds,
persistFloatingTerminalPanelBounds,
readPersistedFloatingTerminalPanelBounds,
resolveFloatingTerminalPanelCommittedBounds,
resolveFloatingTerminalPanelBounds,
resolveFloatingTerminalPanelCommittedBounds,
shouldReconcileFloatingTerminalPanelBounds,
type FloatingTerminalPanelBounds,
type FloatingTerminalPanelCommittedBounds,
type FloatingTerminalPanelBoundsSource
type FloatingTerminalPanelBoundsSource,
type FloatingTerminalPanelCommittedBounds
} from './floating-terminal-panel-bounds'
import {
persistFloatingTerminalPanelMaximized,
readPersistedFloatingTerminalPanelViewState
} from './floating-terminal-panel-view-state'
import { useSettledPanelViewport } from './use-settled-panel-viewport'
import { shouldRestoreMaximizedPanelBounds } from './floating-terminal-panel-restore-geometry'
import { translate } from '@/i18n/i18n'
import { consumeFloatingTerminalOpenMaximizedIntent } from '@/lib/floating-terminal'
import { selectFloatingTerminalPanelInputs } from './floating-terminal-panel-inputs'
@ -162,6 +168,17 @@ function readInitialPanelBounds(): FloatingTerminalPanelBoundsState {
const defaultCommittedBounds = getDefaultFloatingTerminalCommittedBounds()
const defaultRenderedBounds = getDefaultFloatingTerminalBounds()
const persistedBounds = readPersistedFloatingTerminalPanelBounds()
if (shouldRestoreMaximizedPanelBounds(readPersistedFloatingTerminalPanelViewState())) {
// Why maximized wins the RENDERED rect while the committed rect stays the restore
// target: the first paint must already be final geometry, or the panes fit at the
// smaller size and the later maximize reflows them. Un-maximizing still returns to
// the user's own bounds because those remain committed.
return {
committedBounds: persistedBounds ?? defaultCommittedBounds,
renderedBounds: getMaximizedFloatingTerminalBounds(),
source: persistedBounds ? 'user' : 'default'
}
}
return persistedBounds
? {
committedBounds: persistedBounds,
@ -268,7 +285,10 @@ export function FloatingTerminalPanel({
initialBoundsStateRef.current.committedBounds
)
const [bounds, setBounds] = useState(initialBoundsStateRef.current.renderedBounds)
const [maximized, setMaximized] = useState(false)
const [maximized, setMaximized] = useState(
() => readPersistedFloatingTerminalPanelViewState()?.maximized === true
)
const panelViewportSettled = useSettledPanelViewport()
const [orchestrationDialogOpen, setOrchestrationDialogOpen] = useState(false)
const [showOrchestrationSetup, setShowOrchestrationSetup] = useState(
() => !hasOrchestrationSetupMarker() && !isOrchestrationSetupDismissed()
@ -1115,9 +1135,9 @@ export function FloatingTerminalPanel({
const toggleMaximized = useCallback(() => {
if (maximized) {
const restoredState = restoreBoundsRef.current ?? {
committedBounds: getDefaultFloatingTerminalCommittedBounds(),
renderedBounds: getDefaultFloatingTerminalBounds(),
source: 'default' as const
committedBounds: committedBoundsRef.current,
renderedBounds: resolveFloatingTerminalPanelCommittedBounds(committedBoundsRef.current),
source: boundsSourceRef.current
}
restoreBoundsRef.current = null
boundsSourceRef.current = restoredState.source
@ -1128,6 +1148,7 @@ export function FloatingTerminalPanel({
stagedBoundsRef.current = null
setBounds(restoredBounds)
setMaximized(false)
persistFloatingTerminalPanelMaximized(false)
return
}
restoreBoundsRef.current = {
@ -1138,6 +1159,7 @@ export function FloatingTerminalPanel({
stagedBoundsRef.current = null
setBounds(getMaximizedFloatingTerminalBounds())
setMaximized(true)
persistFloatingTerminalPanelMaximized(true)
}, [bounds, maximized])
const maximizePanel = useCallback(() => {
@ -1155,6 +1177,7 @@ export function FloatingTerminalPanel({
stagedBoundsRef.current = null
setBounds(getMaximizedFloatingTerminalBounds())
setMaximized(true)
persistFloatingTerminalPanelMaximized(true)
}, [bounds, maximized])
useEffect(() => {
@ -1840,7 +1863,11 @@ export function FloatingTerminalPanel({
hasVisibleFloatingTabs ? 'floating-workspace-surface' : undefined
}
>
{cwd
{/* Why also gated on a settled viewport: a restored-maximized panel derives its
rect from the live viewport, so mounting terminals before the window finishes
maximizing fits them to a grid it is about to leave, and the correcting fit
reflows the buffer under a live TUI. */}
{cwd && panelViewportSettled
? tabs
.filter((tab) => !parkedTerminalTabIds.has(tab.id))
.map((tab) => {

View File

@ -0,0 +1,18 @@
import { hasUsableFloatingTerminalPanelViewport } from './floating-terminal-panel-bounds'
import type { FloatingTerminalPanelViewState } from './floating-terminal-panel-view-state'
/**
* Whether the panel's first rendered rect should be the maximized one.
*
* Why this is a decision and not just a flag read: maximized geometry is derived from the
* live viewport, so restoring it against a viewport too small to hold the panel would pin
* the terminals to a grid the window is about to leave the size jump this restore exists
* to remove. When the viewport cannot answer yet, fall back to the committed bounds and let
* the ordinary reconcile path maximize once layout is real.
*/
export function shouldRestoreMaximizedPanelBounds(
viewState: FloatingTerminalPanelViewState | null,
hasUsableViewport: () => boolean = hasUsableFloatingTerminalPanelViewport
): boolean {
return viewState?.maximized === true && hasUsableViewport()
}

View File

@ -0,0 +1,93 @@
/** @vitest-environment happy-dom */
import { afterEach, describe, expect, it } from 'vitest'
import {
FLOATING_TERMINAL_PANEL_VIEW_STATE_STORAGE_KEY,
persistFloatingTerminalPanelMaximized,
persistFloatingTerminalPanelOpen,
readPersistedFloatingTerminalPanelViewState
} from './floating-terminal-panel-view-state'
import { shouldRestoreMaximizedPanelBounds } from './floating-terminal-panel-restore-geometry'
afterEach(() => {
window.localStorage.clear()
})
describe('floating terminal panel view state', () => {
it('returns null when nothing was ever persisted', () => {
expect(readPersistedFloatingTerminalPanelViewState()).toBeNull()
})
it('round-trips both flags', () => {
persistFloatingTerminalPanelOpen(true)
persistFloatingTerminalPanelMaximized(true)
expect(readPersistedFloatingTerminalPanelViewState()).toEqual({ open: true, maximized: true })
})
it('does not clobber the other owner_s flag', () => {
// Why: `open` is written by the app shell and `maximized` by the panel, so a
// whole-record write from either would drop the other's value.
persistFloatingTerminalPanelMaximized(true)
persistFloatingTerminalPanelOpen(false)
expect(readPersistedFloatingTerminalPanelViewState()).toEqual({ open: false, maximized: true })
persistFloatingTerminalPanelOpen(true)
persistFloatingTerminalPanelMaximized(false)
expect(readPersistedFloatingTerminalPanelViewState()).toEqual({ open: true, maximized: false })
})
it('lets a later write destroy a restored open preference', () => {
// Why pinned: this is the hazard the App-side guard exists for. `settings` hydrates
// asynchronously, so the feature flag reads false on every boot before it resolves and
// the feature-off effect force-closes the panel. If that path persists, it overwrites a
// preference the user never changed - which is exactly what shipped and had to be fixed.
persistFloatingTerminalPanelOpen(true)
expect(readPersistedFloatingTerminalPanelViewState()?.open).toBe(true)
persistFloatingTerminalPanelOpen(false)
expect(readPersistedFloatingTerminalPanelViewState()?.open).toBe(false)
})
it('restores the half a older record carries', () => {
// Why: a record written before the second flag existed must still restore.
window.localStorage.setItem(
FLOATING_TERMINAL_PANEL_VIEW_STATE_STORAGE_KEY,
JSON.stringify({ open: true })
)
expect(readPersistedFloatingTerminalPanelViewState()).toEqual({ open: true, maximized: false })
})
it('treats malformed storage as absent instead of throwing', () => {
for (const value of ['not json', '[]', 'null', '"open"', '42']) {
window.localStorage.setItem(FLOATING_TERMINAL_PANEL_VIEW_STATE_STORAGE_KEY, value)
expect(readPersistedFloatingTerminalPanelViewState()).toBeNull()
}
})
it('ignores non-boolean flag values rather than coercing them', () => {
window.localStorage.setItem(
FLOATING_TERMINAL_PANEL_VIEW_STATE_STORAGE_KEY,
JSON.stringify({ open: 'yes', maximized: 1 })
)
expect(readPersistedFloatingTerminalPanelViewState()).toEqual({ open: false, maximized: false })
})
})
describe('shouldRestoreMaximizedPanelBounds', () => {
it('restores maximized geometry only when the viewport can hold it', () => {
expect(shouldRestoreMaximizedPanelBounds({ open: true, maximized: true }, () => true)).toBe(
true
)
// Why: maximized bounds come from the live viewport, so restoring against one too small
// pins terminals to a grid the window is about to leave - the jump this restore removes.
expect(shouldRestoreMaximizedPanelBounds({ open: true, maximized: true }, () => false)).toBe(
false
)
})
it('does not restore maximized geometry for a non-maximized or absent record', () => {
expect(shouldRestoreMaximizedPanelBounds({ open: true, maximized: false }, () => true)).toBe(
false
)
expect(shouldRestoreMaximizedPanelBounds(null, () => true)).toBe(false)
})
})

View File

@ -0,0 +1,66 @@
export const FLOATING_TERMINAL_PANEL_VIEW_STATE_STORAGE_KEY =
'orca-floating-terminal-panel-view-state-v1'
export type FloatingTerminalPanelViewState = {
open: boolean
maximized: boolean
}
function getWindowStorage(): Storage | null {
return typeof window === 'undefined' ? null : window.localStorage
}
/**
* Why persisted separately from the bounds record: maximized geometry is derived
* from the live viewport rather than stored, so this is view state, not a rect.
* Keeping it out of the bounds union also keeps that type a pure rectangle.
*
* Why it matters: an unpersisted maximize means every restart drops the user into
* a default-sized panel that they re-maximize by hand. That size jump reflows the
* xterm buffer under a live relative-cursor TUI, which unwraps its rows and leaves
* permanently mangled scrollback. Restoring the panel as it was removes the jump.
*/
export function readPersistedFloatingTerminalPanelViewState(): FloatingTerminalPanelViewState | null {
try {
const serialized = getWindowStorage()?.getItem(FLOATING_TERMINAL_PANEL_VIEW_STATE_STORAGE_KEY)
if (!serialized) {
return null
}
const parsed: unknown = JSON.parse(serialized)
if (parsed === null || typeof parsed !== 'object' || Array.isArray(parsed)) {
return null
}
const record = parsed as Record<string, unknown>
// Why each flag is read independently: a record written before the other flag
// existed must still restore the half it does carry.
return {
open: record.open === true,
maximized: record.maximized === true
}
} catch {
return null
}
}
export function persistFloatingTerminalPanelViewState(state: FloatingTerminalPanelViewState): void {
try {
getWindowStorage()?.setItem(
FLOATING_TERMINAL_PANEL_VIEW_STATE_STORAGE_KEY,
JSON.stringify(state)
)
} catch {
// Why: storage can be unavailable or full; losing the restore is not worth a crash.
}
}
// Why field-scoped writers: `open` is owned by the app shell and `maximized` by the panel,
// so a whole-record write from either side would clobber the other's flag.
export function persistFloatingTerminalPanelOpen(open: boolean): void {
const current = readPersistedFloatingTerminalPanelViewState()
persistFloatingTerminalPanelViewState({ maximized: current?.maximized === true, open })
}
export function persistFloatingTerminalPanelMaximized(maximized: boolean): void {
const current = readPersistedFloatingTerminalPanelViewState()
persistFloatingTerminalPanelViewState({ open: current?.open === true, maximized })
}

View File

@ -0,0 +1,69 @@
import { useEffect, useState } from 'react'
// Why two frames: the main window restores its saved bounds and only then maximizes, so
// the renderer's first layout can be a size the window is about to leave. One unchanged
// frame can land inside that gap; two straddle it.
const REQUIRED_STABLE_FRAMES = 2
// Why a cap: a window that never stops resizing (a drag, a display change) must not hold
// the panel's terminals forever. Mounting at a stale size is recoverable; never mounting is not.
const MAX_SETTLE_MS = 300
function readViewport(): { width: number; height: number } {
return { width: window.innerWidth, height: window.innerHeight }
}
/**
* Reports true once the viewport has held one size across consecutive frames.
*
* Why the floating panel waits for this: its maximized rect is derived from the live
* viewport, so mounting terminals against a pre-maximize viewport fits them to a grid the
* window is about to leave. The correcting fit then reflows the xterm buffer under a live
* TUI, which is the damage this whole path exists to avoid. Latching true is deliberate
* later resizes are ordinary user resizes and the normal fit path owns them.
*/
export function useSettledPanelViewport(): boolean {
const [settled, setSettled] = useState(false)
useEffect(() => {
if (settled || typeof window === 'undefined') {
return
}
if (typeof requestAnimationFrame !== 'function') {
setSettled(true)
return
}
let frameId: number | null = null
let previous = readViewport()
let stableFrames = 0
const settle = (): void => {
if (frameId !== null) {
cancelAnimationFrame(frameId)
frameId = null
}
setSettled(true)
}
const capId = setTimeout(settle, MAX_SETTLE_MS)
const step = (): void => {
const current = readViewport()
stableFrames =
current.width === previous.width && current.height === previous.height
? stableFrames + 1
: 0
previous = current
if (stableFrames >= REQUIRED_STABLE_FRAMES) {
settle()
return
}
frameId = requestAnimationFrame(step)
}
frameId = requestAnimationFrame(step)
return () => {
clearTimeout(capId)
if (frameId !== null) {
cancelAnimationFrame(frameId)
}
}
}, [settled])
return settled
}