From a907dd2ba2fdf0976bee0ecbab18f9a5e1e25f58 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Fri, 28 Aug 2026 04:17:36 -0700 Subject: [PATCH] fix(pty): key buffered pre-attach PTY exits on the incarnation, not a sequence fence (#17010) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(pty): key buffered pre-attach exits on the PTY incarnation, not a clock A restarted SSH relay renumbers PTYs from pty-1, so a fresh spawn is routinely handed an id whose previous shell is still emitting a late exit. #16970 stopped that exit blanking the new tab by dating every buffered record and dropping anything older than the spawn request. That fence is a clock, so it cannot judge a stale exit that arrives AFTER the request left — the residual risk #16970 documented. Thread the incarnation main already puts on the pty:exit payload (and the pty:spawn reply) through preload to the pre-handler buffer, so a buffered exit names which lifetime of the id died. An exit disagreeing with the incarnation now attaching is discarded whenever it arrived. Only a positive disagreement discards: absence stays "unknown", never a mismatch, so hosts that predate the field keep #16970's behaviour exactly. The fence is retained for the two cases with no incarnation to compare — buffered bytes (pty:data carries none) and unnamed exits. No wire change: incarnationId was already published on the relay's pty.exit notification and pty.spawn reply, and already forwarded over the in-process pty:exit / pty:spawn IPC. Only the preload types and the renderer read it now. * fix(pty): read the incarnation through the shared guard, not truthiness A malformed incarnation is evidence of nothing, so it must read as "unknown" rather than as a value that disagrees with every well-formed one — otherwise a non-string on the payload would discard the very exits the buffer exists to deliver. Route both the record and the comparison through the existing isPtyIncarnationId guard. * refactor(pty): name the bounded-map helper after what it does It evicts the oldest entry when the map is full and the id is new; it reserves nothing. Rename only — no behaviour change. * fix(pty): key the buffered-exit STORAGE on the incarnation too, not just the check Review caught a swallowed exit. Keying only the comparison on the incarnation while the storage stayed one slot per pty id left the two races this buffer exists for able to cancel each other out: 1. the freshly spawned shell dies before the pane attaches -> its exit (X) is buffered; 2. the relay flushes the previous owner's exit for the same recycled id (W), which OVERWRITES X in the single slot; 3. the spawn reply names X, so the identity discard drops W -- the only record left. registerExit then finds nothing and the pane binds to a PTY that is dead and will never be reported dead: a hang instead of the blank tab #16970 fixed. Store one record per lifetime, capped at 4 per id, so W can never evict X. A duplicate exit for a lifetime replaces that lifetime's record rather than crowding out another's; drain still delivers the newest survivor, preserving the last-write-wins behaviour a single slot always had. * fix(pty): filter buffered exits by lifetime inside the buffer, not at call sites Review found the identity was enforced only where connectIpcPty calls the discard, while preHandlerPtyExit has several other consumers. The severe one is registerEagerPtyBuffer: both background launchers spawn directly and then drain whatever is buffered for the returned id, so a relay-recycled id holding the previous owner's exit tore a freshly launched agent session down seconds after it started -- no fence, no admitPtyId, no identity check at all. Move the rule into the buffer: every read goes through admissiblePreHandlerPtyExits, so a record proven to belong to another lifetime is unreachable by construction rather than because each caller remembered to discard first. hasPreHandlerPtyExit/drainPreHandlerPtyExit take the asking lifetime; registerEagerPtyBuffer and registerExit thread it through, and both background launchers pass the incarnation their own spawn returned. A reader that cannot name an incarnation still sees everything, which is the honest answer -- it holds no evidence to discriminate with. That keeps the pre-spawn fast path in connectIpcPty behaving exactly as it does today; see the PR for the consumers this still does not cover. * chore: drop unrelated formatter churn in reliability-gates.jsonc Repo-wide oxfmt reindented pre-existing entries in a file this change never touches. Keep the diff to the PTY incarnation work. --- src/preload/api/pty-api.ts | 10 +- src/preload/index.ts | 21 ++- .../terminal-pane/ipc-pty-connect.ts | 7 +- .../terminal-pane/ipc-pty-session-handlers.ts | 8 +- .../terminal-pane/ipc-pty-spawn-request.ts | 12 +- .../terminal-pane/pty-dispatcher.ts | 11 +- .../terminal-pane/pty-exit-delivery.ts | 4 +- .../pty-pre-handler-buffer.test.ts | 120 +++++++++++++ .../terminal-pane/pty-pre-handler-buffer.ts | 162 +++++++++++++++--- ...transport-recycled-pty-incarnation.test.ts | 97 +++++++++++ .../pty-transport-test-harness.ts | 8 +- .../launch-agent-background-session.test.ts | 9 +- .../lib/launch-agent-background-session.ts | 12 +- .../launch-worktree-background-terminals.ts | 43 +++-- 14 files changed, 464 insertions(+), 60 deletions(-) create mode 100644 src/renderer/src/components/terminal-pane/pty-transport-recycled-pty-incarnation.test.ts diff --git a/src/preload/api/pty-api.ts b/src/preload/api/pty-api.ts index fc558b60d74..7eee16ea522 100644 --- a/src/preload/api/pty-api.ts +++ b/src/preload/api/pty-api.ts @@ -48,6 +48,8 @@ export type PtyApi = { telemetry?: { agent_kind: AgentKind; launch_source: LaunchSource; request_kind: RequestKind } }) => Promise<{ id: string + /** Which lifetime of `id` this reply named; absent when the execution host predates the field. */ + incarnationId?: string launchAgent?: TuiAgent launchConfig?: SleepingAgentLaunchConfig snapshot?: string @@ -196,7 +198,13 @@ export type PtyApi = { /** Title-only replay snapshot for (re)attach; attention facts never replay. */ getSideEffectSnapshot: (id: string) => Promise onExit: ( - callback: (data: { id: string; code: number; preserveRendererBinding?: boolean }) => void + callback: (data: { + id: string + code: number + preserveRendererBinding?: boolean + /** Which lifetime of `id` died; absent when the execution host predates the field. */ + incarnationId?: string + }) => void ) => () => void onSpawned: (callback: (data: { id: string }) => void) => () => void onSerializeBufferRequest: ( diff --git a/src/preload/index.ts b/src/preload/index.ts index 1b75b5b56da..5bc62cbfe2e 100644 --- a/src/preload/index.ts +++ b/src/preload/index.ts @@ -1058,6 +1058,8 @@ const api = { telemetry?: { agent_kind: AgentKind; launch_source: LaunchSource; request_kind: RequestKind } }): Promise<{ id: string + /** Which lifetime of `id` this reply named; absent when the execution host predates the field. */ + incarnationId?: string launchConfig?: SleepingAgentLaunchConfig snapshot?: string snapshotCols?: number @@ -1304,10 +1306,23 @@ const api = { ipcRenderer.invoke('pty:sideEffectSnapshot', { id }), onExit: ( - callback: (data: { id: string; code: number; preserveRendererBinding?: boolean }) => void + callback: (data: { + id: string + code: number + preserveRendererBinding?: boolean + /** Which lifetime of `id` died; absent when the execution host predates the field. */ + incarnationId?: string + }) => void ): (() => void) => { - const listener = (_event: Electron.IpcRendererEvent, data: { id: string; code: number }) => - callback(data) + const listener = ( + _event: Electron.IpcRendererEvent, + data: { + id: string + code: number + preserveRendererBinding?: boolean + incarnationId?: string + } + ) => callback(data) ipcRenderer.on('pty:exit', listener) return () => ipcRenderer.removeListener('pty:exit', listener) }, diff --git a/src/renderer/src/components/terminal-pane/ipc-pty-connect.ts b/src/renderer/src/components/terminal-pane/ipc-pty-connect.ts index c734ea6ccee..70d3a0fd047 100644 --- a/src/renderer/src/components/terminal-pane/ipc-pty-connect.ts +++ b/src/renderer/src/components/terminal-pane/ipc-pty-connect.ts @@ -4,6 +4,7 @@ import { ensurePtyDispatcher } from './pty-dispatcher' import { clearConsumedPreHandlerPtyExit, currentPreHandlerPtySequence, + discardPreHandlerPtyExitFromForeignIncarnation, discardPreHandlerPtyStateFromPriorIncarnation, hasPreHandlerPtyExit, isPreHandlerPtyStateDiscarded @@ -93,6 +94,10 @@ export async function connectIpcPty( context.getCallbacks().onReattachDetermined?.() } + // Why unconditional: this runs on identity, not timing. Whatever we attached to — fresh, + // reattach or cold restore — an exit naming a different incarnation of the id is not ours, so + // it is safe to drop even for the reattach the fence below deliberately leaves alone. + discardPreHandlerPtyExitFromForeignIncarnation(spawnResult.id, spawnResult.incarnationId) if (!admittedSessionId && !spawnResult.isReattach && !spawnResult.coldRestore) { // Why only a fresh spawn: a reattach deliberately re-owns an id that already existed, so its // buffered exit is the real thing. A fresh spawn's PTY did not exist yet. @@ -103,7 +108,7 @@ export async function connectIpcPty( onPtySpawn?.(spawnResult.id) } handlers.registerData(spawnResult.id) - const exitedBeforeAttach = handlers.registerExit(spawnResult.id) + const exitedBeforeAttach = handlers.registerExit(spawnResult.id, spawnResult.incarnationId) if (exitedBeforeAttach) { return { id: spawnResult.id, exitedBeforeAttach: true } } diff --git a/src/renderer/src/components/terminal-pane/ipc-pty-session-handlers.ts b/src/renderer/src/components/terminal-pane/ipc-pty-session-handlers.ts index 8d73479687c..ad225b928bb 100644 --- a/src/renderer/src/components/terminal-pane/ipc-pty-session-handlers.ts +++ b/src/renderer/src/components/terminal-pane/ipc-pty-session-handlers.ts @@ -30,7 +30,7 @@ type IpcPtySessionHandlersOptions = { export type IpcPtySessionHandlers = { registerData: (id: string) => void - registerExit: (id: string) => boolean + registerExit: (id: string, incarnationId?: string) => boolean unregisterAll: (id: string) => void unregisterData: (id: string) => void clearAccumulatedState: () => void @@ -133,8 +133,8 @@ export function createIpcPtySessionHandlers({ } } - function registerExit(id: string): boolean { - const hadBufferedExit = hasPreHandlerPtyExit(id) + function registerExit(id: string, incarnationId?: string): boolean { + const hadBufferedExit = hasPreHandlerPtyExit(id, incarnationId) const exit = (code: number): void => { const currentId = getPtyId() if (currentId !== null && currentId !== id) { @@ -154,7 +154,7 @@ export function createIpcPtySessionHandlers({ ptyTeardownHandlers.set(id, clearAccumulatedState) ptyShutdownLifecycleHandlers.set(id, shutdownLifecycle) try { - drainPreHandlerPtyExit(id, exit) + drainPreHandlerPtyExit(id, exit, incarnationId) } catch (error) { if (!hadBufferedExit) { throw error diff --git a/src/renderer/src/components/terminal-pane/ipc-pty-spawn-request.ts b/src/renderer/src/components/terminal-pane/ipc-pty-spawn-request.ts index bef2d22a06f..f6ff1a8e7e2 100644 --- a/src/renderer/src/components/terminal-pane/ipc-pty-spawn-request.ts +++ b/src/renderer/src/components/terminal-pane/ipc-pty-spawn-request.ts @@ -2,11 +2,19 @@ import type { IpcPtyTransportOptions, PtyConnectResult, PtyTransport } from './p type PtyConnectOptions = Parameters[0] +/** `incarnationId` names which lifetime of the returned id this spawn owns; absent when the + * execution host predates the field. It is deliberately NOT on `PtyConnectResult` — only the + * connect handshake needs it, to fence buffered state left by an earlier owner of the same id. */ +export type IpcPtySpawnResponse = PtyConnectResult & { + isReattach?: boolean + incarnationId?: string +} + export async function spawnIpcPty( transportOptions: IpcPtyTransportOptions, connectOptions: PtyConnectOptions, admittedSessionId?: string -): Promise { +): Promise { const { cwd, cwdFallback, @@ -70,5 +78,5 @@ export async function spawnIpcPty( ...(projectRuntime ? { projectRuntime } : {}), ...(terminalColorQueryReplies ? { terminalColorQueryReplies } : {}), ...(telemetry ? { telemetry } : {}) - }) as Promise + }) as Promise } diff --git a/src/renderer/src/components/terminal-pane/pty-dispatcher.ts b/src/renderer/src/components/terminal-pane/pty-dispatcher.ts index 73f756c0857..ccb38ae2f55 100644 --- a/src/renderer/src/components/terminal-pane/pty-dispatcher.ts +++ b/src/renderer/src/components/terminal-pane/pty-dispatcher.ts @@ -203,6 +203,9 @@ function attachPtySecondaryPushListeners(unsubscribes: (() => void)[]): void { deliverPtyExitToHandlers({ ptyId: payload.id, code: payload.code, + // Why forwarded: pty ids are reused, so a buffered exit needs the lifetime it describes to + // tell "this pane's shell died" from "the id's previous owner died" (#16970). + ...(payload.incarnationId ? { incarnationId: payload.incarnationId } : {}), ...(primary ? { primary } : {}), sidecars: sidecars ? Array.from(sidecars) : [] }) @@ -258,9 +261,13 @@ export function getEagerPtyBufferHandle(ptyId: string): EagerPtyHandle | undefin // Why: cap matches TerminalPane's scrollback serialization limit so a restored shell (e.g. tail -f) can't grow unbounded. const EAGER_BUFFER_MAX_BYTES = TERMINAL_SCROLLBACK_SESSION_BUFFER_BYTE_LIMIT +/** `incarnationId` names the lifetime the caller just spawned. Without it a background launch that + * is handed a relay-recycled id drains whatever the id's PREVIOUS owner left here and tears its own + * freshly started agent session down seconds after launch. */ export function registerEagerPtyBuffer( ptyId: string, - onExit: (ptyId: string, code: number) => void + onExit: (ptyId: string, code: number) => void, + incarnationId?: string ): EagerPtyHandle { ensurePtyDispatcher() // Why: head index instead of Array.shift() (O(n)) so pre-attach buffering isn't quadratic under many small chunks. @@ -328,7 +335,7 @@ export function registerEagerPtyBuffer( // Why: defer the pre-handler exit one microtask so the caller receives the returned handle before onExit fires. queueMicrotask(() => { if (ptyExitHandlers.get(ptyId) === exitHandler) { - drainPreHandlerPtyExit(ptyId, exitHandler) + drainPreHandlerPtyExit(ptyId, exitHandler, incarnationId) } else { clearPreHandlerPtyState(ptyId) } diff --git a/src/renderer/src/components/terminal-pane/pty-exit-delivery.ts b/src/renderer/src/components/terminal-pane/pty-exit-delivery.ts index 9180aabf890..a01e09c2d8f 100644 --- a/src/renderer/src/components/terminal-pane/pty-exit-delivery.ts +++ b/src/renderer/src/components/terminal-pane/pty-exit-delivery.ts @@ -7,6 +7,8 @@ import { type PtyExitDelivery = { ptyId: string code: number + /** Which lifetime of `ptyId` died. Absent when the execution host predates the field. */ + incarnationId?: string primary?: (code: number) => void sidecars: readonly ((code: number, context: { hadPrimary: boolean }) => void)[] } @@ -26,7 +28,7 @@ export function deliverPtyExitToHandlers(delivery: PtyExitDelivery): void { consumePreHandlerPtyState(delivery.ptyId) } } else { - bufferPreHandlerPtyExit(delivery.ptyId, delivery.code) + bufferPreHandlerPtyExit(delivery.ptyId, delivery.code, delivery.incarnationId) } } catch (error) { firstError = error diff --git a/src/renderer/src/components/terminal-pane/pty-pre-handler-buffer.test.ts b/src/renderer/src/components/terminal-pane/pty-pre-handler-buffer.test.ts index 489b2f3cd24..a8f4a706a89 100644 --- a/src/renderer/src/components/terminal-pane/pty-pre-handler-buffer.test.ts +++ b/src/renderer/src/components/terminal-pane/pty-pre-handler-buffer.test.ts @@ -8,6 +8,7 @@ import { currentPreHandlerPtySequence, drainPreHandlerPtyData, drainPreHandlerPtyExit, + discardPreHandlerPtyExitFromForeignIncarnation, discardPreHandlerPtyState, discardPreHandlerPtyStateFromPriorIncarnation, hasPreHandlerPtyExit, @@ -19,6 +20,10 @@ const TRIM_PTY_ID = 'pty-pre-handler-trim' const EXIT_PTY_ID = 'pty-pre-handler-exit' const CAPPED_EXIT_PTY_IDS = Array.from({ length: 65 }, (_, index) => `pty-capped-exit-${index}`) const RECYCLED_PTY_ID = 'ssh:target@@pty-2' +// Two lifetimes of the same relay-renumbered id: the shell that died while the transport was down, +// and the one the fresh spawn just got handed. +const PRIOR_INCARNATION_ID = 'incarnation-before-the-relay-restarted' +const FRESH_INCARNATION_ID = 'incarnation-of-the-shell-now-attaching' describe('pre-handler PTY buffer', () => { afterEach(() => { @@ -228,6 +233,121 @@ describe('pre-handler PTY buffer', () => { expect(data).toHaveBeenCalledWith('startup bytes', undefined) }) + // The case the sequence fence structurally cannot reach: the stale exit is recorded AFTER the + // renderer asked for a fresh PTY, so it is newer than the fence and passes it. Only the + // incarnation says the exit describes a lifetime of the id that ended before this one began. + it("drops a recycled id's exit that arrived after the spawn request, on incarnation alone", () => { + const fence = currentPreHandlerPtySequence() + bufferPreHandlerPtyExit(RECYCLED_PTY_ID, 0, PRIOR_INCARNATION_ID) + + discardPreHandlerPtyExitFromForeignIncarnation(RECYCLED_PTY_ID, FRESH_INCARNATION_ID) + discardPreHandlerPtyStateFromPriorIncarnation(RECYCLED_PTY_ID, fence) + + const exit = vi.fn() + expect(hasPreHandlerPtyExit(RECYCLED_PTY_ID)).toBe(false) + drainPreHandlerPtyExit(RECYCLED_PTY_ID, exit) + expect(exit).not.toHaveBeenCalled() + }) + + it('keeps a post-fence exit from the incarnation that is attaching, so an instantly dead shell still reports', () => { + const fence = currentPreHandlerPtySequence() + bufferPreHandlerPtyExit(RECYCLED_PTY_ID, 1, FRESH_INCARNATION_ID) + + discardPreHandlerPtyExitFromForeignIncarnation(RECYCLED_PTY_ID, FRESH_INCARNATION_ID) + discardPreHandlerPtyStateFromPriorIncarnation(RECYCLED_PTY_ID, fence) + + const exit = vi.fn() + drainPreHandlerPtyExit(RECYCLED_PTY_ID, exit) + expect(exit).toHaveBeenCalledWith(1) + }) + + // Absence is unknown, never a mismatch: a host that predates the field, and the relay's own + // `{ id, code: -1 }` stale-PTY drop, must keep the fence's behaviour exactly. + it('never discards on incarnation when either side is unnamed', () => { + bufferPreHandlerPtyExit(RECYCLED_PTY_ID, 2) + discardPreHandlerPtyExitFromForeignIncarnation(RECYCLED_PTY_ID, FRESH_INCARNATION_ID) + expect(hasPreHandlerPtyExit(RECYCLED_PTY_ID)).toBe(true) + drainPreHandlerPtyExit(RECYCLED_PTY_ID, vi.fn()) + + clearConsumedPreHandlerPtyExit(RECYCLED_PTY_ID) + bufferPreHandlerPtyExit(RECYCLED_PTY_ID, 3, PRIOR_INCARNATION_ID) + discardPreHandlerPtyExitFromForeignIncarnation(RECYCLED_PTY_ID, undefined) + expect(hasPreHandlerPtyExit(RECYCLED_PTY_ID)).toBe(true) + }) + + // The identity rule is about exits only. `pty:data` carries no incarnation, so buffered bytes + // stay on the sequence fence and must survive a discard that rejects an exit beside them. + it('leaves buffered bytes alone when it discards a foreign exit', () => { + bufferPreHandlerPtyExit(RECYCLED_PTY_ID, 0, PRIOR_INCARNATION_ID) + bufferPreHandlerPtyData(RECYCLED_PTY_ID, 'startup bytes') + + discardPreHandlerPtyExitFromForeignIncarnation(RECYCLED_PTY_ID, FRESH_INCARNATION_ID) + + const data = vi.fn() + expect(hasPreHandlerPtyExit(RECYCLED_PTY_ID)).toBe(false) + drainPreHandlerPtyData(RECYCLED_PTY_ID, data) + expect(data).toHaveBeenCalledWith('startup bytes', undefined) + }) + + // The intersection of the two races this buffer exists for: the shell we just spawned dies before + // the pane attaches, AND the relay flushes the previous owner's exit for the same recycled id + // afterwards. Keyed on the id alone, the stranger overwrites our own exit and the identity discard + // then removes the only survivor — a pane bound to a PTY that is dead and never reported dead. + it("keeps our own pre-attach exit when a late stranger's exit lands on the same recycled id", () => { + const fence = currentPreHandlerPtySequence() + bufferPreHandlerPtyExit(RECYCLED_PTY_ID, 1, FRESH_INCARNATION_ID) + bufferPreHandlerPtyExit(RECYCLED_PTY_ID, 0, PRIOR_INCARNATION_ID) + + discardPreHandlerPtyExitFromForeignIncarnation(RECYCLED_PTY_ID, FRESH_INCARNATION_ID) + discardPreHandlerPtyStateFromPriorIncarnation(RECYCLED_PTY_ID, fence) + + const exit = vi.fn() + expect(hasPreHandlerPtyExit(RECYCLED_PTY_ID)).toBe(true) + drainPreHandlerPtyExit(RECYCLED_PTY_ID, exit) + expect(exit).toHaveBeenCalledWith(1) + }) + + it('replaces a duplicate exit for the same lifetime instead of crowding out another lifetime', () => { + bufferPreHandlerPtyExit(RECYCLED_PTY_ID, 1, FRESH_INCARNATION_ID) + bufferPreHandlerPtyExit(RECYCLED_PTY_ID, 9, FRESH_INCARNATION_ID) + bufferPreHandlerPtyExit(RECYCLED_PTY_ID, 0, PRIOR_INCARNATION_ID) + + discardPreHandlerPtyExitFromForeignIncarnation(RECYCLED_PTY_ID, FRESH_INCARNATION_ID) + + const exit = vi.fn() + drainPreHandlerPtyExit(RECYCLED_PTY_ID, exit) + expect(exit).toHaveBeenCalledWith(9) + }) + + // Reads are filtered by lifetime inside the buffer, so a consumer that never calls the discard — + // a background launch registering an eager buffer straight off its own spawn — still cannot be + // handed the previous owner's exit and tear its freshly started session down. + it('never reports or delivers a foreign exit to a reader that names its lifetime', () => { + bufferPreHandlerPtyExit(RECYCLED_PTY_ID, 0, PRIOR_INCARNATION_ID) + + const exit = vi.fn() + expect(hasPreHandlerPtyExit(RECYCLED_PTY_ID, FRESH_INCARNATION_ID)).toBe(false) + drainPreHandlerPtyExit(RECYCLED_PTY_ID, exit, FRESH_INCARNATION_ID) + expect(exit).not.toHaveBeenCalled() + + // A reader with no incarnation still sees it: absence is unknown, so it has nothing to judge by. + expect(hasPreHandlerPtyExit(RECYCLED_PTY_ID)).toBe(true) + }) + + // A malformed incarnation is evidence of nothing. Treating it as a value that disagrees with + // everything would discard the very exits the buffer exists to deliver. + it('treats a malformed incarnation as unknown rather than as a disagreement', () => { + bufferPreHandlerPtyExit(RECYCLED_PTY_ID, 4, { not: 'a string' }) + discardPreHandlerPtyExitFromForeignIncarnation(RECYCLED_PTY_ID, FRESH_INCARNATION_ID) + expect(hasPreHandlerPtyExit(RECYCLED_PTY_ID)).toBe(true) + drainPreHandlerPtyExit(RECYCLED_PTY_ID, vi.fn()) + + clearConsumedPreHandlerPtyExit(RECYCLED_PTY_ID) + bufferPreHandlerPtyExit(RECYCLED_PTY_ID, 5, PRIOR_INCARNATION_ID) + discardPreHandlerPtyExitFromForeignIncarnation(RECYCLED_PTY_ID, 42) + expect(hasPreHandlerPtyExit(RECYCLED_PTY_ID)).toBe(true) + }) + it('re-admits exits for a recycled id whose prior incarnation was consumed', () => { consumePreHandlerPtyState(RECYCLED_PTY_ID) diff --git a/src/renderer/src/components/terminal-pane/pty-pre-handler-buffer.ts b/src/renderer/src/components/terminal-pane/pty-pre-handler-buffer.ts index 9e2f4d3e929..bbe4b783ee9 100644 --- a/src/renderer/src/components/terminal-pane/pty-pre-handler-buffer.ts +++ b/src/renderer/src/components/terminal-pane/pty-pre-handler-buffer.ts @@ -1,3 +1,4 @@ +import { isPtyIncarnationId } from '../../../../shared/pty-incarnation' import { clampUtf8Tail } from './pty-eager-buffer-clamp' import type { PtyDataMeta } from './pty-dispatcher' @@ -18,10 +19,16 @@ type BufferedPreHandlerPtyState = { type BufferedPreHandlerPtyExit = { code: number sequence: number + /** Which lifetime of `ptyId` died. Absent when the emitting host predates the field. */ + incarnationId?: string } const preHandlerPtyData = new Map() -const preHandlerPtyExit = new Map() +// Why one record per lifetime and not one per id: a recycled id can have its previous lifetime's +// exit still in flight while the lifetime that just replaced it also dies pre-attach. With a single +// slot the late stranger overwrites the real exit, and the identity discard below then removes the +// only survivor — leaving a pane bound to a PTY that is dead and will never be reported dead. +const preHandlerPtyExit = new Map() const consumedPreHandlerPtyExits = new Map() const discardedPreHandlerPtyStates = new Map>() const DISCARDED_PRE_HANDLER_PTY_STATE_TTL_MS = 60_000 @@ -32,6 +39,9 @@ const DISCARDED_PRE_HANDLER_PTY_STATE_TTL_MS = 60_000 const PRE_HANDLER_PTY_DATA_MAX_BYTES = 512 * 1024 const PRE_HANDLER_PTY_DATA_MAX_PTYS = 64 const PRE_HANDLER_PTY_EXIT_MAX_PTYS = 64 +// Why small: only lifetimes racing the same pre-attach window can coexist, which in practice is the +// outgoing one and the incoming one. +const PRE_HANDLER_PTY_EXIT_MAX_INCARNATIONS_PER_PTY = 4 // 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. @@ -53,6 +63,64 @@ export function currentPreHandlerPtySequence(): number { return preHandlerPtySequence } +/** Map preserves insertion order, so the first key is the least recently admitted id. */ +function evictOldestPtyIfAtCap(map: Map, ptyId: string, cap: number): void { + if (map.has(ptyId) || map.size < cap) { + return + } + const oldestPtyId = map.keys().next().value + if (typeof oldestPtyId === 'string') { + map.delete(oldestPtyId) + } +} + +/** Drop a buffered exit proven to describe a different lifetime of `ptyId` than the one now + * attaching, whenever it arrived. + * + * Why identity and not the sequence fence below: a fence is a clock, so it can only reject records + * dated before the spawn request left the renderer. An exit from the id's previous owner that + * arrives AFTER that — a relay that restarted mid-spawn and flushed it late — passes the fence and + * reports a brand-new shell as already dead. The incarnation says whose lifetime the exit + * describes, so it settles that case on evidence rather than timing. + * + * Only a positive disagreement discards: both sides must name an incarnation. Absence is + * "unknown", never a mismatch, so an execution host that predates the field keeps the fence's + * behaviour exactly. Safe on a reattach and a cold restore too — those deliberately re-own an + * existing id, and re-owning incarnation X is still proof that W's exit was not theirs. */ +export function discardPreHandlerPtyExitFromForeignIncarnation( + ptyId: string, + incarnationId: unknown +): void { + // Why the shared guard and not a truthiness check: anything that is not a well-formed incarnation + // is evidence of nothing, and must read as "unknown" rather than disagree with everything. + if (!isPtyIncarnationId(incarnationId)) { + return + } + retainPreHandlerPtyExits( + ptyId, + (exit) => exit.incarnationId === undefined || exit.incarnationId === incarnationId + ) +} + +function retainPreHandlerPtyExits( + ptyId: string, + keep: (exit: BufferedPreHandlerPtyExit) => boolean +): void { + const exits = preHandlerPtyExit.get(ptyId) + if (!exits) { + return + } + const kept = exits.filter(keep) + if (kept.length === exits.length) { + return + } + if (kept.length === 0) { + preHandlerPtyExit.delete(ptyId) + return + } + preHandlerPtyExit.set(ptyId, kept) +} + export function bufferPreHandlerPtyData(ptyId: string, data: string, meta?: PtyDataMeta): void { if (discardedPreHandlerPtyStates.has(ptyId)) { return @@ -61,12 +129,7 @@ export function bufferPreHandlerPtyData(ptyId: string, data: string, meta?: PtyD if (!chunk.data) { return } - if (!preHandlerPtyData.has(ptyId) && preHandlerPtyData.size >= PRE_HANDLER_PTY_DATA_MAX_PTYS) { - const oldestPtyId = preHandlerPtyData.keys().next().value - if (typeof oldestPtyId === 'string') { - preHandlerPtyData.delete(oldestPtyId) - } - } + evictOldestPtyIfAtCap(preHandlerPtyData, ptyId, PRE_HANDLER_PTY_DATA_MAX_PTYS) const bufferedMeta = meta && chunk.data.length !== data.length && typeof meta.rawLength === 'number' ? { ...meta, rawLength: chunk.bytes } @@ -130,17 +193,37 @@ export function replayPreHandlerPtyData(ptyId: string, observer: (data: string) } } -export function bufferPreHandlerPtyExit(ptyId: string, code: number): void { +export function bufferPreHandlerPtyExit( + ptyId: string, + code: number, + incarnationId?: unknown +): void { if (consumedPreHandlerPtyExits.has(ptyId) || discardedPreHandlerPtyStates.has(ptyId)) { return } - if (!preHandlerPtyExit.has(ptyId) && preHandlerPtyExit.size >= PRE_HANDLER_PTY_EXIT_MAX_PTYS) { - const oldestPtyId = preHandlerPtyExit.keys().next().value - if (typeof oldestPtyId === 'string') { - preHandlerPtyExit.delete(oldestPtyId) - } + evictOldestPtyIfAtCap(preHandlerPtyExit, ptyId, PRE_HANDLER_PTY_EXIT_MAX_PTYS) + const exit: BufferedPreHandlerPtyExit = { + code, + sequence: nextPreHandlerPtySequence(), + // Record only a well-formed incarnation; a malformed one must not become evidence. + ...(isPtyIncarnationId(incarnationId) ? { incarnationId } : {}) + } + const exits = preHandlerPtyExit.get(ptyId) + if (!exits) { + preHandlerPtyExit.set(ptyId, [exit]) + return + } + // A duplicate exit for a lifetime replaces that lifetime's record rather than crowding out + // another one's; unnamed records share the single `undefined` slot, as they did before. + const sameLifetime = exits.findIndex((entry) => entry.incarnationId === exit.incarnationId) + if (sameLifetime !== -1) { + exits[sameLifetime] = exit + return + } + exits.push(exit) + if (exits.length > PRE_HANDLER_PTY_EXIT_MAX_INCARNATIONS_PER_PTY) { + exits.shift() } - preHandlerPtyExit.set(ptyId, { code, sequence: nextPreHandlerPtySequence() }) } /** Drop pre-handler state a freshly spawned PTY inherited from an earlier owner of its id. @@ -148,15 +231,18 @@ export function bufferPreHandlerPtyExit(ptyId: string, code: number): void { * `fenceSequence` is read before the spawn request leaves the renderer, so anything at or below it * was recorded when this PTY did not yet exist and cannot describe it. Bytes and exits recorded * after the fence are kept: that is the real pre-attach race (a shell that dies instantly, or - * writes before the pane registers its handler) and losing it would blank a legitimate pane. */ + * writes before the pane registers its handler) and losing it would blank a legitimate pane. + * + * Still needed alongside `discardPreHandlerPtyExitFromForeignIncarnation`, which supersedes it + * wherever both sides name an incarnation. Two cases have none to compare and rest on the fence + * alone: buffered BYTES, because `pty:data` carries no incarnation at all; and exits from an + * execution host that predates the field or from a main-side path that synthesizes one without it + * (a relay dropping a stale PTY after a failed reattach sends `{ id, code: -1 }`). */ export function discardPreHandlerPtyStateFromPriorIncarnation( ptyId: string, fenceSequence: number ): void { - const exit = preHandlerPtyExit.get(ptyId) - if (exit && exit.sequence <= fenceSequence) { - preHandlerPtyExit.delete(ptyId) - } + retainPreHandlerPtyExits(ptyId, (exit) => exit.sequence > fenceSequence) const data = preHandlerPtyData.get(ptyId) if (data && data.sequence <= fenceSequence) { preHandlerPtyData.delete(ptyId) @@ -215,12 +301,42 @@ export function discardPreHandlerPtyState(ptyId: string): void { discardedPreHandlerPtyStates.set(ptyId, timer) } -export function hasPreHandlerPtyExit(ptyId: string): boolean { - return preHandlerPtyExit.has(ptyId) +/** The records for `ptyId` that could describe `incarnationId`'s lifetime. + * + * Every read of the exit buffer goes through here, so a record proven to belong to a different + * lifetime is unreachable BY CONSTRUCTION rather than because each caller remembered to discard + * first. A caller that cannot name an incarnation — a reattach that has not round-tripped yet — + * still sees everything, which is the honest answer: it holds no evidence to discriminate with. */ +function admissiblePreHandlerPtyExits( + ptyId: string, + incarnationId: unknown +): BufferedPreHandlerPtyExit[] { + const exits = preHandlerPtyExit.get(ptyId) ?? [] + if (!isPtyIncarnationId(incarnationId)) { + return exits + } + return exits.filter( + (exit) => exit.incarnationId === undefined || exit.incarnationId === incarnationId + ) } -export function drainPreHandlerPtyExit(ptyId: string, handler: (code: number) => void): void { - const exit = preHandlerPtyExit.get(ptyId) +export function hasPreHandlerPtyExit(ptyId: string, incarnationId?: unknown): boolean { + return admissiblePreHandlerPtyExits(ptyId, incarnationId).length > 0 +} + +export function drainPreHandlerPtyExit( + ptyId: string, + handler: (code: number) => void, + incarnationId?: unknown +): void { + // Newest admissible record: picking by sequence keeps the last-write-wins delivery a single-slot + // buffer always had, now scoped to the lifetime actually asking. + let exit: BufferedPreHandlerPtyExit | undefined + for (const candidate of admissiblePreHandlerPtyExits(ptyId, incarnationId)) { + if (!exit || candidate.sequence > exit.sequence) { + exit = candidate + } + } if (exit === undefined) { return } diff --git a/src/renderer/src/components/terminal-pane/pty-transport-recycled-pty-incarnation.test.ts b/src/renderer/src/components/terminal-pane/pty-transport-recycled-pty-incarnation.test.ts new file mode 100644 index 00000000000..7936300fb06 --- /dev/null +++ b/src/renderer/src/components/terminal-pane/pty-transport-recycled-pty-incarnation.test.ts @@ -0,0 +1,97 @@ +/** A restarted SSH relay renumbers PTYs from pty-1, so a fresh spawn is routinely handed an id + * whose previous shell is still emitting a late exit. Applying that exit to the new shell reports + * it as already dead (`exitedBeforeAttach`), the pane never binds a PTY, and the tab is blank and + * unusable forever — the runtime-proven failure behind #16970. + * + * #16970 dated every buffered record and dropped anything older than the spawn request. That fence + * is a clock, so it cannot judge a stale exit that arrives AFTER the request left. These specs pin + * the identity rule that can: the incarnation names which lifetime of the id died. */ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { + installIpcPtyWindow, + restorePtySpecWindow, + type PtyExitPayload +} from './pty-transport-test-harness' + +const RECYCLED_PTY_ID = 'ssh:target@@pty-1' +const PRIOR_INCARNATION_ID = 'incarnation-before-the-relay-restarted' +const FRESH_INCARNATION_ID = 'incarnation-of-the-shell-now-attaching' + +describe('createIpcPtyTransport against a relay-recycled PTY id', () => { + const originalWindow = (globalThis as { window?: typeof window }).window + let onExit: ((payload: PtyExitPayload) => void) | null = null + + beforeEach(() => { + vi.resetModules() + onExit = null + installIpcPtyWindow(originalWindow, { + exit: (callback) => { + onExit = callback + } + }) + }) + + afterEach(() => { + restorePtySpecWindow(originalWindow) + }) + + /** Deliver `exit` while the spawn request is in flight, then settle the spawn. + * + * Firing from inside the spawn mock makes "after the request left the renderer" *definitional* + * rather than microtask-scheduled: the fence is read before `spawnIpcPty` is called, so an exit + * buffered during the call is always newer than the fence. That is the whole point — the fence + * keeps this record, and only the incarnation can reject it. */ + async function connectWithExitDuringSpawn( + exit: PtyExitPayload, + spawnResponse: { id: string; incarnationId?: string } + ): Promise<{ result: unknown; paneExit: ReturnType }> { + const { createIpcPtyTransport } = await import('./pty-transport') + const spawn = window.api.pty.spawn as unknown as ReturnType + spawn.mockImplementationOnce(() => { + onExit?.(exit) + return Promise.resolve(spawnResponse) + }) + const paneExit = vi.fn() + const transport = createIpcPtyTransport({}) + + const result = await transport.connect({ url: '', callbacks: { onExit: paneExit } }) + return { result, paneExit } + } + + it('does not report a fresh shell as exited when the id’s previous incarnation exits mid-spawn', async () => { + const { result, paneExit } = await connectWithExitDuringSpawn( + { id: RECYCLED_PTY_ID, code: 0, incarnationId: PRIOR_INCARNATION_ID }, + { id: RECYCLED_PTY_ID, incarnationId: FRESH_INCARNATION_ID } + ) + + // Positive, not just "not dead": a bare `not.toMatchObject` would also pass on a bailed-out + // connect that returned undefined. A healthy fresh spawn resolves to its pty id. + expect(result).toBe(RECYCLED_PTY_ID) + expect(paneExit).not.toHaveBeenCalled() + }) + + // The reason the pre-handler buffer exists: a shell that dies before the pane registers its exit + // handler must still be reported, or the pane hangs on a PTY that is already gone. + it('still reports an exit from the incarnation it actually attached to', async () => { + const { result, paneExit } = await connectWithExitDuringSpawn( + { id: RECYCLED_PTY_ID, code: 3, incarnationId: FRESH_INCARNATION_ID }, + { id: RECYCLED_PTY_ID, incarnationId: FRESH_INCARNATION_ID } + ) + + expect(result).toMatchObject({ id: RECYCLED_PTY_ID, exitedBeforeAttach: true }) + expect(paneExit).toHaveBeenCalledWith(3) + }) + + // Absence is unknown, never a mismatch — so an SSH host predating the field, and the relay's own + // unnamed `{ id, code: -1 }` drop, keep exactly the behaviour #16970 shipped. (`remote:` runtime + // PTYs are not covered by this: their exits never traverse `pty:exit` at all.) + it('never discards on incarnation when the host names none', async () => { + const { result, paneExit } = await connectWithExitDuringSpawn( + { id: RECYCLED_PTY_ID, code: 3 }, + { id: RECYCLED_PTY_ID } + ) + + expect(result).toMatchObject({ id: RECYCLED_PTY_ID, exitedBeforeAttach: true }) + expect(paneExit).toHaveBeenCalledWith(3) + }) +}) diff --git a/src/renderer/src/components/terminal-pane/pty-transport-test-harness.ts b/src/renderer/src/components/terminal-pane/pty-transport-test-harness.ts index 0937830959c..727327aa811 100644 --- a/src/renderer/src/components/terminal-pane/pty-transport-test-harness.ts +++ b/src/renderer/src/components/terminal-pane/pty-transport-test-harness.ts @@ -1,7 +1,13 @@ import { vi } from 'vitest' export type PtyStreamPayload = { id: string; data: string } -export type PtyExitPayload = { id: string; code: number; preserveRendererBinding?: boolean } +export type PtyExitPayload = { + id: string + code: number + preserveRendererBinding?: boolean + /** Which lifetime of `id` died; absent when the execution host predates the field. */ + incarnationId?: string +} /** Sinks let each spec own its `onData`/`onExit` bindings, so test bodies keep calling them directly. */ export type PtyListenerSinks = { diff --git a/src/renderer/src/lib/launch-agent-background-session.test.ts b/src/renderer/src/lib/launch-agent-background-session.test.ts index eedd1e9fb32..42df8332882 100644 --- a/src/renderer/src/lib/launch-agent-background-session.test.ts +++ b/src/renderer/src/lib/launch-agent-background-session.test.ts @@ -92,6 +92,7 @@ describe('launchAgentBackgroundSession', () => { it('spawns a PTY first and creates the inactive tab already bound to it', async () => { const { launchAgentBackgroundSession } = await import('./launch-agent-background-session') + mockSpawn.mockResolvedValue({ id: 'pty-1', incarnationId: 'inc-fresh' }) const result = await launchAgentBackgroundSession({ agent: 'claude', @@ -160,7 +161,13 @@ describe('launchAgentBackgroundSession', () => { recordInteraction: false }) expect(mockUpdateTabPtyId).toHaveBeenCalledWith(tabId, 'pty-1') - expect(mockRegisterEagerPtyBuffer).toHaveBeenCalledWith('pty-1', expect.any(Function)) + // The incarnation rides along so a relay-recycled id cannot drain the previous owner's exit + // into this handler and tear the session down right after launch. + expect(mockRegisterEagerPtyBuffer).toHaveBeenCalledWith( + 'pty-1', + expect.any(Function), + 'inc-fresh' + ) expect(mockSubscribeToPtyData).toHaveBeenCalledWith('pty-1', expect.any(Function)) expect(mockSubscribeToPtyExit).toHaveBeenCalledWith('pty-1', expect.any(Function)) expect(result).toMatchObject({ tabId, paneKey, ptyId: 'pty-1' }) diff --git a/src/renderer/src/lib/launch-agent-background-session.ts b/src/renderer/src/lib/launch-agent-background-session.ts index 947cabb317b..a1d9f1df654 100644 --- a/src/renderer/src/lib/launch-agent-background-session.ts +++ b/src/renderer/src/lib/launch-agent-background-session.ts @@ -127,7 +127,9 @@ export async function launchAgentBackgroundSession( ) let ptyId = '', runtimeTerminalHandle: string | null = null - let returnedLaunchConfig: typeof startupPlan.launchConfig | undefined + // What the local spawn answered and later steps still need: which lifetime of `ptyId` this launch + // owns, and the config the host actually launched. Both absent for a runtime terminal. + let spawned: { incarnationId?: string; launchConfig?: typeof startupPlan.launchConfig } = {} let tab: ReturnType | null = null let exitHandled = false, eagerPtyBuffer: EagerPtyHandle | null = null @@ -219,7 +221,7 @@ export async function launchAgentBackgroundSession( } }) ptyId = result.id - returnedLaunchConfig = result.launchConfig + spawned = result } const adopted = await adoptAgentBackgroundSessionTab({ store, @@ -227,7 +229,7 @@ export async function launchAgentBackgroundSession( reservedTabId, ptyId, paneKey, - launchConfig: returnedLaunchConfig ?? startupPlan.launchConfig, + launchConfig: spawned.launchConfig ?? startupPlan.launchConfig, launchRegistration, runtimeTarget, runtimeTerminalHandle, @@ -280,7 +282,9 @@ export async function launchAgentBackgroundSession( .then((result) => handleExit(ptyId, result.wait.exitCode ?? 0)) .catch(() => {}) } else { - eagerPtyBuffer = registerEagerPtyBuffer(ptyId, handleExit) + // Why the incarnation: a relay-recycled id can hold the previous owner's exit, and draining + // that into this handler tears the agent session down seconds after it launched. + eagerPtyBuffer = registerEagerPtyBuffer(ptyId, handleExit, spawned.incarnationId) unsubscribeData = subscribeToPtyData(ptyId, handleData) // Why: opening the workspace attaches a real terminal transport and disposes // the eager exit handler. This sidecar keeps automation completion tracking diff --git a/src/renderer/src/lib/launch-worktree-background-terminals.ts b/src/renderer/src/lib/launch-worktree-background-terminals.ts index 049176c1e81..791504d6948 100644 --- a/src/renderer/src/lib/launch-worktree-background-terminals.ts +++ b/src/renderer/src/lib/launch-worktree-background-terminals.ts @@ -113,12 +113,15 @@ function persistExitedPaneOutput(tabId: string, leafId: string, output: string): }) } -function registerBackgroundPaneBuffer(tabId: string, leafId: string, ptyId: string): void { +// Why the incarnation: a relay-recycled id can hold the previous owner's exit, and draining that +// into this handler tears the pane down seconds after it launched. +function registerBackgroundPaneBuffer(tabId: string, leafId: string, pane: SpawnedPane): void { let eagerBuffer: EagerPtyHandle | null = null - eagerBuffer = registerEagerPtyBuffer(ptyId, (exitPtyId) => { + const onExit = (exitPtyId: string): void => { persistExitedPaneOutput(tabId, leafId, eagerBuffer?.flush() ?? '') useAppStore.getState().clearTabPtyId(tabId, exitPtyId) - }) + } + eagerBuffer = registerEagerPtyBuffer(pane.ptyId, onExit, pane.incarnationId) } function buildSetupCommand(setup: WorktreeSetupLaunch): string { @@ -130,6 +133,9 @@ function buildSetupCommand(setup: WorktreeSetupLaunch): string { ) } +/** The id a background pane got, plus which lifetime of it this spawn owns. */ +type SpawnedPane = { ptyId: string; incarnationId?: string } + async function spawnPane(args: { worktree: Worktree connectionId: string | null @@ -137,7 +143,7 @@ async function spawnPane(args: { leafId: string command?: string env?: Record -}): Promise { +}): Promise { const result = await window.api.pty.spawn({ cols: 120, rows: 40, @@ -149,7 +155,10 @@ async function spawnPane(args: { tabId: args.tabId, leafId: args.leafId }) - return result.id + return { + ptyId: result.id, + ...(result.incarnationId ? { incarnationId: result.incarnationId } : {}) + } } async function createBackgroundTab(args: { @@ -171,9 +180,9 @@ async function createBackgroundTab(args: { const leafId = createBrowserUuid() store.setTabLayout(tab.id, singlePaneLayoutSnapshot(leafId)) - let ptyId: string + let pane: SpawnedPane try { - ptyId = await spawnPane({ + pane = await spawnPane({ worktree: args.worktree, connectionId: args.connectionId, tabId: tab.id, @@ -188,16 +197,16 @@ async function createBackgroundTab(args: { if ( await retireUnownedTerminal({ owner: { tabId: tab.id }, - ptyId, + ptyId: pane.ptyId, runtimeTarget: { kind: 'local' } }) ) { throw new Error('The terminal tab was closed before its session finished starting.') } - store.updateTabPtyId(tab.id, ptyId) - store.setTabLayout(tab.id, singlePaneLayoutSnapshot(leafId, ptyId)) - registerBackgroundPaneBuffer(tab.id, leafId, ptyId) - return { tabId: tab.id, primary: { leafId, ptyId } } + store.updateTabPtyId(tab.id, pane.ptyId) + store.setTabLayout(tab.id, singlePaneLayoutSnapshot(leafId, pane.ptyId)) + registerBackgroundPaneBuffer(tab.id, leafId, pane) + return { tabId: tab.id, primary: { leafId, ptyId: pane.ptyId } } } async function addSetupSplit(args: { @@ -209,7 +218,7 @@ async function addSetupSplit(args: { }): Promise { const store = useAppStore.getState() const setupLeafId = createBrowserUuid() - const setupPtyId = await spawnPane({ + const setupPane = await spawnPane({ worktree: args.worktree, connectionId: args.connectionId, tabId: args.tab.tabId, @@ -220,23 +229,23 @@ async function addSetupSplit(args: { if ( await retireUnownedTerminal({ owner: { tabId: args.tab.tabId }, - ptyId: setupPtyId, + ptyId: setupPane.ptyId, runtimeTarget: { kind: 'local' } }) ) { return } - store.updateTabPtyId(args.tab.tabId, setupPtyId) + store.updateTabPtyId(args.tab.tabId, setupPane.ptyId) store.setTabLayout( args.tab.tabId, buildSplitLayout( args.tab.primary, - { leafId: setupLeafId, ptyId: setupPtyId }, + { leafId: setupLeafId, ptyId: setupPane.ptyId }, args.direction, getSetupTabTitle() ) ) - registerBackgroundPaneBuffer(args.tab.tabId, setupLeafId, setupPtyId) + registerBackgroundPaneBuffer(args.tab.tabId, setupLeafId, setupPane) } function getDefaultTabLaunches(