fix(terminal): report PTY's applied size so dropped resizes self-heal (split-mount desync) (#6785)
Co-authored-by: Orca <help@stably.ai>
This commit is contained in:
parent
5f3f45354d
commit
22b00a7cd2
|
|
@ -0,0 +1,22 @@
|
|||
import type { TerminalOscLinkRange } from '../../shared/terminal-osc-link-ranges'
|
||||
import type { TerminalModes } from './types'
|
||||
|
||||
/** On-disk shape of checkpoint.json. Written by history-manager, read by
|
||||
* history-reader — one type so the generation pairing with output.log's
|
||||
* header (see terminal-history-log.ts) cannot silently diverge between the
|
||||
* writer and the consumer. */
|
||||
export type TerminalCheckpointFile = {
|
||||
snapshotAnsi: string
|
||||
scrollbackAnsi: string
|
||||
oscLinks?: TerminalOscLinkRange[]
|
||||
rehydrateSequences: string
|
||||
cwd: string | null
|
||||
cols: number
|
||||
rows: number
|
||||
modes: TerminalModes
|
||||
scrollbackLines: number
|
||||
/** Ties this checkpoint to the output.log whose header carries the same
|
||||
* generation. Absent on checkpoints written before incremental logs. */
|
||||
generation?: number
|
||||
checkpointedAt: string
|
||||
}
|
||||
|
|
@ -178,6 +178,46 @@ describe('DaemonPtyAdapter (IPtyProvider)', () => {
|
|||
})
|
||||
})
|
||||
|
||||
describe('getAppliedSize', () => {
|
||||
it('reports the spawn dims before any resize', async () => {
|
||||
const { id } = await adapter.spawn({ cols: 80, rows: 24 })
|
||||
expect(await adapter.getAppliedSize(id)).toEqual({ cols: 80, rows: 24 })
|
||||
})
|
||||
|
||||
it('reflects the size the daemon actually applied after a resize', async () => {
|
||||
const { id } = await adapter.spawn({ cols: 80, rows: 24 })
|
||||
adapter.resize(id, 120, 40)
|
||||
await waitFor(() => vi.mocked(lastSubprocess.resize).mock.calls.length > 0)
|
||||
expect(await adapter.getAppliedSize(id)).toEqual({ cols: 120, rows: 40 })
|
||||
})
|
||||
|
||||
// Why: this is the regression the fix targets. resize() is a fire-and-forget
|
||||
// notify; a resize that arrives after the session exited is silently dropped
|
||||
// daemon-side. getAppliedSize must keep reporting the last size the PTY
|
||||
// genuinely took (the stale pre-exit dims) rather than the dropped request —
|
||||
// so the renderer can tell its requested size never landed. The old
|
||||
// requested-size cache would have masked the drop.
|
||||
it('does not advance when a resize is dropped after the session exited', async () => {
|
||||
const { id } = await adapter.spawn({ cols: 200, rows: 50 })
|
||||
|
||||
// Simulate the child exiting, then a late narrow resize racing in. The
|
||||
// daemon Session.resize early-returns for an exited session, so the child
|
||||
// is never resized and the applied size never becomes the requested 80×24.
|
||||
lastSubprocess._simulateExit(0)
|
||||
await new Promise((r) => setTimeout(r, 50))
|
||||
|
||||
adapter.resize(id, 80, 24)
|
||||
await new Promise((r) => setTimeout(r, 50))
|
||||
|
||||
// The drop must be visible: the subprocess was never resized to the narrow
|
||||
// dims the renderer requested, and getAppliedSize never reports 80 cols —
|
||||
// it stays wide (or null once reaped), never masking the drop as "applied".
|
||||
expect(lastSubprocess.resize).not.toHaveBeenCalledWith(80, 24)
|
||||
const applied = await adapter.getAppliedSize(id)
|
||||
expect(applied?.cols).not.toBe(80)
|
||||
})
|
||||
})
|
||||
|
||||
describe('shutdown', () => {
|
||||
it('kills the session', async () => {
|
||||
const { id } = await adapter.spawn({ cols: 80, rows: 24 })
|
||||
|
|
|
|||
|
|
@ -362,6 +362,24 @@ export class DaemonPtyAdapter implements IPtyProvider {
|
|||
return this.initialCwds.get(id) ?? ''
|
||||
}
|
||||
|
||||
// Why: resize() is a fire-and-forget notify, so a resize can be dropped
|
||||
// daemon-side (session not yet alive, exited, invalid dims, cold-restore
|
||||
// snapshot-col coercion) without the renderer knowing. This reads the size
|
||||
// the daemon actually applied so the renderer can detect that drift on resume
|
||||
// and re-assert. Null (RPC failure / unknown session) means "cannot confirm",
|
||||
// which the renderer treats as a cue to re-forward once.
|
||||
async getAppliedSize(id: string): Promise<{ cols: number; rows: number } | null> {
|
||||
try {
|
||||
const result = await this.client.request<{ size: { cols: number; rows: number } | null }>(
|
||||
'getSize',
|
||||
{ sessionId: id }
|
||||
)
|
||||
return result.size ?? null
|
||||
} catch {
|
||||
return null
|
||||
}
|
||||
}
|
||||
|
||||
async clearBuffer(id: string): Promise<void> {
|
||||
await this.client.request('clearScrollback', { sessionId: id })
|
||||
this.markSessionDirty(id)
|
||||
|
|
|
|||
|
|
@ -96,6 +96,10 @@ export class DaemonPtyRouter implements IPtyProvider {
|
|||
return this.adapterFor(id).getInitialCwd(id)
|
||||
}
|
||||
|
||||
async getAppliedSize(id: string): Promise<{ cols: number; rows: number } | null> {
|
||||
return (await this.adapterFor(id).getAppliedSize?.(id)) ?? null
|
||||
}
|
||||
|
||||
async clearBuffer(id: string): Promise<void> {
|
||||
await this.adapterFor(id).clearBuffer(id)
|
||||
}
|
||||
|
|
|
|||
|
|
@ -377,6 +377,9 @@ export class DaemonServer {
|
|||
case 'getSnapshot':
|
||||
return { snapshot: this.host.getSnapshot(request.payload.sessionId) }
|
||||
|
||||
case 'getSize':
|
||||
return { size: this.host.getAppliedSize(request.payload.sessionId) }
|
||||
|
||||
case 'takePendingOutput':
|
||||
// Why no await before this call: with includeSnapshot, drain and
|
||||
// serialize must share one synchronous turn — an intervening await
|
||||
|
|
|
|||
|
|
@ -126,6 +126,14 @@ export class HeadlessEmulator {
|
|||
this.terminal.resize(cols, rows)
|
||||
}
|
||||
|
||||
// Why: Session.resize applies this emulator and the node-pty subprocess
|
||||
// together behind the same dead/invalid-size gate, so the emulator's dims are
|
||||
// an accurate proxy for the size the child actually took — and stay stale
|
||||
// when a resize is dropped, which is exactly the drop the renderer must detect.
|
||||
getAppliedSize(): { cols: number; rows: number } {
|
||||
return { cols: this.terminal.cols, rows: this.terminal.rows }
|
||||
}
|
||||
|
||||
getSnapshot(opts: { scrollbackRows?: number } = {}): TerminalSnapshot {
|
||||
const modes = this.getModes()
|
||||
const snapshotAnsi = this.normalizeSnapshotAnsiForModes(
|
||||
|
|
|
|||
|
|
@ -226,6 +226,17 @@ export class Session {
|
|||
return this.emulator.getSnapshot()
|
||||
}
|
||||
|
||||
// Why: the size the PTY actually applied (emulator dims, which Session.resize
|
||||
// advances atomically with the subprocess), so the renderer can detect a
|
||||
// resize that was dropped here (exited/disposed/invalid) instead of trusting
|
||||
// its own last-requested size. Null on a disposed session.
|
||||
getAppliedSize(): { cols: number; rows: number } | null {
|
||||
if (this._disposed) {
|
||||
return null
|
||||
}
|
||||
return this.emulator.getAppliedSize()
|
||||
}
|
||||
|
||||
/** Drains the records accumulated since the last take. Runs synchronously —
|
||||
* when includeSnapshot is set, the serialize happens in the same turn so no
|
||||
* PTY data can land between the drain and the snapshot (which would later
|
||||
|
|
|
|||
|
|
@ -260,6 +260,17 @@ export class TerminalHost {
|
|||
return session.getSnapshot()
|
||||
}
|
||||
|
||||
// Why: read-only readback of the size the PTY actually applied (null-not-throw
|
||||
// like getSnapshot). The renderer compares this against xterm to detect a
|
||||
// resize that was dropped/coerced daemon-side and re-assert it.
|
||||
getAppliedSize(sessionId: string): { cols: number; rows: number } | null {
|
||||
const session = this.sessions.get(sessionId)
|
||||
if (!session || !session.isAlive) {
|
||||
return null
|
||||
}
|
||||
return session.getAppliedSize()
|
||||
}
|
||||
|
||||
// Why: same null-not-throw semantics as getSnapshot — incremental
|
||||
// checkpoints are best-effort against sessions that may have just exited.
|
||||
takePendingOutput(
|
||||
|
|
|
|||
|
|
@ -43,25 +43,10 @@ export type TerminalModes = {
|
|||
alternateScreen: boolean
|
||||
}
|
||||
|
||||
/** On-disk shape of checkpoint.json. Written by history-manager, read by
|
||||
* history-reader — one type so the generation pairing with output.log's
|
||||
* header (see terminal-history-log.ts) cannot silently diverge between the
|
||||
* writer and the consumer. */
|
||||
export type TerminalCheckpointFile = {
|
||||
snapshotAnsi: string
|
||||
scrollbackAnsi: string
|
||||
oscLinks?: TerminalOscLinkRange[]
|
||||
rehydrateSequences: string
|
||||
cwd: string | null
|
||||
cols: number
|
||||
rows: number
|
||||
modes: TerminalModes
|
||||
scrollbackLines: number
|
||||
/** Ties this checkpoint to the output.log whose header carries the same
|
||||
* generation. Absent on checkpoints written before incremental logs. */
|
||||
generation?: number
|
||||
checkpointedAt: string
|
||||
}
|
||||
// The on-disk checkpoint.json shape lives in daemon-checkpoint-file.ts (it
|
||||
// depends only on TerminalModes here) — re-exported so existing importers of
|
||||
// `./types` keep working.
|
||||
export type { TerminalCheckpointFile } from './daemon-checkpoint-file'
|
||||
|
||||
// ─── NDJSON Protocol Messages ───────────────────────────────────────
|
||||
|
||||
|
|
@ -225,6 +210,17 @@ export type GetSnapshotRequest = {
|
|||
}
|
||||
}
|
||||
|
||||
// Why: read-only readback of the size the PTY actually applied (vs the size the
|
||||
// renderer last requested via the fire-and-forget resize notify). Lets the
|
||||
// renderer's resume drift-check re-assert a resize the daemon dropped/coerced.
|
||||
export type GetSizeRequest = {
|
||||
id: string
|
||||
type: 'getSize'
|
||||
payload: {
|
||||
sessionId: string
|
||||
}
|
||||
}
|
||||
|
||||
// ─── Incremental checkpoint records (v13+) ──────────────────────────
|
||||
// Why: the 5s checkpoint used to re-serialize the full emulator buffer per
|
||||
// tick, stalling the daemon's PTY pump for O(buffer). Incremental checkpoints
|
||||
|
|
@ -283,6 +279,7 @@ export type DaemonRequest =
|
|||
| SystemResolverHealthRequest
|
||||
| PtySpawnHealthRequest
|
||||
| GetSnapshotRequest
|
||||
| GetSizeRequest
|
||||
| TakePendingOutputRequest
|
||||
|
||||
// ─── RPC Responses (Daemon → Client, on control socket) ────────────
|
||||
|
|
|
|||
|
|
@ -3391,6 +3391,107 @@ describe('registerPtyHandlers', () => {
|
|||
expect(provider.getForegroundProcess).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
// Why: regression for the Claude-Code split-pane garbled-render desync. resize
|
||||
// is fire-and-forget for daemon-backed PTYs, so a corrective narrow resize can
|
||||
// be dropped while the renderer believes it landed. pty:getSize must report the
|
||||
// size the PTY ACTUALLY applied (so the renderer's resume drift-check re-asserts
|
||||
// the dropped resize) rather than the size last requested. This models a daemon
|
||||
// provider whose resize is dropped but whose getAppliedSize stays at the wide
|
||||
// spawn size; pty:getSize must surface the wide (applied) size, not the narrow
|
||||
// (requested) one.
|
||||
describe('pty:getSize reports applied size, not requested size', () => {
|
||||
function setupProviderWithAppliedSize(args: {
|
||||
applied: { cols: number; rows: number } | null
|
||||
resize?: (cols: number, rows: number) => void
|
||||
getAppliedSize?: (id: string) => Promise<{ cols: number; rows: number } | null>
|
||||
}): void {
|
||||
setLocalPtyProvider({
|
||||
spawn: vi.fn(async (opts: { sessionId?: string }) => ({
|
||||
id: opts.sessionId ?? 'daemon-pty'
|
||||
})),
|
||||
write: vi.fn(),
|
||||
resize: vi.fn(args.resize ?? (() => {})),
|
||||
getAppliedSize: vi.fn(args.getAppliedSize ?? (async () => args.applied)),
|
||||
kill: vi.fn(),
|
||||
shutdown: vi.fn(),
|
||||
onData: vi.fn(() => vi.fn()),
|
||||
onExit: vi.fn(() => vi.fn()),
|
||||
listProcesses: vi.fn(async () => []),
|
||||
getForegroundProcess: vi.fn(async () => null)
|
||||
} as never)
|
||||
}
|
||||
|
||||
const resizeListener = (): ((event: unknown, args: unknown) => void) => {
|
||||
const call = onMock.mock.calls.find((entry: unknown[]) => entry[0] === 'pty:resize')
|
||||
if (!call) {
|
||||
throw new Error('missing pty:resize listener')
|
||||
}
|
||||
return call[1] as (event: unknown, args: unknown) => void
|
||||
}
|
||||
|
||||
it('returns the applied (wide) size after a dropped narrow resize', async () => {
|
||||
// The daemon keeps the PTY at its wide spawn size; the narrow resize is
|
||||
// silently dropped (provider.resize is a no-op fire-and-forget).
|
||||
setupProviderWithAppliedSize({ applied: { cols: 200, rows: 50 } })
|
||||
handlers.clear()
|
||||
registerPtyHandlers(mainWindow as never)
|
||||
const spawn = await handlers.get('pty:spawn')!(null, { cols: 200, rows: 50, env: {} })
|
||||
const id = (spawn as { id: string }).id
|
||||
|
||||
// Renderer forwards a corrective narrow resize; it is dropped daemon-side.
|
||||
resizeListener()(mainWindowIpcEvent, { id, cols: 80, rows: 24 })
|
||||
|
||||
// pty:getSize must surface the applied wide size so the renderer detects
|
||||
// drift (xterm=80 vs PTY=200) and re-asserts — NOT the requested 80.
|
||||
const reported = await handlers.get('pty:getSize')!(null, { id })
|
||||
expect(reported).toEqual({ cols: 200, rows: 50 })
|
||||
})
|
||||
|
||||
it('falls back to the requested size when the provider cannot report applied size', async () => {
|
||||
// No getAppliedSize (e.g. SSH relay): the requested-size cache is the only
|
||||
// signal, so getSize returns it — preserving prior behavior, not a regression.
|
||||
setupProviderWithAppliedSize({ applied: null, getAppliedSize: undefined })
|
||||
setLocalPtyProvider({
|
||||
spawn: vi.fn(async (opts: { sessionId?: string }) => ({
|
||||
id: opts.sessionId ?? 'daemon-pty'
|
||||
})),
|
||||
write: vi.fn(),
|
||||
resize: vi.fn(),
|
||||
kill: vi.fn(),
|
||||
shutdown: vi.fn(),
|
||||
onData: vi.fn(() => vi.fn()),
|
||||
onExit: vi.fn(() => vi.fn()),
|
||||
listProcesses: vi.fn(async () => []),
|
||||
getForegroundProcess: vi.fn(async () => null)
|
||||
} as never)
|
||||
handlers.clear()
|
||||
registerPtyHandlers(mainWindow as never)
|
||||
const spawn = await handlers.get('pty:spawn')!(null, { cols: 200, rows: 50, env: {} })
|
||||
const id = (spawn as { id: string }).id
|
||||
resizeListener()(mainWindowIpcEvent, { id, cols: 80, rows: 24 })
|
||||
|
||||
const reported = await handlers.get('pty:getSize')!(null, { id })
|
||||
expect(reported).toEqual({ cols: 80, rows: 24 })
|
||||
})
|
||||
|
||||
it('falls back to the requested size when getAppliedSize throws', async () => {
|
||||
// A dead daemon/relay must never throw across the IPC boundary or block.
|
||||
setupProviderWithAppliedSize({
|
||||
applied: null,
|
||||
getAppliedSize: async () => {
|
||||
throw new Error('daemon unreachable')
|
||||
}
|
||||
})
|
||||
handlers.clear()
|
||||
registerPtyHandlers(mainWindow as never)
|
||||
const spawn = await handlers.get('pty:spawn')!(null, { cols: 100, rows: 30, env: {} })
|
||||
const id = (spawn as { id: string }).id
|
||||
|
||||
const reported = await handlers.get('pty:getSize')!(null, { id })
|
||||
expect(reported).toEqual({ cols: 100, rows: 30 })
|
||||
})
|
||||
})
|
||||
|
||||
it('injects ORCA_TERMINAL_HANDLE for non-local PTY providers', async () => {
|
||||
const spawn = vi.fn(async () => ({ id: 'remote-pty' }))
|
||||
registerSshPtyProvider('ssh-1', {
|
||||
|
|
|
|||
|
|
@ -3241,15 +3241,29 @@ export function registerPtyHandlers(
|
|||
|
||||
// Why: the renderer forwards resizes fire-and-forget and otherwise has no way
|
||||
// to learn the PTY's actual size. A resize dropped main-side (suppression
|
||||
// window, mobile-driver gate, or a provider no-op) leaves the renderer
|
||||
// believing it synced when it did not, so a later same-cols layout never
|
||||
// re-forwards and the TUI stays garbled. Exposing the last APPLIED size lets
|
||||
// the renderer detect true drift on resume and re-assert. ptySizes is only
|
||||
// written when a resize actually lands (and at spawn), so it is the
|
||||
// authoritative "what the PTY believes it is".
|
||||
// window, mobile-driver gate, or a provider no-op) OR daemon/SSH-side (the
|
||||
// remote resize notify is unacked and can be silently dropped — session not
|
||||
// yet alive, exited, invalid dims, cold-restore snapshot-col coercion) leaves
|
||||
// the renderer believing it synced when it did not, so a later same-cols
|
||||
// layout never re-forwards and the TUI stays garbled. ptySizes records only
|
||||
// the REQUESTED size, so it cannot reveal such a drop. Prefer the provider's
|
||||
// APPLIED size (node-pty's cached winsize / the daemon emulator's dims, which
|
||||
// track the subprocess resize) so the renderer's resume drift-check sees the
|
||||
// truth; fall back to ptySizes only when the provider can't report (no
|
||||
// getAppliedSize, e.g. SSH relay, or an unknown id) — a null then reads as
|
||||
// "cannot confirm", which the renderer treats as a cue to re-forward once.
|
||||
ipcMain.handle(
|
||||
'pty:getSize',
|
||||
(_event, args: { id: string }): { cols: number; rows: number } | null => {
|
||||
async (_event, args: { id: string }): Promise<{ cols: number; rows: number } | null> => {
|
||||
try {
|
||||
const applied = await tryGetProviderForPty(args.id)?.getAppliedSize?.(args.id)
|
||||
if (applied) {
|
||||
return applied
|
||||
}
|
||||
} catch {
|
||||
// Fall through to the requested-size cache on any provider/RPC failure
|
||||
// so a dead daemon/relay never blocks or throws across the IPC boundary.
|
||||
}
|
||||
return ptySizes.get(args.id) ?? null
|
||||
}
|
||||
)
|
||||
|
|
|
|||
|
|
@ -817,6 +817,19 @@ export class LocalPtyProvider implements IPtyProvider {
|
|||
ptyProcesses.get(id)?.resize(cols, rows)
|
||||
}
|
||||
|
||||
// Why: node-pty caches the last winsize it applied on the IPty handle, so its
|
||||
// cols/rows are the authoritative applied size (node-pty clamps invalid dims
|
||||
// and a resize on a dead handle is a no-op, neither of which the requested
|
||||
// size in ptySizes would reflect). The renderer's resume drift-check compares
|
||||
// against this to re-assert a resize the PTY never actually took.
|
||||
async getAppliedSize(id: string): Promise<{ cols: number; rows: number } | null> {
|
||||
const proc = ptyProcesses.get(id)
|
||||
if (!proc || proc.cols <= 0 || proc.rows <= 0) {
|
||||
return null
|
||||
}
|
||||
return { cols: proc.cols, rows: proc.rows }
|
||||
}
|
||||
|
||||
async shutdown(id: string, _opts: { immediate?: boolean; keepHistory?: boolean }): Promise<void> {
|
||||
const proc = ptyProcesses.get(id)
|
||||
if (!proc) {
|
||||
|
|
|
|||
|
|
@ -105,6 +105,19 @@ export type IPtyProvider = {
|
|||
hasPty?: (id: string) => boolean
|
||||
write(id: string, data: string): void
|
||||
resize(id: string, cols: number, rows: number): void
|
||||
/**
|
||||
* The size the PTY has ACTUALLY applied, not the last size requested.
|
||||
* resize() is fire-and-forget for remote providers (daemon/SSH `notify`),
|
||||
* so a resize can be silently dropped (session not yet alive, dead handle,
|
||||
* cold-restore snapshot-cols coercion) while the caller still believes it
|
||||
* landed. This is the readback the renderer's resume drift-check compares
|
||||
* against to detect — and re-assert past — such drops. Returns null when the
|
||||
* provider cannot confirm the applied size (unknown id, relay unreachable);
|
||||
* callers treat null as "cannot confirm" and re-forward once. Optional so
|
||||
* providers without an authoritative size source can omit it.
|
||||
*/
|
||||
getAppliedSize?: (id: string) => Promise<{ cols: number; rows: number } | null>
|
||||
|
||||
shutdown(id: string, opts: { immediate?: boolean; keepHistory?: boolean }): Promise<void>
|
||||
sendSignal(id: string, signal: string): Promise<void>
|
||||
getCwd(id: string): Promise<string>
|
||||
|
|
|
|||
|
|
@ -2283,6 +2283,13 @@ export function connectPanePty(
|
|||
return cols > 0 && rows > 0 ? { cols, rows } : null
|
||||
},
|
||||
resize: (cols, rows) => transport.resize(cols, rows),
|
||||
// Why: confirm the PTY actually applied the size we forwarded before the
|
||||
// reconcile hands off. transport.resize is fire-and-forget for daemon/SSH
|
||||
// PTYs, so the loop can otherwise settle on a size the PTY dropped, leaving
|
||||
// it pinned wide while xterm shows narrow — the mount-time desync. Skip
|
||||
// remote-runtime PTYs (separate viewport channel; pty:getSize never tracks
|
||||
// them) so they fall back to the grid-stable handoff.
|
||||
getAppliedSize: isRemoteRuntimePtyId(ptyId) ? undefined : () => window.api.pty.getSize(ptyId),
|
||||
requestFrame: (callback) => requestAnimationFrame(callback),
|
||||
cancelFrame: (handle) => {
|
||||
if (typeof cancelAnimationFrame === 'function') {
|
||||
|
|
|
|||
|
|
@ -312,4 +312,127 @@ describe('reconcilePtySizeAcrossFrames', () => {
|
|||
expect(resize.mock.calls.length).toBe(callsBefore)
|
||||
expect(scheduler.pending()).toBe(0)
|
||||
})
|
||||
|
||||
// Why: the grid being stable only proves what the loop SENT held steady, not
|
||||
// what the PTY APPLIED. transport.resize is fire-and-forget for daemon/SSH
|
||||
// PTYs, so the loop can settle on a size the PTY dropped — the mount-time twin
|
||||
// of the resume drift. getAppliedSize lets the loop confirm before handing off.
|
||||
describe('applied-size verification before handoff', () => {
|
||||
/** Drain frames, flushing microtasks between each so async getAppliedSize
|
||||
* promises resolve and influence the next frame (mirrors real rAF timing). */
|
||||
async function runAsync(
|
||||
scheduler: ReturnType<typeof createFrameScheduler>,
|
||||
maxFrames = 1000
|
||||
): Promise<void> {
|
||||
let ran = 0
|
||||
while (scheduler.pending() > 0 && ran < maxFrames) {
|
||||
scheduler.run(1)
|
||||
ran += 1
|
||||
// Let any getAppliedSize().then(...) settle before the next frame.
|
||||
await Promise.resolve()
|
||||
await Promise.resolve()
|
||||
}
|
||||
}
|
||||
|
||||
it('keeps converging when the PTY drops the resize (applied stays wide)', async () => {
|
||||
const scheduler = createFrameScheduler()
|
||||
const resize = vi.fn()
|
||||
// xterm settles narrow immediately, but the PTY never applies it: every
|
||||
// applied-size read reports the stale wide spawn width.
|
||||
const pane = createTimelinePane(() => ({ cols: 79, rows: 50 }))
|
||||
reconcilePtySizeAcrossFrames({
|
||||
spawnCols: 203,
|
||||
spawnRows: 50,
|
||||
isAlive: () => true,
|
||||
isParked: () => false,
|
||||
isAuthoritative: () => true,
|
||||
measure: pane.measure,
|
||||
resize,
|
||||
getAppliedSize: async () => ({ cols: 203, rows: 50 }),
|
||||
requestFrame: scheduler.requestFrame,
|
||||
cancelFrame: scheduler.cancelFrame
|
||||
})
|
||||
await runAsync(scheduler, 400)
|
||||
|
||||
// The loop must have re-forwarded the narrow size more than once (the
|
||||
// initial settle plus at least one verify-driven re-forward) and only
|
||||
// terminated at the hard cap, never falsely handing off on a dropped size.
|
||||
const narrowForwards = resize.mock.calls.filter((c) => c[0] === 79 && c[1] === 50)
|
||||
expect(narrowForwards.length).toBeGreaterThan(1)
|
||||
})
|
||||
|
||||
it('hands off once the applied size matches the forwarded grid', async () => {
|
||||
const scheduler = createFrameScheduler()
|
||||
const resize = vi.fn()
|
||||
const pane = createTimelinePane(() => ({ cols: 79, rows: 50 }))
|
||||
let applied = { cols: 203, rows: 50 }
|
||||
// The PTY applies the narrow size after the first corrective forward.
|
||||
reconcilePtySizeAcrossFrames({
|
||||
spawnCols: 203,
|
||||
spawnRows: 50,
|
||||
isAlive: () => true,
|
||||
isParked: () => false,
|
||||
isAuthoritative: () => true,
|
||||
measure: pane.measure,
|
||||
resize: vi.fn((cols, rows) => {
|
||||
resize(cols, rows)
|
||||
applied = { cols, rows }
|
||||
}),
|
||||
getAppliedSize: async () => applied,
|
||||
requestFrame: scheduler.requestFrame,
|
||||
cancelFrame: scheduler.cancelFrame
|
||||
})
|
||||
await runAsync(scheduler, 400)
|
||||
|
||||
// It converges and then STOPS (no pending frames) well before the hard cap.
|
||||
expect(resize).toHaveBeenLastCalledWith(79, 50)
|
||||
expect(scheduler.pending()).toBe(0)
|
||||
})
|
||||
|
||||
it('hands off when applied size cannot be confirmed (null read)', async () => {
|
||||
const scheduler = createFrameScheduler()
|
||||
const resize = vi.fn()
|
||||
const pane = createTimelinePane(() => ({ cols: 79, rows: 50 }))
|
||||
reconcilePtySizeAcrossFrames({
|
||||
spawnCols: 203,
|
||||
spawnRows: 50,
|
||||
isAlive: () => true,
|
||||
isParked: () => false,
|
||||
isAuthoritative: () => true,
|
||||
measure: pane.measure,
|
||||
resize,
|
||||
// A provider that cannot confirm applied size must not wedge the loop.
|
||||
getAppliedSize: async () => null,
|
||||
requestFrame: scheduler.requestFrame,
|
||||
cancelFrame: scheduler.cancelFrame
|
||||
})
|
||||
await runAsync(scheduler, 400)
|
||||
expect(scheduler.pending()).toBe(0)
|
||||
})
|
||||
|
||||
it('does not verify or re-forward while parked — mobile drives at phone dims', async () => {
|
||||
const scheduler = createFrameScheduler()
|
||||
const resize = vi.fn()
|
||||
// A mobile-driven PTY legitimately sits at phone dims (≠ our desktop grid).
|
||||
// The parked gate must suppress the verify entirely so we never spin
|
||||
// re-forwarding a desktop size the mobile gate would drop.
|
||||
const getAppliedSize = vi.fn(async () => ({ cols: 40, rows: 30 }))
|
||||
const pane = createTimelinePane(() => ({ cols: 120, rows: 40 }))
|
||||
reconcilePtySizeAcrossFrames({
|
||||
spawnCols: 120,
|
||||
spawnRows: 40,
|
||||
isAlive: () => true,
|
||||
isParked: () => true,
|
||||
isAuthoritative: () => true,
|
||||
measure: pane.measure,
|
||||
resize,
|
||||
getAppliedSize,
|
||||
requestFrame: scheduler.requestFrame,
|
||||
cancelFrame: scheduler.cancelFrame
|
||||
})
|
||||
await runAsync(scheduler, 400)
|
||||
expect(getAppliedSize).not.toHaveBeenCalled()
|
||||
expect(resize).not.toHaveBeenCalled()
|
||||
})
|
||||
})
|
||||
})
|
||||
|
|
|
|||
|
|
@ -55,6 +55,18 @@ export type PtySizeReconcileOptions = {
|
|||
measure: () => PtySizeReconcileDimensions | null
|
||||
/** Forward the settled size to the PTY (authoritative — bypasses visibility). */
|
||||
resize: (cols: number, rows: number) => void
|
||||
/**
|
||||
* Read the size the PTY has ACTUALLY applied (vs what this loop last sent).
|
||||
* Optional. resize() is fire-and-forget for remote (daemon/SSH) PTYs, so the
|
||||
* loop can settle on a size it sent that the PTY silently dropped — the
|
||||
* mount-time twin of the resume drift the visibility-resume re-assert heals.
|
||||
* Before handing off, the loop reads this once; if it diverges from the
|
||||
* last-sent grid it re-forwards and keeps converging. Returns null when the
|
||||
* applied size cannot be confirmed (treated as "synced enough to hand off" so
|
||||
* a provider without a readback, or a transient failure, cannot wedge the
|
||||
* loop until MAX_FRAMES).
|
||||
*/
|
||||
getAppliedSize?: () => Promise<PtySizeReconcileDimensions | null>
|
||||
/** Schedule the next frame; mirrors requestAnimationFrame's id contract. */
|
||||
requestFrame: (callback: () => void) => number
|
||||
cancelFrame: (handle: number) => void
|
||||
|
|
@ -92,6 +104,12 @@ export function reconcilePtySizeAcrossFrames(
|
|||
let lastSentRows = options.spawnRows
|
||||
let pendingFrame: number | null = null
|
||||
let cancelled = false
|
||||
// One-shot applied-size verification before handoff. `verifyInFlight` prevents
|
||||
// re-issuing the async read every frame while it resolves; `appliedVerified`
|
||||
// is the terminal "the PTY confirmed our size (or can't be read)" flag that
|
||||
// lets the loop stop. A re-forward on divergence clears it so we re-verify.
|
||||
let verifyInFlight = false
|
||||
let appliedVerified = options.getAppliedSize === undefined
|
||||
|
||||
const tick = (): void => {
|
||||
pendingFrame = null
|
||||
|
|
@ -110,6 +128,7 @@ export function reconcilePtySizeAcrossFrames(
|
|||
lastSentCols = measured.cols
|
||||
lastSentRows = measured.rows
|
||||
authoritativeStableFrames = 0
|
||||
appliedVerified = options.getAppliedSize === undefined
|
||||
} else if (options.isAuthoritative()) {
|
||||
// Only stability seen *under authority* counts toward handoff — a grid
|
||||
// that merely held steady while hidden is not a safe stopping point.
|
||||
|
|
@ -124,7 +143,47 @@ export function reconcilePtySizeAcrossFrames(
|
|||
// layout settle. Once the pane has been visible AND its grid has held steady
|
||||
// for the settle window, the live onResize/ResizeObserver path owns any
|
||||
// further change, so we hand off. The hard cap guarantees termination.
|
||||
const settled = authoritativeStableFrames >= POST_SPAWN_RECONCILE_SETTLE_FRAMES
|
||||
const gridStable = authoritativeStableFrames >= POST_SPAWN_RECONCILE_SETTLE_FRAMES
|
||||
// Why verify before handoff: the grid being stable only proves what we SENT
|
||||
// held steady, not what the PTY APPLIED. For a fire-and-forget remote resize
|
||||
// the PTY can be pinned at a stale (wide) size while xterm reflowed narrow —
|
||||
// the mount-time form of the bug that garbles alt-screen TUIs. Read the
|
||||
// applied size once; re-forward and keep converging on divergence. Skip while
|
||||
// parked: a mobile-driven PTY legitimately sits at phone dims (≠ our desktop
|
||||
// grid), and verifying there would spin the loop re-forwarding a size the
|
||||
// mobile gate correctly drops — the same reason measure/forward skip parked.
|
||||
if (
|
||||
gridStable &&
|
||||
!appliedVerified &&
|
||||
!verifyInFlight &&
|
||||
!options.isParked() &&
|
||||
options.getAppliedSize
|
||||
) {
|
||||
verifyInFlight = true
|
||||
void options
|
||||
.getAppliedSize()
|
||||
.then((applied) => {
|
||||
if (cancelled || !options.isAlive()) {
|
||||
return
|
||||
}
|
||||
if (applied && (applied.cols !== lastSentCols || applied.rows !== lastSentRows)) {
|
||||
// The PTY never took our size — re-forward and keep the loop running.
|
||||
options.resize(lastSentCols, lastSentRows)
|
||||
authoritativeStableFrames = 0
|
||||
} else {
|
||||
// Applied matches, or cannot be confirmed (null) — safe to hand off.
|
||||
appliedVerified = true
|
||||
}
|
||||
})
|
||||
.catch(() => {
|
||||
// A failed read must not wedge the loop until MAX_FRAMES.
|
||||
appliedVerified = true
|
||||
})
|
||||
.finally(() => {
|
||||
verifyInFlight = false
|
||||
})
|
||||
}
|
||||
const settled = gridStable && appliedVerified
|
||||
if (!settled && frame < POST_SPAWN_RECONCILE_MAX_FRAMES) {
|
||||
pendingFrame = options.requestFrame(tick)
|
||||
}
|
||||
|
|
|
|||
|
|
@ -87,6 +87,16 @@ async function readRenderedColsForPty(page: Page, ptyId: string): Promise<number
|
|||
}, ptyId)
|
||||
}
|
||||
|
||||
/** Read the size the main process reports as APPLIED for a PTY (pty:getSize).
|
||||
* After the applied-size fix this must reflect what the PTY actually took
|
||||
* (process.stdout.columns), not the renderer's last-requested size. */
|
||||
async function readReportedPtyCols(page: Page, ptyId: string): Promise<number> {
|
||||
return page.evaluate(async (ptyId) => {
|
||||
const size = await window.api?.pty?.getSize?.(ptyId)
|
||||
return size?.cols ?? 0
|
||||
}, ptyId)
|
||||
}
|
||||
|
||||
type ColumnSnapshot = { xtermCols: number; ptyCols: number }
|
||||
|
||||
async function readColumnSnapshot(page: Page, ptyId: string): Promise<ColumnSnapshot> {
|
||||
|
|
@ -150,6 +160,32 @@ test.describe('Terminal column desync repro', () => {
|
|||
).toBe(wide.xtermCols)
|
||||
})
|
||||
|
||||
// Why: guards the applied-size IPC contract the desync fix relies on. The
|
||||
// renderer's resume/handoff drift-check compares xterm against pty:getSize; if
|
||||
// pty:getSize reports the renderer's last-REQUESTED size (the old intent-only
|
||||
// behavior) instead of the size the PTY actually APPLIED, a dropped resize is
|
||||
// invisible and the TUI stays garbled. So pty:getSize must equal the real
|
||||
// in-PTY process.stdout.columns, not just xterm.
|
||||
test('pty:getSize reports the size the PTY actually applied', async ({ orcaPage }) => {
|
||||
test.setTimeout(120_000)
|
||||
await waitForSessionReady(orcaPage)
|
||||
await waitForActiveWorktree(orcaPage)
|
||||
await closeRightSidebarAndFeatureTips(orcaPage)
|
||||
await ensureTerminalVisible(orcaPage)
|
||||
const ptyId = await settleTerminal(orcaPage)
|
||||
|
||||
await orcaPage.setViewportSize({ width: 900, height: 800 })
|
||||
await orcaPage.waitForTimeout(500)
|
||||
|
||||
const ptyCols = await readPtyCols(orcaPage, ptyId)
|
||||
const reportedCols = await readReportedPtyCols(orcaPage, ptyId)
|
||||
expect(
|
||||
reportedCols,
|
||||
`pty:getSize reported ${reportedCols} but the PTY's process.stdout.columns is ${ptyCols}; ` +
|
||||
`getSize must reflect the APPLIED size so the drift-check can detect a dropped resize`
|
||||
).toBe(ptyCols)
|
||||
})
|
||||
|
||||
test('PTY columns re-sync after the terminal is resized while hidden', async ({ orcaPage }) => {
|
||||
test.setTimeout(120_000)
|
||||
await waitForSessionReady(orcaPage)
|
||||
|
|
|
|||
Loading…
Reference in New Issue