fix: harden reused PTY lifecycle ordering

This commit is contained in:
Neil
2026-07-11 04:19:49 -07:00
parent e1227b9064
commit 99ada0571e
5 changed files with 70 additions and 4 deletions
+3 -3
View File
@@ -3264,7 +3264,7 @@
"file": "src/renderer/src/components/terminal-pane/pty-pre-handler-buffer.test.ts",
"assertions": [
"65 exit-before-handler identities evict the oldest and retain the newest",
"a reused PTY id refreshes recency, delivers only its latest exit once, and filters older-lifecycle data and exits by cursor",
"a reused PTY id refreshes both data and exit recency, delivers only its latest exit once, and filters older-lifecycle data and exits by cursor",
"warning identities are released with exact data-map eviction so warning state cannot grow independently"
]
},
@@ -3272,7 +3272,7 @@
"file": "src/renderer/src/components/terminal-pane/pty-dispatcher-pi-routing.test.ts",
"assertions": [
"pre-handler data drains before its exit",
"fresh PTY-id reuse filters old lifecycle events while explicit daemon reattach preserves pre-connect backlog",
"fresh PTY-id reuse and failed-reattach fresh fallback filter old lifecycle events while explicit daemon reattach preserves pre-connect backlog",
"a registered exit handler remains authoritative while 65 orphan exits churn through the bounded buffer"
]
},
@@ -3291,7 +3291,7 @@
"command": "pnpm exec vitest run --config config/vitest.config.ts src/renderer/src/components/terminal-pane/pty-pre-handler-buffer.test.ts src/renderer/src/components/terminal-pane/pty-transport.test.ts src/renderer/src/components/terminal-pane/pty-dispatcher-pi-routing.test.ts src/renderer/src/lib/launch-agent-background-session.test.ts src/renderer/src/lib/launch-worktree-background-terminals.test.ts",
"result": "passed",
"durationSeconds": 1,
"summary": "5 test files passed, 112 tests passed on main@20ebe858af plus the #7695 candidate. The current-main repro retained the 65th oldest exit (received code 0 instead of eviction); the bounded candidate evicts it and preserves all ordering/reuse cases."
"summary": "5 test files passed, 114 tests passed on main@20ebe858af plus the #7695 candidate. Main@20ebe858af retained the 65th oldest exit (received code 0 instead of eviction); the bounded candidate evicts it and preserves all ordering/reuse cases."
}
],
"runtimeBudget": {
@@ -368,6 +368,38 @@ describe('dispatcher → transport → onTitleChange for Pi spinner', () => {
expect(events).toEqual(['data:new-data', 'exit:2'])
})
it('filters an older lifecycle when a failed reattach spawns fresh with the same id', async () => {
const { createIpcPtyTransport } = await import('./pty-transport')
const { ensurePtyDispatcher } = await import('./pty-dispatcher')
const events: string[] = []
let resolveSpawn: (value: { id: string }) => void = () => {}
ensurePtyDispatcher()
dispatcherCallback?.({ id: 'pty-reattach-fallback', data: 'old-data' })
exitDispatcherCallback?.({ id: 'pty-reattach-fallback', code: 1 })
;(window.api.pty.spawn as ReturnType<typeof vi.fn>).mockReturnValueOnce(
new Promise((resolve) => {
resolveSpawn = resolve
})
)
const transport = createIpcPtyTransport()
const connectPromise = transport.connect({
url: '',
sessionId: 'pty-reattach-fallback',
callbacks: {
onData: (data) => events.push(`data:${data}`),
onExit: (code) => events.push(`exit:${code}`)
}
})
dispatcherCallback?.({ id: 'pty-reattach-fallback', data: 'fresh-data' })
exitDispatcherCallback?.({ id: 'pty-reattach-fallback', code: 2 })
resolveSpawn({ id: 'pty-reattach-fallback' })
await connectPromise
expect(events).toEqual(['data:fresh-data', 'exit:2'])
})
it('preserves pre-connect backlog for an explicit daemon reattach', async () => {
const { createIpcPtyTransport } = await import('./pty-transport')
const { ensurePtyDispatcher } = await import('./pty-dispatcher')
@@ -20,6 +20,10 @@ const WARN_PTY_IDS = Array.from(
{ length: PRE_HANDLER_PTY_MAX_PTYS + 1 },
(_, index) => `pty-pre-handler-warn-${index}`
)
const DATA_PTY_IDS = Array.from(
{ length: PRE_HANDLER_PTY_MAX_PTYS + 1 },
(_, index) => `pty-pre-handler-data-${index}`
)
describe('pre-handler PTY buffer', () => {
afterEach(() => {
@@ -32,6 +36,9 @@ describe('pre-handler PTY buffer', () => {
for (const ptyId of WARN_PTY_IDS) {
clearPreHandlerPtyState(ptyId)
}
for (const ptyId of DATA_PTY_IDS) {
clearPreHandlerPtyState(ptyId)
}
})
it('does not rescan historical chunks while buffering small startup output', () => {
@@ -128,6 +135,26 @@ describe('pre-handler PTY buffer', () => {
expect(reusedHandler).toHaveBeenCalledWith(99)
})
it('refreshes data recency when a reused PTY id receives current lifecycle output', () => {
bufferPreHandlerPtyData(DATA_PTY_IDS[0], 'old-data')
for (let index = 1; index < PRE_HANDLER_PTY_MAX_PTYS; index += 1) {
bufferPreHandlerPtyData(DATA_PTY_IDS[index], `data-${index}`)
}
bufferPreHandlerPtyData(DATA_PTY_IDS[0], 'current-data')
bufferPreHandlerPtyData(DATA_PTY_IDS.at(-1)!, 'newest-data')
const oldestHandler = vi.fn()
drainPreHandlerPtyData(DATA_PTY_IDS[1], oldestHandler)
expect(oldestHandler).not.toHaveBeenCalled()
const reusedHandler = vi.fn()
drainPreHandlerPtyData(DATA_PTY_IDS[0], reusedHandler)
expect(reusedHandler.mock.calls).toEqual([
['old-data', undefined],
['current-data', undefined]
])
})
it('clears an old lifecycle before a PTY id is reused and drains the new exit once', () => {
const ptyId = EXIT_PTY_IDS[0]
bufferPreHandlerPtyExit(ptyId, 1)
@@ -58,6 +58,11 @@ export function bufferPreHandlerPtyData(ptyId: string, data: string, meta?: PtyD
if (!state) {
state = { chunks: [], head: 0, bytes: 0 }
preHandlerPtyData.set(ptyId, state)
} else {
// Why: relay-style providers can reuse ids. New output makes this identity
// current, so later churn must evict an actually older buffered PTY.
preHandlerPtyData.delete(ptyId)
preHandlerPtyData.set(ptyId, state)
}
state.chunks.push({
data: chunk.data,
@@ -691,7 +691,9 @@ export function createIpcPtyTransport(opts: IpcPtyTransportOptions = {}): PtyTra
}
try {
const freshSpawnCursor = options.sessionId ? undefined : capturePreHandlerPtyEventCursor()
// Why: a requested reattach can fall back to a fresh PTY with the same
// id. Keep the cursor unless the provider confirms an actual restore.
const freshSpawnCursor = capturePreHandlerPtyEventCursor()
// 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.