Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions src/renderer/lib/terminal/terminalLifecycle.ts
Original file line number Diff line number Diff line change
Expand Up @@ -337,6 +337,22 @@ export async function getOrCreate(panelId: string, opts: CreateOpts): Promise<Re
// brand-new PTY, so the instant-exit diagnostic applies.
wireTerminalListeners({ panelId, ptyId, opts, terminal, cleanupListeners, freshSpawn: true })

// 6b. Push the terminal's ACTUAL size to the freshly spawned PTY.
//
// The entry is registered before the awaits above so concurrent callers
// share one object — which also means attach() can, and normally does, fit
// the xterm to its real container while we are still waiting for the spawn.
// Those fits call terminal.resize(), which fires onResize with no listener
// attached yet, so the new size never reaches the PTY: the grid is correct
// on screen while the PTY stays at the 80x24 it was created with. Nothing
// corrects it afterwards either, because fit() only resizes when the size
// CHANGES — so a terminal the user never happens to resize by hand keeps a
// shell wrapping at 80 columns for the rest of its life, and any TUI
// started in it draws its frame to 80.
if (terminal.cols !== cols || terminal.rows !== rows) {
electronAPI.terminalResize(ptyId, terminal.cols, terminal.rows)
}

// 11. Write initialInput immediately — the PTY buffers writes until the
// shell is ready to consume them, so a fixed setTimeout was both
// fragile (slow systems) and unnecessary.
Expand Down
62 changes: 61 additions & 1 deletion src/renderer/lib/terminal/terminalRegistry.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,9 @@ vi.mock('../logger', () => ({ default: { warn: () => {}, info: () => {}, error:
// a pre-existing ptyId. The bug manifests when a leaked pending-transfer
// causes a fresh getOrCreate to silently go down the reconnect path.
const terminalCreate = vi.fn(async () => 'pty-fresh')
// Every winsize pushed to the PTY. Used to pin that a terminal fitted while the
// spawn was still in flight still gets its real size through.
const terminalResize = vi.fn()
const panelTransferAck = vi.fn(async (_id: string) => undefined as undefined)
// Process-wide WebGL grant broker (main-side); default to granting so the
// attach() upgrade path runs. Individual tests can override the resolved value.
Expand All @@ -136,6 +139,7 @@ beforeEach(() => {
settingsState.terminalContrast = 4.5
settingsState.terminalOptionIsMeta = true
terminalCreate.mockClear()
terminalResize.mockClear()
panelTransferAck.mockClear()
webglRequestGrant.mockClear()
webglRequestGrant.mockImplementation(async () => true)
Expand All @@ -149,7 +153,7 @@ beforeEach(() => {
value: {
terminalCreate,
terminalWrite: vi.fn(),
terminalResize: vi.fn(),
terminalResize,
terminalKill: vi.fn(async () => undefined),
onTerminalData: vi.fn(() => () => {}),
onTerminalExit: vi.fn(() => () => {}),
Expand Down Expand Up @@ -821,3 +825,59 @@ describe('process-wide WebGL context grant lifecycle', () => {
terminalRegistry.dispose('panel-reset')
})
})

// Regression: a terminal the user never resizes by hand keeps its PTY at the
// 80x24 spawn default forever, so the shell wraps at 80 columns and any TUI
// started in it draws its frame to 80 — while the on-screen grid looks correct.
//
// Root cause: getOrCreate registers the entry BEFORE awaiting the spawn (so
// concurrent callers share one object), which lets attach() fit the xterm to its
// real container while the spawn is still in flight. Those fits call
// terminal.resize(), firing onResize before the listener that forwards it to the
// PTY exists — the size is applied to xterm and dropped on the floor for the
// PTY. Nothing corrects it later because fit() only resizes on a CHANGE.
//
// Contract: once the spawn resolves, the PTY must be told the terminal's actual
// size.
describe('PTY adopts a size the terminal reached during spawn', () => {
it('pushes the real size once the spawn resolves', async () => {
const { terminalRegistry } = await import('./terminalRegistry')

// Hold the spawn open so we can fit the terminal mid-flight, exactly as
// attach() does while getOrCreate is still awaiting.
let releaseSpawn!: (id: string) => void
terminalCreate.mockImplementationOnce(
() => new Promise<string>((resolve) => { releaseSpawn = resolve }),
)

const pending = terminalRegistry.getOrCreate('panel-race', { workspaceId: 'ws-1' })

// getOrCreate awaits settingsGet before terminalCreate; drain the queue so
// the spawn is genuinely in flight when we fit.
await new Promise((resolve) => setTimeout(resolve, 0))

// The entry is already registered even though the spawn hasn't resolved.
const entry = terminalRegistry.getEntry('panel-race')!
expect(entry.terminal.cols).toBe(80)
entry.terminal.resize(120, 40) // what safeFit() does — no PTY listener yet

releaseSpawn('pty-fresh')
await pending

expect(terminalResize).toHaveBeenCalledWith('pty-fresh', 120, 40)

terminalRegistry.dispose('panel-race')
})

it('stays quiet when the terminal is still at the spawn size', async () => {
const { terminalRegistry } = await import('./terminalRegistry')

// Never fitted (panel not laid out yet): the PTY was created at 80x24 and
// is already correct, so re-sending it would be a pointless SIGWINCH.
await terminalRegistry.getOrCreate('panel-norace', { workspaceId: 'ws-1' })

expect(terminalResize).not.toHaveBeenCalled()

terminalRegistry.dispose('panel-norace')
})
})