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.
This commit is contained in:
Neil
2026-08-28 03:25:09 -07:00
parent 0d18648b40
commit 6cb28c0d29
9 changed files with 101 additions and 39 deletions
+4 -4
View File
@@ -3333,7 +3333,7 @@
"durationSeconds": 1.5,
"summary": "175/175 at the relocated paths on the unified tool authority, replacing the 2026-08-27 record of the same command at 174/174. That run killed twelve of fourteen mutants as sole deltas — dropping the document branch from the one authority door, removing either browsing-registration refusal or the mint-side refusal that is its converse, deleting the registration notification or widening it to the worktree-wide and any-tab waits, disposing revoked state by grant instead of by page, dropping the hosting-renderer or destroyed-contents check, rebuilding the renderer tool target from the grant, and deleting the teardown identity check under a re-mint — and reported two as ill-posed rather than as kills. The relocation changed no file it names except their directory."
},
{
{
"date": "2026-08-27",
"runner": "local",
"platform": "macos",
@@ -3378,7 +3378,7 @@
"durationSeconds": 0.17,
"summary": "22/22 on the candidate that stops a previewed document writing into this desktop's Downloads folder. The hole was read out of the shipping path, not guessed: the preview partition took the shared browser partition policies whole, including will-download, and a preview guest joins no browser-page registry, so handleGuestWillDownload resolved no owner context, routeBrowserClientDownload answered local for a non-client-hosted contents, and the item got a reserved Downloads name with no prompt and no tab to attribute it to. Downloads are now the one policy the preview partition does not inherit. Red-green with each mutant as the sole delta: making the installer ignore the deny option routes the preview partition's download to the router again, and making the preview protocol installer stop asking for the deny leaves the option unset. The absence half is paired with a presence precondition in the same run — an ordinary browsing partition installed beside the preview one in one test still reaches the download router, so a deny that fenced everything, or a download path that had been removed entirely, would fail rather than pass quietly."
},
{
{
"date": "2026-08-27",
"runner": "local",
"platform": "macos",
@@ -3441,7 +3441,7 @@
"durationSeconds": 0.36,
"summary": "The four surfaces the trusted-click route is built from passed 63/63: the preload's click policy over a real document, including an untrusted dispatched click, an untrusted middle click, an SVG animated href resolved against the document so a relative, rooted or fragment SVG link behaves exactly like the identical HTML anchor, fragment and percent-encoded fragment targets and a trusted middle click; the guest policy's deny-only sinks, its refusal of a navigation that arrives before a document has bound the guest to a grant, and its report gate for an unfocused guest, an unregistered sender, a missing or revoked grant and non-web URLs; will-attach-webview pinning the preview preload in both directions; and the click channel handing main its own sender without a trusted-renderer check."
},
{
{
"date": "2026-08-26",
"runner": "local",
"platform": "macos",
@@ -3468,7 +3468,7 @@
"durationSeconds": 66,
"summary": "The journey passed 3/3 again under --repeat-each=3 on the candidate that clears grants on every fresh shell document and gates the mint channel on the trusted renderer, so neither change disturbs a live preview: the document still rendered, both browser counts held at baseline, and the target=_blank press still routed out as an Orca browser tab."
},
{
{
"date": "2026-08-26",
"runner": "local",
"platform": "macos",
@@ -108,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 }
}
@@ -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
@@ -261,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.
@@ -331,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)
}
@@ -319,6 +319,21 @@ describe('pre-handler PTY buffer', () => {
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', () => {
@@ -301,15 +301,38 @@ export function discardPreHandlerPtyState(ptyId: string): void {
discardedPreHandlerPtyStates.set(ptyId, timer)
}
export function hasPreHandlerPtyExit(ptyId: string): boolean {
return (preHandlerPtyExit.get(ptyId)?.length ?? 0) > 0
/** 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 {
// Newest survivor: after the discards above at most one lifetime's records can still be ours, and
// picking by sequence keeps the last-write-wins delivery a single-slot buffer always had.
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 preHandlerPtyExit.get(ptyId) ?? []) {
for (const candidate of admissiblePreHandlerPtyExits(ptyId, incarnationId)) {
if (!exit || candidate.sequence > exit.sequence) {
exit = candidate
}
@@ -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' })
@@ -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<typeof store.createTab> | 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
@@ -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<string, string>
}): Promise<string> {
}): Promise<SpawnedPane> {
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<void> {
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(