mirror of
https://github.com/stablyai/orca.git
synced 2026-10-31 17:17:38 +00:00
fix(remote-terminal): a pane-restore snapshot moves the gap baseline so reveal skips a resync repaint (#29285)
Refs #22087
This commit is contained in:
@@ -8,6 +8,7 @@
|
||||
* can't be reached, the pane stays bound and reattaches once it can: loss of contact is never exit.
|
||||
*/
|
||||
import type { PtyConnectResult, PtyTransport } from './pty-transport-types'
|
||||
import type { RemoteRuntimeSnapshotRequestOptions } from '../../runtime/remote-runtime-terminal-multiplexer-types'
|
||||
import { SSH_SESSION_EXPIRED_ERROR } from './pty-connection/pty-connect-limits'
|
||||
import {
|
||||
legacyRelayBridgedPane,
|
||||
@@ -173,7 +174,7 @@ export function createLegacyRelayBridgeTransport(ptyId: string, inner: PtyTransp
|
||||
: {}),
|
||||
...(innerSnapshotOutcome
|
||||
? {
|
||||
serializeBufferOutcome: (opts?: { scrollbackRows?: number }) =>
|
||||
serializeBufferOutcome: (opts?: RemoteRuntimeSnapshotRequestOptions) =>
|
||||
bridged()
|
||||
? Promise.resolve({
|
||||
availability: { kind: 'unknown-legacy-host' as const },
|
||||
|
||||
+4
-1
@@ -202,7 +202,10 @@ describe('connectPanePty', () => {
|
||||
await flushAsyncTicks(20)
|
||||
|
||||
expect(getMainBufferSnapshot).not.toHaveBeenCalled()
|
||||
expect(transport.serializeBuffer).toHaveBeenCalledWith({ scrollbackRows: 5000 })
|
||||
expect(transport.serializeBuffer).toHaveBeenCalledWith({
|
||||
scrollbackRows: 5000,
|
||||
repaintsPane: true
|
||||
})
|
||||
expect(pane.terminal.write).not.toHaveBeenCalledWith(hidden)
|
||||
expect(pane.terminal.write).toHaveBeenCalledWith(
|
||||
expect.stringContaining('remote snapshot with hidden remote output'),
|
||||
|
||||
+8
-2
@@ -488,7 +488,10 @@ export function registerPtyConnectionHiddenOutputRestoreTests(): void {
|
||||
await flushAsyncTicks(20)
|
||||
|
||||
expect(transport.setOutputPaused).toHaveBeenLastCalledWith(false)
|
||||
expect(transport.serializeBuffer).toHaveBeenCalledWith({ scrollbackRows: 5000 })
|
||||
expect(transport.serializeBuffer).toHaveBeenCalledWith({
|
||||
scrollbackRows: 5000,
|
||||
repaintsPane: true
|
||||
})
|
||||
expect(transport.setOutputPaused.mock.invocationCallOrder.at(-1)).toBeLessThan(
|
||||
transport.serializeBuffer.mock.invocationCallOrder[0]
|
||||
)
|
||||
@@ -564,7 +567,10 @@ export function registerPtyConnectionHiddenOutputRestoreTests(): void {
|
||||
await flushAsyncTicks(20)
|
||||
|
||||
expect(transport.setOutputPaused).toHaveBeenLastCalledWith(false)
|
||||
expect(transport.serializeBuffer).toHaveBeenCalledWith({ scrollbackRows: 5000 })
|
||||
expect(transport.serializeBuffer).toHaveBeenCalledWith({
|
||||
scrollbackRows: 5000,
|
||||
repaintsPane: true
|
||||
})
|
||||
callbacks.onData?.(live, {
|
||||
seq: hidden.length + live.length,
|
||||
rawLength: live.length
|
||||
|
||||
+4
-1
@@ -95,7 +95,10 @@ export function registerPtyConnectionParkRevealModelSnapshotTests(): void {
|
||||
await flushAsyncTicks(2)
|
||||
}
|
||||
|
||||
expect(transport.serializeBuffer).toHaveBeenCalledWith({ scrollbackRows: 5000 })
|
||||
expect(transport.serializeBuffer).toHaveBeenCalledWith({
|
||||
scrollbackRows: 5000,
|
||||
repaintsPane: true
|
||||
})
|
||||
expect(transport.attach).not.toHaveBeenCalled()
|
||||
expect(writes.join('')).toContain('DEEP_PAIRED_SCROLLBACK')
|
||||
expect(writes.join('')).toContain('current screen from initial subscribe')
|
||||
|
||||
+6
-2
@@ -51,11 +51,15 @@ export function bindSerializeHiddenOutputSnapshot(session: ConnectPanePtySession
|
||||
session.hiddenOutputRestoreLegacyPtyId === ptyId ||
|
||||
typeof session.transport.serializeBufferOutcome !== 'function'
|
||||
) {
|
||||
const snapshot = await session.transport.serializeBuffer(opts)
|
||||
const snapshot = await session.transport.serializeBuffer({ ...opts, repaintsPane: true })
|
||||
return snapshot ? { kind: 'snapshot', snapshot } : { kind: 'unknown-legacy-host' }
|
||||
}
|
||||
try {
|
||||
const outcome = await session.transport.serializeBufferOutcome(opts)
|
||||
// Why repaintsPane: every caller paints this image, so live output resumes after its seq.
|
||||
const outcome = await session.transport.serializeBufferOutcome({
|
||||
...opts,
|
||||
repaintsPane: true
|
||||
})
|
||||
if (outcome.availability.kind === 'snapshot') {
|
||||
// A success frame with no image is still the host's own answer to a request it received.
|
||||
return outcome.snapshot
|
||||
|
||||
@@ -20,7 +20,10 @@ import type { TuiAgent } from '../../../../shared/tui-agent'
|
||||
import type { TerminalPanePlacement } from '../../../../shared/terminal-pane-placement'
|
||||
import type { ExecutionHostId } from '../../../../shared/execution-host'
|
||||
import type { PtyDataMeta } from './pty-dispatcher'
|
||||
import type { RemoteRuntimeSnapshotOutcome } from '../../runtime/remote-runtime-terminal-multiplexer-types'
|
||||
import type {
|
||||
RemoteRuntimeSnapshotOutcome,
|
||||
RemoteRuntimeSnapshotRequestOptions
|
||||
} from '../../runtime/remote-runtime-terminal-multiplexer-types'
|
||||
import type { AcceptedInputOptions, PtyPreconnectInputEntry } from './pty-preconnect-input-buffer'
|
||||
import type { AgentPaneLaunchRequest } from '../../../../shared/agent-pane-launch-request'
|
||||
|
||||
@@ -254,10 +257,12 @@ export type PtyTransport = {
|
||||
* model-restore marker reports dropped bytes — a carry spanning the gap
|
||||
* would corrupt the next live chunk. IPC transports only. */
|
||||
resetCrossChunkParserState?: () => void
|
||||
serializeBuffer?: (opts?: { scrollbackRows?: number }) => Promise<PtyBufferSnapshot | null>
|
||||
serializeBufferOutcome?: (opts?: {
|
||||
scrollbackRows?: number
|
||||
}) => Promise<RemoteRuntimeSnapshotOutcome>
|
||||
serializeBuffer?: (
|
||||
opts?: RemoteRuntimeSnapshotRequestOptions
|
||||
) => Promise<PtyBufferSnapshot | null>
|
||||
serializeBufferOutcome?: (
|
||||
opts?: RemoteRuntimeSnapshotRequestOptions
|
||||
) => Promise<RemoteRuntimeSnapshotOutcome>
|
||||
preserve?: () => void
|
||||
/** Hand the live PTY to a successor without process teardown. Terminal for this instance:
|
||||
* it also drops the transport's output processor from the pty side-effect memory census,
|
||||
|
||||
+4
@@ -591,6 +591,10 @@ describe('remote hidden-output restore outcomes', () => {
|
||||
await flushAsyncTicks(20)
|
||||
|
||||
expect(serializeBufferOutcome).toHaveBeenCalledTimes(1)
|
||||
// Why: the multiplexer only moves its gap baseline for images a pane repaints from.
|
||||
expect(serializeBufferOutcome).toHaveBeenCalledWith(
|
||||
expect.objectContaining({ repaintsPane: true })
|
||||
)
|
||||
expect(drive.writtenChunks().join('')).toContain(HOST_SNAPSHOT_MARKER)
|
||||
expect(serializeBuffer).not.toHaveBeenCalled()
|
||||
drive.disposable.dispose()
|
||||
|
||||
+2
-2
@@ -620,7 +620,7 @@ describe('remote hidden-output restore abandonment (issue2-hidden-output-skip)',
|
||||
|
||||
// Reveal triggered exactly one remote snapshot request against the owning runtime.
|
||||
expect(serializeBuffer).toHaveBeenCalledTimes(1)
|
||||
expect(serializeBuffer).toHaveBeenNthCalledWith(1, { scrollbackRows: 5000 })
|
||||
expect(serializeBuffer).toHaveBeenNthCalledWith(1, { scrollbackRows: 5000, repaintsPane: true })
|
||||
|
||||
await advanceThroughNullRetryBudget(drive)
|
||||
|
||||
@@ -628,7 +628,7 @@ describe('remote hidden-output restore abandonment (issue2-hidden-output-skip)',
|
||||
// 1 initial + 3 deferred retries (50ms each), then abandonment unblocks live
|
||||
// output — but for a remote pane it must stay quiet, not claim loss.
|
||||
expect(serializeBuffer).toHaveBeenCalledTimes(4)
|
||||
expect(serializeBuffer).toHaveBeenNthCalledWith(4, { scrollbackRows: 5000 })
|
||||
expect(serializeBuffer).toHaveBeenNthCalledWith(4, { scrollbackRows: 5000, repaintsPane: true })
|
||||
expect(written.some((data) => data.includes('LIVE_AGENT_CHUNK'))).toBe(true)
|
||||
expect(written.some((data) => data.includes(BANNER_FRAGMENT))).toBe(false)
|
||||
// Remote pane never consulted local main recovery — the banner's "main
|
||||
|
||||
@@ -98,8 +98,20 @@ export abstract class RemoteRuntimeTerminalBinarySnapshots extends RemoteRuntime
|
||||
carriesHistory: (info?.scrollbackRows ?? 0) > 0
|
||||
}
|
||||
if (matchesPendingRequest) {
|
||||
const availability = classifySnapshotAvailability(stream.snapshotOverflowed, info)
|
||||
if (
|
||||
pendingRequest.repaintsPane &&
|
||||
availability.kind === 'snapshot' &&
|
||||
typeof info?.seq === 'number' &&
|
||||
typeof stream.expectedSeq === 'number' &&
|
||||
info.seq > stream.expectedSeq
|
||||
) {
|
||||
// Why: the pane repaints through info.seq (output paused while hidden was dropped);
|
||||
// a stale baseline would flag the next chunk as a gap and clear the pane (#22087).
|
||||
stream.expectedSeq = info.seq
|
||||
}
|
||||
pendingRequest.resolve({
|
||||
availability: classifySnapshotAvailability(stream.snapshotOverflowed, info),
|
||||
availability,
|
||||
snapshot: {
|
||||
data: data ?? '',
|
||||
cols: info?.cols ?? 80,
|
||||
|
||||
@@ -285,6 +285,48 @@ describe('remote terminal frame-drop resync', () => {
|
||||
expect(data).toEqual(['aaa', 'ddd'])
|
||||
})
|
||||
|
||||
// Why: an old host drops output while a hidden pane's stream is paused; the reveal's
|
||||
// own requested image already covers that gap, so a resync would clear and repaint
|
||||
// the TUI one round trip later (#22087).
|
||||
it('takes one requested image on reveal and no untagged resync after paused output', async () => {
|
||||
const { data, snapshots, stream } = await subscribeClient()
|
||||
server.output('aaa')
|
||||
server.dropNextOutput = true
|
||||
server.output('dropped while paused')
|
||||
|
||||
const revealImage = await stream.serializeBufferOutcome({
|
||||
scrollbackRows: 100,
|
||||
repaintsPane: true
|
||||
})
|
||||
expect(revealImage.snapshot?.data).toBe('RECOVERED')
|
||||
|
||||
server.output('live after reveal')
|
||||
await Promise.resolve()
|
||||
await Promise.resolve()
|
||||
|
||||
expect(server.snapshotRequests).toEqual([expect.any(Number)])
|
||||
expect(snapshots).toEqual(['INITIAL'])
|
||||
expect(data).toEqual(['aaa', 'live after reveal'])
|
||||
})
|
||||
|
||||
// Why: stall probes and previews read the host without repainting the pane, so a gap
|
||||
// they span is still real and must resync.
|
||||
it('still resyncs a gap that a non-repainting request spanned', async () => {
|
||||
const { data, snapshots, stream } = await subscribeClient()
|
||||
server.output('aaa')
|
||||
server.dropNextOutput = true
|
||||
server.output('dropped')
|
||||
|
||||
await stream.serializeBufferOutcome({ scrollbackRows: 100 })
|
||||
server.output('ccc')
|
||||
await Promise.resolve()
|
||||
await Promise.resolve()
|
||||
|
||||
expect(server.snapshotRequests).toEqual([expect.any(Number), undefined])
|
||||
expect(snapshots).toEqual(['INITIAL', '\x1b[?2026l\x1b[2J\x1b[3J\x1b[HRECOVERED'])
|
||||
expect(data).toEqual(['aaa'])
|
||||
})
|
||||
|
||||
it('retries a truncated recovery on a backoff while keeping live output flowing', async () => {
|
||||
vi.useFakeTimers()
|
||||
try {
|
||||
|
||||
@@ -157,13 +157,13 @@ export type RemoteRuntimeMultiplexedTerminal = {
|
||||
setOutputPaused: (paused: boolean) => boolean
|
||||
// Why: only then does a pause with no drop notice prove nothing was lost.
|
||||
reportsPausedOutputDrops?: () => boolean
|
||||
serializeBuffer: (opts?: {
|
||||
scrollbackRows?: number
|
||||
}) => Promise<RemoteRuntimeSnapshotImage | null>
|
||||
serializeBuffer: (
|
||||
opts?: RemoteRuntimeSnapshotRequestOptions
|
||||
) => Promise<RemoteRuntimeSnapshotImage | null>
|
||||
// Why: same request as serializeBuffer, but keeps the host's reason for an absent buffer instead of collapsing it to null.
|
||||
serializeBufferOutcome: (opts?: {
|
||||
scrollbackRows?: number
|
||||
}) => Promise<RemoteRuntimeSnapshotOutcome>
|
||||
serializeBufferOutcome: (
|
||||
opts?: RemoteRuntimeSnapshotRequestOptions
|
||||
) => Promise<RemoteRuntimeSnapshotOutcome>
|
||||
close: () => void
|
||||
}
|
||||
|
||||
@@ -230,8 +230,15 @@ export type RemoteRuntimeSnapshotInfo = {
|
||||
pendingEscapeTailAnsi?: string
|
||||
}
|
||||
|
||||
export type RemoteRuntimeSnapshotRequestOptions = {
|
||||
scrollbackRows?: number
|
||||
/** Client-only: the pane repaints from this image, so live output resumes after its seq. */
|
||||
repaintsPane?: boolean
|
||||
}
|
||||
|
||||
export type RemoteRuntimeSnapshotRequest = {
|
||||
requestId: number
|
||||
repaintsPane?: boolean
|
||||
resolve: (outcome: RemoteRuntimeSnapshotOutcome) => void
|
||||
reject: (error: Error) => void
|
||||
timer: ReturnType<typeof setTimeout>
|
||||
|
||||
@@ -63,7 +63,16 @@ describe('remote terminal paused-output drop notice', () => {
|
||||
send(TerminalStreamOpcode.SnapshotStart, encodeTerminalStreamJson({ seq: cursor }))
|
||||
send(TerminalStreamOpcode.SnapshotEnd, new Uint8Array())
|
||||
} else if (frame?.opcode === TerminalStreamOpcode.SnapshotRequest) {
|
||||
snapshotRequests.push(decodeTerminalStreamJson(frame.payload) ?? {})
|
||||
const request = decodeTerminalStreamJson<{ requestId?: number }>(frame.payload) ?? {}
|
||||
snapshotRequests.push(request)
|
||||
// Answer only a pane's tagged request; resyncs stay unanswered so tests count them.
|
||||
if (typeof request.requestId === 'number') {
|
||||
send(
|
||||
TerminalStreamOpcode.SnapshotStart,
|
||||
encodeTerminalStreamJson({ requestId: request.requestId, seq: cursor })
|
||||
)
|
||||
send(TerminalStreamOpcode.SnapshotEnd, new Uint8Array())
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -122,6 +131,32 @@ describe('remote terminal paused-output drop notice', () => {
|
||||
expect(snapshotRequests[0]?.requestId).toBeUndefined()
|
||||
})
|
||||
|
||||
// Why: with both the drop notice and the pane's repaint-baseline advance, a reveal
|
||||
// costs one tagged snapshot and the stream never falls back to a resync.
|
||||
it('composes with the reveal snapshot: one tagged request, no resync', async () => {
|
||||
for (const announce of [true, false]) {
|
||||
hostCapabilities = announce
|
||||
? { outputPause: 1, outputPauseDropNotice: 1 }
|
||||
: { outputPause: 1 }
|
||||
snapshotRequests = []
|
||||
const { data, stream } = await subscribeClient()
|
||||
output('before')
|
||||
stream.setOutputPaused(true)
|
||||
cursor += 'dropped-while-hidden'.length
|
||||
if (announce) {
|
||||
send(TerminalStreamOpcode.PausedOutputDropped, new Uint8Array())
|
||||
}
|
||||
stream.setOutputPaused(false)
|
||||
await stream.serializeBufferOutcome({ repaintsPane: true })
|
||||
output('live')
|
||||
|
||||
expect(snapshotRequests).toEqual([{ requestId: expect.any(Number) }])
|
||||
expect(data).toEqual(['before', 'live'])
|
||||
stream.close()
|
||||
resetRemoteRuntimeTerminalMultiplexersForTests()
|
||||
}
|
||||
})
|
||||
|
||||
it('keeps the resync for an older host that does not negotiate drop notices', async () => {
|
||||
hostCapabilities = { outputPause: 1 }
|
||||
const { data, onPausedOutputDropped, stream } = await subscribeClient()
|
||||
|
||||
@@ -19,7 +19,8 @@ import {
|
||||
import type {
|
||||
RemoteRuntimeMultiplexedTerminalState,
|
||||
RemoteRuntimeSnapshotImage,
|
||||
RemoteRuntimeSnapshotOutcome
|
||||
RemoteRuntimeSnapshotOutcome,
|
||||
RemoteRuntimeSnapshotRequestOptions
|
||||
} from './remote-runtime-terminal-multiplexer-types'
|
||||
|
||||
export abstract class RemoteRuntimeTerminalSnapshotController extends RemoteRuntimeTerminalMultiplexerBase {
|
||||
@@ -151,7 +152,7 @@ export abstract class RemoteRuntimeTerminalSnapshotController extends RemoteRunt
|
||||
|
||||
protected async requestSnapshot(
|
||||
stream: RemoteRuntimeMultiplexedTerminalState,
|
||||
opts?: { scrollbackRows?: number }
|
||||
opts?: RemoteRuntimeSnapshotRequestOptions
|
||||
): Promise<RemoteRuntimeSnapshotImage | null> {
|
||||
const outcome = await this.requestSnapshotOutcome(stream, opts)
|
||||
// Why: the concurrent-request guard used to reject before the outcome existed; keep that contract for legacy callers.
|
||||
@@ -166,7 +167,7 @@ export abstract class RemoteRuntimeTerminalSnapshotController extends RemoteRunt
|
||||
|
||||
protected requestSnapshotOutcome(
|
||||
stream: RemoteRuntimeMultiplexedTerminalState,
|
||||
opts?: { scrollbackRows?: number }
|
||||
opts?: RemoteRuntimeSnapshotRequestOptions
|
||||
): Promise<RemoteRuntimeSnapshotOutcome> {
|
||||
if (this.streams.get(stream.streamId) !== stream) {
|
||||
return Promise.resolve(retryWorthySnapshotOutcome('stream-detached'))
|
||||
@@ -194,7 +195,13 @@ export abstract class RemoteRuntimeTerminalSnapshotController extends RemoteRunt
|
||||
if (typeof timer.unref === 'function') {
|
||||
timer.unref()
|
||||
}
|
||||
stream.pendingSnapshotRequest = { requestId, resolve, reject, timer }
|
||||
stream.pendingSnapshotRequest = {
|
||||
requestId,
|
||||
resolve,
|
||||
reject,
|
||||
timer,
|
||||
...(opts?.repaintsPane ? { repaintsPane: true } : {})
|
||||
}
|
||||
if (
|
||||
!this.sendFrame(
|
||||
stream.streamId,
|
||||
|
||||
Reference in New Issue
Block a user