fix(terminal): serialize renderer session admission

This commit is contained in:
Brennan Benson
2026-07-13 13:44:26 -07:00
parent a6a6107892
commit d30f59eefc
4 changed files with 127 additions and 6 deletions
@@ -0,0 +1,39 @@
import { afterEach, describe, expect, it } from 'vitest'
import {
acquirePtySessionConnectAdmission,
getPtySessionConnectAdmissionCountForTest
} from './pty-session-connect-admission'
describe('PTY session connect admission', () => {
afterEach(() => {
expect(getPtySessionConnectAdmissionCountForTest()).toBe(0)
})
it('serializes one session without blocking a different session', async () => {
const releaseFirst = await acquirePtySessionConnectAdmission('same-id')
let secondAdmitted = false
const secondAdmission = acquirePtySessionConnectAdmission('same-id').then((release) => {
secondAdmitted = true
return release
})
const releaseDifferent = await acquirePtySessionConnectAdmission('different-id')
await Promise.resolve()
expect(secondAdmitted).toBe(false)
expect(getPtySessionConnectAdmissionCountForTest()).toBe(2)
releaseFirst()
const releaseSecond = await secondAdmission
releaseSecond()
releaseDifferent()
})
it('releases every retained identity after repeated admissions', async () => {
for (let index = 0; index < 100; index += 1) {
const release = await acquirePtySessionConnectAdmission(`session-${index}`)
release()
}
expect(getPtySessionConnectAdmissionCountForTest()).toBe(0)
})
})
@@ -0,0 +1,29 @@
const sessionAdmissionTails = new Map<string, Promise<void>>()
export async function acquirePtySessionConnectAdmission(sessionId: string): Promise<() => void> {
const predecessor = sessionAdmissionTails.get(sessionId) ?? Promise.resolve()
let releaseCurrent!: () => void
const current = new Promise<void>((resolve) => {
releaseCurrent = resolve
})
const tail = predecessor.then(() => current)
sessionAdmissionTails.set(sessionId, tail)
await predecessor
let released = false
return () => {
if (released) {
return
}
released = true
releaseCurrent()
// Why: a settled predecessor must not delete a newer queued owner.
if (sessionAdmissionTails.get(sessionId) === tail) {
sessionAdmissionTails.delete(sessionId)
}
}
}
export function getPtySessionConnectAdmissionCountForTest(): number {
return sessionAdmissionTails.size
}
@@ -147,6 +147,52 @@ describe('createIpcPtyTransport', () => {
expect(onDataCallback).not.toHaveBeenCalledWith('dead generation output')
})
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>
let rejectFirstSpawn: ((error: Error) => void) | undefined
let resolveReplacementSpawn: ((result: { id: string }) => void) | undefined
spawn
.mockImplementationOnce(
() =>
new Promise((_resolve, reject) => {
rejectFirstSpawn = reject
})
)
.mockImplementationOnce(() => {
onData?.({ id: 'same-id', data: 'replacement startup output' })
onExit?.({ id: 'same-id', code: 23 })
return new Promise<{ id: string }>((resolve) => {
resolveReplacementSpawn = resolve
})
})
const firstTransport = createIpcPtyTransport({})
const replacementData = vi.fn()
const replacementExit = vi.fn()
const replacementTransport = createIpcPtyTransport({})
const firstConnect = firstTransport.connect({
url: '',
sessionId: 'same-id',
callbacks: {}
})
const replacementConnect = replacementTransport.connect({
url: '',
sessionId: 'same-id',
callbacks: { onData: replacementData, onExit: replacementExit }
})
await vi.waitFor(() => expect(rejectFirstSpawn).toBeTypeOf('function'))
rejectFirstSpawn?.(new Error('Daemon PTY exited during admission for "same-id"'))
await firstConnect
await vi.waitFor(() => expect(spawn).toHaveBeenCalledTimes(2))
resolveReplacementSpawn?.({ id: 'same-id' })
await replacementConnect
expect(replacementData).toHaveBeenCalledWith('replacement startup output')
expect(replacementExit).toHaveBeenCalledWith(23)
})
it('ignores a stale exit for a previous PTY after reconnecting the same transport', async () => {
const { createIpcPtyTransport } = await import('./pty-transport')
const spawn = window.api.pty.spawn as unknown as ReturnType<typeof vi.fn>
@@ -29,6 +29,7 @@ import {
reconcilePreHandlerPtyExitAfterOverflow
} from './pty-pre-handler-buffer'
import { createPtyInputWriteQueue } from './pty-input-write-queue'
import { acquirePtySessionConnectAdmission } from './pty-session-connect-admission'
import type { PtyDataMeta } from './pty-dispatcher'
import type { IpcPtyTransportOptions, PtyConnectResult, PtyTransport } from './pty-transport-types'
import { createBellDetector } from '../../../../shared/terminal-bell-detector'
@@ -700,14 +701,18 @@ export function createIpcPtyTransport(opts: IpcPtyTransportOptions = {}): PtyTra
return {
async connect(options) {
storedCallbacks = options.callbacks
ensurePtyDispatcher()
if (destroyed) {
return
}
// Why: rejected-generation cleanup is ID-scoped, so a replacement with
// the same session ID must not publish until its predecessor releases.
const releaseSessionAdmission = options.sessionId
? await acquirePtySessionConnectAdmission(options.sessionId)
: undefined
try {
storedCallbacks = options.callbacks
if (destroyed) {
return
}
// Why: missing-cwd recovery is only valid for fresh local spawns —
// reattach must keep the session's exact cwd and SSH-tagged transports
// resolve cwd on the remote host.
@@ -856,6 +861,8 @@ export function createIpcPtyTransport(opts: IpcPtyTransportOptions = {}): PtyTra
storedCallbacks.onError?.(msg)
}
return undefined
} finally {
releaseSessionAdmission?.()
}
},