mirror of
https://github.com/stablyai/orca.git
synced 2026-09-22 08:02:28 +00:00
fix(terminal): repaint a recovered remote pane from the host's retained buffer (#14095)
* fix(terminal): repaint a recovered remote pane from the host's retained buffer A remote-runtime pane that loses its stream re-subscribes, but the recovery subscribe only carries new bytes. When the host's push snapshot is empty -- an idle pane, or one whose PTY has exited and is preserved with its buffer -- the transport dropped it and nothing re-armed the restore, so the pane stayed blank until a visibility flip issued the tagged snapshot request. That is why switching worktrees once "fixed" it. Emit onStreamRecovered from the recovery subscribe path only, and mark the hidden-output restore needed so the pane pulls the buffer the host still holds. The initial subscribe is untouched: it already carries the host snapshot, and re-arming there would cost every pane a redundant restore request on open. * perf(terminal): avoid duplicate recovery snapshot replay
This commit is contained in:
@@ -5828,6 +5828,11 @@ export function connectPanePty(
|
||||
reportRemoteRendererSerializerReady()
|
||||
}
|
||||
},
|
||||
onStreamRecovered: (): void => {
|
||||
if (isCurrent()) {
|
||||
markHiddenOutputRestoreNeeded()
|
||||
}
|
||||
},
|
||||
onData: (data: string, meta?: PtyDataMeta): void => {
|
||||
if (isCurrent()) {
|
||||
dataCallback(data, meta, generation)
|
||||
|
||||
@@ -104,6 +104,9 @@ type PtyCallbacks = {
|
||||
/** Called before an adopted PTY can publish buffered/live bytes. */
|
||||
onReattachDetermined?: () => void
|
||||
onConnect?: () => void
|
||||
/** A stream re-established after loss carries only new bytes, so the pane must
|
||||
* re-pull the host's retained buffer or an idle/exited pane paints nothing. */
|
||||
onStreamRecovered?: () => void
|
||||
onDisconnect?: () => void
|
||||
onData?: (data: string, meta?: PtyDataMeta) => void
|
||||
onReplayData?: (data: string, meta?: PtyReplayDataMeta) => void
|
||||
|
||||
+23
@@ -32,6 +32,7 @@ function leafIdForPane(paneId: number): string {
|
||||
type ConnectCallbacks = {
|
||||
onReattachDetermined?: () => void
|
||||
onConnect?: () => void
|
||||
onStreamRecovered?: () => void
|
||||
onData?: (
|
||||
data: string,
|
||||
meta?: { seq?: number; rawLength?: number; background?: boolean; droppedOutput?: boolean }
|
||||
@@ -379,6 +380,7 @@ type RemotePaneDrive = {
|
||||
disposable: { dispose: () => void }
|
||||
deliver: (data: string, seq: number) => void
|
||||
setOutputPaused: (paused: boolean) => void
|
||||
recoverStream: () => void
|
||||
writtenChunks: () => string[]
|
||||
}
|
||||
|
||||
@@ -396,9 +398,11 @@ async function connectHiddenRemoteAgentPane(
|
||||
const capturedOutputPauseCallback: {
|
||||
current: ((paused: boolean, supported: boolean) => void) | null
|
||||
} = { current: null }
|
||||
const capturedStreamRecoveredCallback: { current: (() => void) | null } = { current: null }
|
||||
transport.connect.mockImplementation(async ({ callbacks }: { callbacks?: ConnectCallbacks }) => {
|
||||
capturedDataCallback.current = callbacks?.onData ?? null
|
||||
capturedOutputPauseCallback.current = callbacks?.onOutputPauseChanged ?? null
|
||||
capturedStreamRecoveredCallback.current = callbacks?.onStreamRecovered ?? null
|
||||
return REMOTE_PTY_ID
|
||||
})
|
||||
transportFactoryQueue.push(transport)
|
||||
@@ -415,6 +419,7 @@ async function connectHiddenRemoteAgentPane(
|
||||
disposable,
|
||||
deliver: (data, seq) => capturedDataCallback.current?.(data, { seq, rawLength: data.length }),
|
||||
setOutputPaused: (paused) => capturedOutputPauseCallback.current?.(paused, true),
|
||||
recoverStream: () => capturedStreamRecoveredCallback.current?.(),
|
||||
writtenChunks: () => pane.terminal.write.mock.calls.map(([data]) => String(data))
|
||||
}
|
||||
}
|
||||
@@ -618,6 +623,24 @@ describe('remote hidden-output restore outcomes', () => {
|
||||
resetAgentStartupDelayedDeliveryForTests()
|
||||
})
|
||||
|
||||
it('[modern] repaints a recovered visible pane from the retained host buffer', async () => {
|
||||
const serializeBuffer = vi.fn()
|
||||
const serializeBufferOutcome = vi.fn().mockResolvedValue({
|
||||
availability: { kind: 'snapshot' },
|
||||
snapshot: HOST_SNAPSHOT
|
||||
})
|
||||
const drive = await connectHiddenRemoteAgentPane(serializeBuffer, serializeBufferOutcome)
|
||||
;(drive.deps.isVisibleRef as { current: boolean }).current = true
|
||||
|
||||
drive.recoverStream()
|
||||
await flushAsyncTicks(20)
|
||||
|
||||
expect(serializeBufferOutcome).toHaveBeenCalledTimes(1)
|
||||
expect(drive.writtenChunks().join('')).toContain(HOST_SNAPSHOT_MARKER)
|
||||
expect(serializeBuffer).not.toHaveBeenCalled()
|
||||
drive.disposable.dispose()
|
||||
})
|
||||
|
||||
it('[modern] accepts an empty snapshot as successful recovery without a loss banner', async () => {
|
||||
const serializeBuffer = vi.fn()
|
||||
const serializeBufferOutcome = vi.fn().mockResolvedValue({
|
||||
|
||||
@@ -5054,6 +5054,91 @@ describe('createRemoteRuntimePtyTransport', () => {
|
||||
transport.destroy?.()
|
||||
})
|
||||
|
||||
it('re-arms the retained-buffer restore when a recovery subscribe replays no snapshot', async () => {
|
||||
const { createRemoteRuntimePtyTransport } = await import('./remote-runtime-pty-transport')
|
||||
const onReplayData = vi.fn()
|
||||
const onStreamRecovered = vi.fn()
|
||||
const onConnect = vi.fn()
|
||||
const transport = createRemoteRuntimePtyTransport('env-1', {
|
||||
worktreeId: 'wt-1',
|
||||
tabId: 'tab-1',
|
||||
leafId: 'pane:1'
|
||||
})
|
||||
|
||||
await transport.connect({
|
||||
url: '',
|
||||
callbacks: { onReplayData, onStreamRecovered, onConnect }
|
||||
})
|
||||
await vi.waitFor(() => expect(subscriptionSendBinary).toHaveBeenCalled())
|
||||
const firstStreamId = latestSubscribePayload().streamId
|
||||
emitSnapshot(firstStreamId, 'INITIAL_SNAPSHOT')
|
||||
subscriptionCallbacks?.onResponse({
|
||||
ok: true,
|
||||
result: {
|
||||
type: 'subscribed',
|
||||
streamId: firstStreamId,
|
||||
capabilities: { outputPause: 1 }
|
||||
}
|
||||
})
|
||||
await vi.waitFor(() => expect(onConnect).toHaveBeenCalledTimes(1))
|
||||
// The first subscribe already carries the host snapshot; re-arming there would cost
|
||||
// every pane a redundant restore request on open.
|
||||
expect(onStreamRecovered).not.toHaveBeenCalled()
|
||||
|
||||
subscriptionCallbacks?.onClose?.()
|
||||
await vi.waitFor(() => expect(runtimeSubscribe).toHaveBeenCalledTimes(2))
|
||||
await vi.waitFor(() =>
|
||||
expect(
|
||||
subscriptionSendBinary.mock.calls
|
||||
.map((call) => decodeTerminalStreamFrame(call[0]))
|
||||
.filter((frame) => frame?.opcode === TerminalStreamOpcode.Subscribe)
|
||||
).toHaveLength(2)
|
||||
)
|
||||
const reconnectStreamId = latestSubscribePayload().streamId
|
||||
// An exited-but-preserved pane has nothing to push and will never emit live bytes,
|
||||
// so without the re-arm the pane stays blank until a visibility flip.
|
||||
emitSnapshot(reconnectStreamId, '')
|
||||
subscriptionCallbacks?.onResponse({
|
||||
ok: true,
|
||||
result: {
|
||||
type: 'subscribed',
|
||||
streamId: reconnectStreamId,
|
||||
capabilities: { outputPause: 1 }
|
||||
}
|
||||
})
|
||||
|
||||
await vi.waitFor(() => expect(onStreamRecovered).toHaveBeenCalledTimes(1))
|
||||
expect(onReplayData.mock.calls.map((call) => call[0])).toEqual(['INITIAL_SNAPSHOT'])
|
||||
|
||||
subscriptionCallbacks?.onClose?.()
|
||||
await vi.waitFor(() => expect(runtimeSubscribe).toHaveBeenCalledTimes(3))
|
||||
await vi.waitFor(() =>
|
||||
expect(
|
||||
subscriptionSendBinary.mock.calls
|
||||
.map((call) => decodeTerminalStreamFrame(call[0]))
|
||||
.filter((frame) => frame?.opcode === TerminalStreamOpcode.Subscribe)
|
||||
).toHaveLength(3)
|
||||
)
|
||||
const populatedReconnectStreamId = latestSubscribePayload().streamId
|
||||
emitSnapshot(populatedReconnectStreamId, 'RECOVERY_SNAPSHOT')
|
||||
subscriptionCallbacks?.onResponse({
|
||||
ok: true,
|
||||
result: {
|
||||
type: 'subscribed',
|
||||
streamId: populatedReconnectStreamId,
|
||||
capabilities: { outputPause: 1 }
|
||||
}
|
||||
})
|
||||
|
||||
await vi.waitFor(() => expect(onConnect).toHaveBeenCalledTimes(3))
|
||||
expect(onStreamRecovered).toHaveBeenCalledTimes(1)
|
||||
expect(onReplayData.mock.calls.map((call) => call[0])).toEqual([
|
||||
'INITIAL_SNAPSHOT',
|
||||
'RECOVERY_SNAPSHOT'
|
||||
])
|
||||
transport.destroy?.()
|
||||
})
|
||||
|
||||
it('backs off before retrying a capacity-rejected terminal stream', async () => {
|
||||
vi.useFakeTimers()
|
||||
try {
|
||||
|
||||
@@ -1735,6 +1735,7 @@ export function createRemoteRuntimePtyTransport(
|
||||
setAttachmentReady(false)
|
||||
let transportClosed = false
|
||||
let subscriptionAttached = false
|
||||
let subscriptionSnapshotHadContent = false
|
||||
// Why: viewport handed to subscribe; a resize during the round-trip falls back to the refresh-only one-shot RPC, replayed through the stream below once current.
|
||||
const subscribedViewport = desiredViewport
|
||||
const isCurrentSubscription = (): boolean =>
|
||||
@@ -1760,6 +1761,7 @@ export function createRemoteRuntimePtyTransport(
|
||||
onSnapshot: (data, meta) => {
|
||||
// Why: an empty snapshot can still carry a pending mid-escape tail that must replay so the next live chunk completes it.
|
||||
if ((data || meta?.pendingEscapeTailAnsi) && isCurrentSubscription()) {
|
||||
subscriptionSnapshotHadContent = true
|
||||
if (subscribedPtyId && bufferPtyShutdownReplayData(subscribedPtyId, data)) {
|
||||
return
|
||||
}
|
||||
@@ -1806,6 +1808,12 @@ export function createRemoteRuntimePtyTransport(
|
||||
markRecoveryHealthy()
|
||||
emitRecoveryState()
|
||||
storedCallbacks.onConnect?.()
|
||||
// Why: a recovery subscribe replays nothing when the host's push snapshot is
|
||||
// empty (idle or exited pane), so ask for the retained buffer instead of
|
||||
// waiting for bytes that an exited process will never send.
|
||||
if (expectedRecoveryEpoch !== undefined && !subscriptionSnapshotHadContent) {
|
||||
storedCallbacks.onStreamRecovered?.()
|
||||
}
|
||||
storedCallbacks.onStatus?.('shell')
|
||||
},
|
||||
onEnd: () => {
|
||||
|
||||
Reference in New Issue
Block a user