fix(terminal): scope pre-handler state to admission

This commit is contained in:
Brennan Benson
2026-07-13 14:32:21 -07:00
parent b294046538
commit b35367b2fc
4 changed files with 85 additions and 4 deletions
@@ -137,6 +137,52 @@ describe('pre-handler PTY buffer', () => {
expect(handler).toHaveBeenCalledWith(-1)
})
it('keeps a replacement probe owned when an older same-id probe settles', async () => {
const target = 'pty-reused-probe-owner'
const oldPrefix = 'pty-reused-probe-old-'
const newPrefix = 'pty-reused-probe-new-'
const resolvers: ((alive: boolean | null) => void)[] = []
const hasPty = vi.fn(
() =>
new Promise<boolean | null>((resolve) => {
resolvers.push(resolve)
})
)
const handler = vi.fn()
try {
bufferPreHandlerPtyExit(target, 1)
for (let index = 0; index < 64; index += 1) {
bufferPreHandlerPtyExit(`${oldPrefix}${index}`, index)
}
reconcilePreHandlerPtyExitAfterOverflow(target, hasPty, handler, () => true)
clearPreHandlerPtyState(target)
bufferPreHandlerPtyExit(target, 2)
for (let index = 0; index < 64; index += 1) {
bufferPreHandlerPtyExit(`${newPrefix}${index}`, index)
}
reconcilePreHandlerPtyExitAfterOverflow(target, hasPty, handler, () => true)
expect(hasPty).toHaveBeenCalledTimes(2)
resolvers[0]?.(true)
await Promise.resolve()
await Promise.resolve()
await Promise.resolve()
reconcilePreHandlerPtyExitAfterOverflow(target, hasPty, handler, () => true)
expect(hasPty).toHaveBeenCalledTimes(2)
expect(handler).not.toHaveBeenCalled()
resolvers[1]?.(true)
await Promise.resolve()
} finally {
clearPreHandlerPtyState(target)
for (let index = 0; index < 64; index += 1) {
clearPreHandlerPtyState(`${oldPrefix}${index}`)
clearPreHandlerPtyState(`${newPrefix}${index}`)
}
}
})
it('caps eviction tombstones at exactly 1024 ids without probing unrelated PTYs', async () => {
const prefix = 'pty-exact-tombstone-cap-'
const hasPty = vi.fn(async () => false)
@@ -26,7 +26,7 @@ const PRE_HANDLER_PTY_EXIT_MAX_PTYS = 64
// window avoids turning one overflow into liveness IPC for unrelated panes.
const PRE_HANDLER_PTY_EVICTED_EXIT_MAX_PTYS = 1_024
const preHandlerPtyEvictedExitIds = new Set<string>()
const preHandlerPtyEvictedExitProbes = new Set<string>()
const preHandlerPtyEvictedExitProbes = new Map<string, symbol>()
// Why: legit pre-attach windows drain within milliseconds and hold little
// data. Sustained accumulation means a pane lost its data handler (the
// frozen-pane detach/attach race) — leave a breadcrumb for trace capture.
@@ -137,12 +137,18 @@ export function reconcilePreHandlerPtyExitAfterOverflow(
) {
return
}
preHandlerPtyEvictedExitProbes.add(ptyId)
const probeToken = Symbol(ptyId)
preHandlerPtyEvictedExitProbes.set(ptyId, probeToken)
// Why: the bounded exit buffer may evict a legitimate pre-registration exit.
// Keep its tombstone through unknown liveness so reconnect can retry proof.
void hasPty(ptyId)
.then((alive) => {
if (!isCurrent() || !preHandlerPtyEvictedExitIds.has(ptyId) || alive === null) {
if (
preHandlerPtyEvictedExitProbes.get(ptyId) !== probeToken ||
!isCurrent() ||
!preHandlerPtyEvictedExitIds.has(ptyId) ||
alive === null
) {
return
}
preHandlerPtyEvictedExitIds.delete(ptyId)
@@ -152,7 +158,10 @@ export function reconcilePreHandlerPtyExitAfterOverflow(
})
.catch(() => {})
.finally(() => {
preHandlerPtyEvictedExitProbes.delete(ptyId)
// Why: a stale probe must not clear a newer same-ID generation's owner.
if (preHandlerPtyEvictedExitProbes.get(ptyId) === probeToken) {
preHandlerPtyEvictedExitProbes.delete(ptyId)
}
})
}
@@ -147,6 +147,27 @@ describe('createIpcPtyTransport', () => {
expect(onDataCallback).not.toHaveBeenCalledWith('dead generation output')
})
it('does not drain an exit buffered before a same-id admission starts', async () => {
const { createIpcPtyTransport, ensurePtyDispatcher } = await import('./pty-transport')
const spawn = window.api.pty.spawn as unknown as ReturnType<typeof vi.fn>
spawn.mockResolvedValueOnce({ id: 'same-id' })
ensurePtyDispatcher()
onExit?.({ id: 'same-id', code: 17 })
const observedExit = vi.fn()
const transport = createIpcPtyTransport()
await transport.connect({
url: '',
sessionId: 'same-id',
callbacks: { onExit: observedExit }
})
expect(transport.isConnected()).toBe(true)
expect(transport.getPtyId()).toBe('same-id')
expect(observedExit).not.toHaveBeenCalled()
transport.disconnect()
})
it('preserves replacement startup events while an older same-id admission rejects', async () => {
const { createIpcPtyTransport } = await import('./pty-transport')
const spawn = window.api.pty.spawn as unknown as ReturnType<typeof vi.fn>
@@ -708,6 +708,11 @@ export function createIpcPtyTransport(opts: IpcPtyTransportOptions = {}): PtyTra
? await acquirePtySessionConnectAdmission(options.sessionId)
: undefined
try {
if (options.sessionId) {
// Why: admission starts a new renderer generation; buffered events
// that predate this boundary belong to the previous same-ID owner.
clearPreHandlerPtyState(options.sessionId)
}
storedCallbacks = options.callbacks
if (destroyed) {
return