mirror of
https://github.com/stablyai/orca.git
synced 2026-10-23 00:03:52 +00:00
fix(terminal): restore gated PTY buffer on hidden-to-visible reveal
STA-4393: a hidden-to-visible tab reveal could leave the visible xterm on a stale alt-screen frame because restore waited for main's one-shot drop marker, and a leftover hidden delivery claim could keep the PTY gated after reveal. Latch current-buffer restore when the pane unhides, and keep the hidden- delivery gate off while any visible renderer still owns the PTY.
This commit is contained in:
+289
@@ -0,0 +1,289 @@
|
||||
import type * as React from 'react'
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
import { flushAsyncTicks } from './pty-connection-test-async'
|
||||
import {
|
||||
createMockTransport,
|
||||
createPane,
|
||||
createManager,
|
||||
type ConnectCallbacks,
|
||||
type MockTransport
|
||||
} from './pty-connection-test-pane-fixtures'
|
||||
import { buildPaneConnectionDeps } from './pty-connection-test-deps'
|
||||
import { createInitialStoreState } from './pty-connection-test-store-fixtures'
|
||||
import type { StoreState } from './pty-connection-test-store-state'
|
||||
import {
|
||||
installTerminalTestGlobals,
|
||||
restoreTerminalTestGlobals
|
||||
} from './pty-connection-test-environment'
|
||||
|
||||
const {
|
||||
resetAndRefreshAllTerminalWebglAtlases,
|
||||
scheduleTerminalWebglAtlasRecovery,
|
||||
scheduleRuntimeGraphSync,
|
||||
shouldSeedCacheTimerOnInitialTitle,
|
||||
toastInfo,
|
||||
notifyCodexPaneBoundForStaleSweep
|
||||
} = vi.hoisted(() => ({
|
||||
resetAndRefreshAllTerminalWebglAtlases: vi.fn(),
|
||||
scheduleTerminalWebglAtlasRecovery: vi.fn(),
|
||||
scheduleRuntimeGraphSync: vi.fn(),
|
||||
shouldSeedCacheTimerOnInitialTitle: vi.fn(() => false),
|
||||
toastInfo: vi.fn(),
|
||||
notifyCodexPaneBoundForStaleSweep: vi.fn()
|
||||
}))
|
||||
|
||||
let mockStoreState: StoreState
|
||||
let transportFactoryQueue: MockTransport[] = []
|
||||
let createdTransportOptions: Record<string, unknown>[] = []
|
||||
let storeSubscribers: ((state: StoreState) => void)[] = []
|
||||
|
||||
vi.mock('@/runtime/sync-runtime-graph', () => ({
|
||||
scheduleRuntimeGraphSync
|
||||
}))
|
||||
|
||||
vi.mock('@/lib/pane-manager/pane-manager-registry', async (importOriginal) => ({
|
||||
...(await importOriginal<Record<string, unknown>>()),
|
||||
resetAndRefreshAllTerminalWebglAtlases
|
||||
}))
|
||||
|
||||
vi.mock('./terminal-webgl-atlas-recovery', () => ({
|
||||
scheduleTerminalWebglAtlasRecovery
|
||||
}))
|
||||
|
||||
vi.mock('@/store', () => ({
|
||||
useAppStore: {
|
||||
getState: () => mockStoreState,
|
||||
subscribe: (listener: (state: StoreState) => void) => {
|
||||
storeSubscribers.push(listener)
|
||||
return () => {
|
||||
storeSubscribers = storeSubscribers.filter((candidate) => candidate !== listener)
|
||||
}
|
||||
}
|
||||
}
|
||||
}))
|
||||
|
||||
vi.mock('@/lib/agent-status', async (importOriginal) => {
|
||||
const { buildAgentStatusModuleMock } = await import('./pty-connection-test-environment')
|
||||
return buildAgentStatusModuleMock(await importOriginal<Record<string, unknown>>())
|
||||
})
|
||||
|
||||
vi.mock('./cache-timer-seeding', () => ({
|
||||
shouldSeedCacheTimerOnInitialTitle
|
||||
}))
|
||||
|
||||
vi.mock('sonner', () => ({
|
||||
toast: {
|
||||
info: toastInfo
|
||||
}
|
||||
}))
|
||||
|
||||
vi.mock('@/lib/codex-stale-pane-sweep', () => ({
|
||||
notifyCodexPaneBoundForStaleSweep
|
||||
}))
|
||||
|
||||
vi.mock('react', async (importOriginal) => {
|
||||
const actual = await importOriginal<typeof React>()
|
||||
return {
|
||||
...actual,
|
||||
useCallback: <T extends (...args: unknown[]) => unknown>(fn: T): T => fn
|
||||
}
|
||||
})
|
||||
|
||||
vi.mock('./pty-transport', () => ({
|
||||
createIpcPtyTransport: vi.fn((options: Record<string, unknown>) => {
|
||||
createdTransportOptions.push(options)
|
||||
const nextTransport = transportFactoryQueue.shift()
|
||||
if (!nextTransport) {
|
||||
throw new Error('No mock transport queued')
|
||||
}
|
||||
return nextTransport
|
||||
})
|
||||
}))
|
||||
|
||||
vi.mock('./remote-runtime-pty-transport', () => ({
|
||||
createRemoteRuntimePtyTransport: vi.fn(
|
||||
(_environmentId: string, options: Record<string, unknown>) => {
|
||||
createdTransportOptions.push(options)
|
||||
const nextTransport = transportFactoryQueue.shift()
|
||||
if (!nextTransport) {
|
||||
throw new Error('No mock transport queued')
|
||||
}
|
||||
return nextTransport
|
||||
}
|
||||
)
|
||||
}))
|
||||
|
||||
vi.mock('./pty-dispatcher', async (importOriginal) => {
|
||||
const actual = await importOriginal<Record<string, unknown>>()
|
||||
return {
|
||||
...actual,
|
||||
getEagerPtyBufferHandle: vi.fn(() => undefined)
|
||||
}
|
||||
})
|
||||
|
||||
function createDeps(overrides: Record<string, unknown> = {}) {
|
||||
return buildPaneConnectionDeps(() => mockStoreState, overrides)
|
||||
}
|
||||
|
||||
function writtenText(pane: ReturnType<typeof createPane>): string {
|
||||
return pane.terminal.write.mock.calls.map((call) => String(call[0])).join('')
|
||||
}
|
||||
|
||||
function writeCallOrder(pane: ReturnType<typeof createPane>, fragment: string): number | undefined {
|
||||
const index = pane.terminal.write.mock.calls.findIndex((call) =>
|
||||
String(call[0]).includes(fragment)
|
||||
)
|
||||
return index !== -1 ? pane.terminal.write.mock.invocationCallOrder[index] : undefined
|
||||
}
|
||||
|
||||
describe('hidden-to-visible PTY reveal replay', () => {
|
||||
beforeEach(() => {
|
||||
vi.resetModules()
|
||||
vi.clearAllMocks()
|
||||
transportFactoryQueue = []
|
||||
createdTransportOptions = []
|
||||
storeSubscribers = []
|
||||
mockStoreState = createInitialStoreState(() => mockStoreState)
|
||||
mockStoreState.settings = {
|
||||
...mockStoreState.settings,
|
||||
terminalMainSideEffectAuthority: true
|
||||
} as StoreState['settings']
|
||||
installTerminalTestGlobals()
|
||||
})
|
||||
|
||||
afterEach(async () => {
|
||||
await restoreTerminalTestGlobals()
|
||||
})
|
||||
|
||||
it('replays the current buffer on visibility sync, then resumes live frames', async () => {
|
||||
const { connectPanePty } = await import('./pty-connection')
|
||||
const transport = createMockTransport('pty-id')
|
||||
const capturedDataCallback: {
|
||||
current: ((data: string, meta?: { seq?: number; rawLength?: number }) => void) | null
|
||||
} = { current: null }
|
||||
transport.connect.mockImplementation(async ({ callbacks }: { callbacks: ConnectCallbacks }) => {
|
||||
capturedDataCallback.current = callbacks.onData ?? null
|
||||
return 'pty-id'
|
||||
})
|
||||
transportFactoryQueue.push(transport)
|
||||
|
||||
const snapshot = 'authoritative frame 000010'
|
||||
const live = 'authoritative frame 000011'
|
||||
const getMainBufferSnapshot = window.api.pty.getMainBufferSnapshot as unknown as ReturnType<
|
||||
typeof vi.fn
|
||||
>
|
||||
getMainBufferSnapshot.mockResolvedValue({
|
||||
data: `${snapshot}\r\n`,
|
||||
cols: 100,
|
||||
rows: 30,
|
||||
seq: 64
|
||||
})
|
||||
|
||||
const pane = createPane(1)
|
||||
const manager = createManager(1)
|
||||
const isVisibleRef = { current: false }
|
||||
const binding = connectPanePty(
|
||||
pane as never,
|
||||
manager as never,
|
||||
createDeps({ isVisibleRef }) as never
|
||||
) as {
|
||||
syncProcessTracking: () => void
|
||||
dispose: () => void
|
||||
}
|
||||
await flushAsyncTicks(6)
|
||||
|
||||
capturedDataCallback.current?.('stale hidden frame\r\n', { seq: 16, rawLength: 18 })
|
||||
pane.terminal.write.mockClear()
|
||||
getMainBufferSnapshot.mockClear()
|
||||
|
||||
// Reveal through the pane visibility hook only. Production tab reveal can
|
||||
// race the restore marker and the backlog-recovery call.
|
||||
isVisibleRef.current = true
|
||||
binding.syncProcessTracking()
|
||||
await flushAsyncTicks(20)
|
||||
|
||||
expect(getMainBufferSnapshot).toHaveBeenCalledWith('pty-id', { scrollbackRows: 5000 })
|
||||
expect(writtenText(pane)).toContain(snapshot)
|
||||
|
||||
capturedDataCallback.current?.(live, {
|
||||
seq: 64 + live.length,
|
||||
rawLength: live.length
|
||||
})
|
||||
await flushAsyncTicks(20)
|
||||
|
||||
expect(writtenText(pane)).toContain(live)
|
||||
expect(writeCallOrder(pane, snapshot)).toBeLessThan(writeCallOrder(pane, live) ?? 0)
|
||||
|
||||
binding.dispose()
|
||||
})
|
||||
|
||||
it('retires a duplicate hidden renderer once so the revealed pane can restore', async () => {
|
||||
const { connectPanePty } = await import('./pty-connection')
|
||||
const getMainBufferSnapshot = window.api.pty.getMainBufferSnapshot as unknown as ReturnType<
|
||||
typeof vi.fn
|
||||
>
|
||||
getMainBufferSnapshot.mockResolvedValue({
|
||||
data: 'authoritative frame 000010\r\n',
|
||||
cols: 100,
|
||||
rows: 30,
|
||||
seq: 64
|
||||
})
|
||||
|
||||
const connectGated = async (
|
||||
paneId: number,
|
||||
isVisibleRef: { current: boolean }
|
||||
): Promise<{
|
||||
pane: ReturnType<typeof createPane>
|
||||
binding: { syncProcessTracking: () => void; dispose: () => void }
|
||||
dataCallback: (data: string, meta?: { seq?: number; rawLength?: number }) => void
|
||||
}> => {
|
||||
const transport = createMockTransport('pty-id')
|
||||
const capturedDataCallback: {
|
||||
current: ((data: string, meta?: { seq?: number; rawLength?: number }) => void) | null
|
||||
} = { current: null }
|
||||
transport.connect.mockImplementation(
|
||||
async ({ callbacks }: { callbacks: ConnectCallbacks }) => {
|
||||
capturedDataCallback.current = callbacks.onData ?? null
|
||||
return 'pty-id'
|
||||
}
|
||||
)
|
||||
transportFactoryQueue.push(transport)
|
||||
const pane = createPane(paneId)
|
||||
const manager = createManager(paneId)
|
||||
const binding = connectPanePty(
|
||||
pane as never,
|
||||
manager as never,
|
||||
createDeps({ isVisibleRef, restoredLeafId: pane.leafId }) as never
|
||||
) as { syncProcessTracking: () => void; dispose: () => void }
|
||||
await flushAsyncTicks(6)
|
||||
return { pane, binding, dataCallback: capturedDataCallback.current! }
|
||||
}
|
||||
|
||||
const retainedVisibleRef = { current: false }
|
||||
const duplicateVisibleRef = { current: false }
|
||||
const retained = await connectGated(1, retainedVisibleRef)
|
||||
const duplicate = await connectGated(2, duplicateVisibleRef)
|
||||
|
||||
retained.dataCallback('stale hidden frame\r\n', { seq: 16, rawLength: 18 })
|
||||
duplicate.dataCallback('stale hidden frame\r\n', { seq: 16, rawLength: 18 })
|
||||
const setHiddenRendererPty = window.api.pty.setHiddenRendererPty as unknown as ReturnType<
|
||||
typeof vi.fn
|
||||
>
|
||||
expect(setHiddenRendererPty).toHaveBeenCalledWith('pty-id', true)
|
||||
setHiddenRendererPty.mockClear()
|
||||
getMainBufferSnapshot.mockClear()
|
||||
retained.pane.terminal.write.mockClear()
|
||||
|
||||
duplicate.binding.dispose()
|
||||
retainedVisibleRef.current = true
|
||||
retained.binding.syncProcessTracking()
|
||||
await flushAsyncTicks(20)
|
||||
|
||||
expect(setHiddenRendererPty).toHaveBeenCalledTimes(1)
|
||||
expect(setHiddenRendererPty).toHaveBeenCalledWith('pty-id', false)
|
||||
expect(getMainBufferSnapshot).toHaveBeenCalledTimes(1)
|
||||
expect(writtenText(retained.pane)).toContain('authoritative frame 000010')
|
||||
|
||||
retained.binding.dispose()
|
||||
})
|
||||
})
|
||||
@@ -6287,6 +6287,10 @@ export function connectPanePty(
|
||||
} else if (releaseHiddenDeliveryClaim) {
|
||||
releaseHiddenDeliveryClaim()
|
||||
releaseHiddenDeliveryClaim = null
|
||||
// Why: unhide must latch restore locally. Waiting for main's one-shot
|
||||
// marker races light-tab reveal, which no-ops when restore isn't needed
|
||||
// yet, then live TUI frames never paint the gated-stale alt-screen.
|
||||
markHiddenOutputRestoreNeeded()
|
||||
} else if (isFirstSyncForPty) {
|
||||
// Why: clear unconditionally on first sync — a stale main-side hidden bit can survive a renderer reload for daemon-backed PTYs that keep their session id.
|
||||
declareRendererPtyDeliveryVisible(ptyId)
|
||||
|
||||
@@ -59,4 +59,17 @@ describe('renderer PTY delivery claims', () => {
|
||||
setRendererPtyVisibilityClaim({}, PTY_ID, false)
|
||||
expect(setRendererPtyVisible).toHaveBeenCalledWith(PTY_ID, false)
|
||||
})
|
||||
|
||||
it('ungates a visible renderer even while a stale hidden owner still holds a claim', () => {
|
||||
const visiblePane = {}
|
||||
const staleHidden = acquireHiddenRendererPtyDeliveryClaim(PTY_ID)
|
||||
expect(setHiddenRendererPty).toHaveBeenLastCalledWith(PTY_ID, true)
|
||||
|
||||
setRendererPtyVisibilityClaim(visiblePane, PTY_ID, true)
|
||||
expect(setHiddenRendererPty).toHaveBeenLastCalledWith(PTY_ID, false)
|
||||
|
||||
staleHidden()
|
||||
expect(setHiddenRendererPty).toHaveBeenLastCalledWith(PTY_ID, false)
|
||||
releaseRendererPtyVisibilityClaim(visiblePane)
|
||||
})
|
||||
})
|
||||
|
||||
@@ -7,6 +7,7 @@ type VisibilityClaim = { ptyId: string; visible: boolean }
|
||||
|
||||
const visibilityClaimsByOwner = new Map<object, VisibilityClaim>()
|
||||
const visibleClaimCounts = new Map<string, number>()
|
||||
const lastPublishedHiddenGate = new Map<string, boolean>()
|
||||
|
||||
function sendHiddenState(ptyId: string, hidden: boolean): void {
|
||||
recordTerminalFreezeBreadcrumb(hidden ? 'renderer-gate-mark' : 'renderer-gate-unmark', {
|
||||
@@ -15,20 +16,38 @@ function sendHiddenState(ptyId: string, hidden: boolean): void {
|
||||
;(globalThis as { window?: Window }).window?.api?.pty?.setHiddenRendererPty?.(ptyId, hidden)
|
||||
}
|
||||
|
||||
function shouldGateHiddenRendererPty(ptyId: string): boolean {
|
||||
return (hiddenClaimCounts.get(ptyId) ?? 0) > 0 && (visibleClaimCounts.get(ptyId) ?? 0) === 0
|
||||
}
|
||||
|
||||
function publishHiddenRendererGate(ptyId: string): void {
|
||||
const hidden = shouldGateHiddenRendererPty(ptyId)
|
||||
if (lastPublishedHiddenGate.get(ptyId) === hidden) {
|
||||
return
|
||||
}
|
||||
if (lastPublishedHiddenGate.get(ptyId) === undefined && !hidden) {
|
||||
lastPublishedHiddenGate.set(ptyId, false)
|
||||
return
|
||||
}
|
||||
lastPublishedHiddenGate.set(ptyId, hidden)
|
||||
sendHiddenState(ptyId, hidden)
|
||||
if (!hidden && !hiddenClaimCounts.has(ptyId) && !visibleClaimCounts.has(ptyId)) {
|
||||
lastPublishedHiddenGate.delete(ptyId)
|
||||
}
|
||||
}
|
||||
|
||||
function sendVisibility(ptyId: string, visible: boolean): void {
|
||||
;(globalThis as { window?: Window }).window?.api?.pty?.setRendererPtyVisible?.(ptyId, visible)
|
||||
}
|
||||
|
||||
/**
|
||||
* Holds a hidden-delivery claim until the returned release function runs.
|
||||
* Main sees only the first-acquire/last-release transitions for each PTY.
|
||||
* Main stays gated only while some hidden owner exists and no visible owner does.
|
||||
*/
|
||||
export function acquireHiddenRendererPtyDeliveryClaim(ptyId: string): () => void {
|
||||
const nextCount = (hiddenClaimCounts.get(ptyId) ?? 0) + 1
|
||||
hiddenClaimCounts.set(ptyId, nextCount)
|
||||
if (nextCount === 1) {
|
||||
sendHiddenState(ptyId, true)
|
||||
}
|
||||
publishHiddenRendererGate(ptyId)
|
||||
|
||||
let released = false
|
||||
return () => {
|
||||
@@ -39,19 +58,22 @@ export function acquireHiddenRendererPtyDeliveryClaim(ptyId: string): () => void
|
||||
const currentCount = hiddenClaimCounts.get(ptyId) ?? 0
|
||||
if (currentCount <= 1) {
|
||||
hiddenClaimCounts.delete(ptyId)
|
||||
sendHiddenState(ptyId, false)
|
||||
publishHiddenRendererGate(ptyId)
|
||||
return
|
||||
}
|
||||
hiddenClaimCounts.set(ptyId, currentCount - 1)
|
||||
publishHiddenRendererGate(ptyId)
|
||||
}
|
||||
}
|
||||
|
||||
/** Clears stale main state for a visible PTY without overriding another live
|
||||
* hidden owner that is still completing a pane-to-watcher handoff. */
|
||||
export function declareRendererPtyDeliveryVisible(ptyId: string): void {
|
||||
if (!hiddenClaimCounts.has(ptyId)) {
|
||||
sendHiddenState(ptyId, false)
|
||||
if (shouldGateHiddenRendererPty(ptyId)) {
|
||||
return
|
||||
}
|
||||
lastPublishedHiddenGate.set(ptyId, false)
|
||||
sendHiddenState(ptyId, false)
|
||||
}
|
||||
|
||||
function removeVisibleClaim(claim: VisibilityClaim): boolean {
|
||||
@@ -86,6 +108,9 @@ export function setRendererPtyVisibilityClaim(
|
||||
if (becameHidden && previous.ptyId !== ptyId) {
|
||||
sendVisibility(previous.ptyId, false)
|
||||
}
|
||||
if (previous.ptyId !== ptyId) {
|
||||
publishHiddenRendererGate(previous.ptyId)
|
||||
}
|
||||
}
|
||||
|
||||
visibilityClaimsByOwner.set(owner, { ptyId, visible })
|
||||
@@ -95,12 +120,14 @@ export function setRendererPtyVisibilityClaim(
|
||||
if (nextCount === 1) {
|
||||
sendVisibility(ptyId, true)
|
||||
}
|
||||
publishHiddenRendererGate(ptyId)
|
||||
return
|
||||
}
|
||||
|
||||
if (!visibleClaimCounts.has(ptyId)) {
|
||||
sendVisibility(ptyId, false)
|
||||
}
|
||||
publishHiddenRendererGate(ptyId)
|
||||
}
|
||||
|
||||
export function releaseRendererPtyVisibilityClaim(owner: object): void {
|
||||
@@ -112,6 +139,7 @@ export function releaseRendererPtyVisibilityClaim(owner: object): void {
|
||||
if (removeVisibleClaim(previous)) {
|
||||
sendVisibility(previous.ptyId, false)
|
||||
}
|
||||
publishHiddenRendererGate(previous.ptyId)
|
||||
}
|
||||
|
||||
/** Test seam: renderer reload naturally clears these module-scoped claims. */
|
||||
@@ -119,4 +147,5 @@ export function _resetPtyRendererDeliveryClaimsForTest(): void {
|
||||
hiddenClaimCounts.clear()
|
||||
visibilityClaimsByOwner.clear()
|
||||
visibleClaimCounts.clear()
|
||||
lastPublishedHiddenGate.clear()
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user