Merge remote-tracking branch 'origin/main' into tmp/native-chat-recovery-main-integration

This commit is contained in:
Brennan Benson
2026-08-28 05:08:57 -07:00
59 changed files with 3673 additions and 240 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",
@@ -317,7 +317,8 @@ describe('registerPtyHandlers', () => {
upsertSshRemotePtyLease: vi.fn(),
persistPtyBinding: vi.fn(),
removeSshRemotePtyLease: vi.fn(),
markSshRemotePtyLease: vi.fn()
markSshRemotePtyLease: vi.fn(),
clearSshRemotePtyKillIntent: vi.fn()
}
const runtime = {
setPtyController: vi.fn(),
@@ -286,7 +286,7 @@ describe('registerPtyHandlers', () => {
getProfiles: vi.fn()
} as never)
const shutdown = vi.fn(async () => undefined)
const store = { markSshRemotePtyLease: vi.fn() }
const store = { markSshRemotePtyLease: vi.fn(), clearSshRemotePtyKillIntent: vi.fn() }
const runtime = {
setPtyController: vi.fn(),
onPtyExit: vi.fn()
@@ -357,7 +357,7 @@ describe('registerPtyHandlers', () => {
getDefaultShell: vi.fn(),
getProfiles: vi.fn()
} as never)
const store = { markSshRemotePtyLease: vi.fn() }
const store = { markSshRemotePtyLease: vi.fn(), clearSshRemotePtyKillIntent: vi.fn() }
const runtime = {
setPtyController: vi.fn(),
onPtyExit: vi.fn()
@@ -385,7 +385,8 @@ describe('registerPtyHandlers', () => {
})
it('marks a detached SSH lease terminated when runtime controller kill has no provider', async () => {
const store = {
markSshRemotePtyLease: vi.fn()
markSshRemotePtyLease: vi.fn(),
clearSshRemotePtyKillIntent: vi.fn()
}
const runtime = {
setPtyController: vi.fn(),
@@ -417,7 +418,8 @@ describe('registerPtyHandlers', () => {
it('keeps a rejected SSH PTY unverifiable after kill shutdown fails transiently', async () => {
const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {})
const store = {
markSshRemotePtyLease: vi.fn()
markSshRemotePtyLease: vi.fn(),
clearSshRemotePtyKillIntent: vi.fn()
}
const runtime = {
setPtyController: vi.fn(),
@@ -462,7 +462,8 @@ describe('registerPtyHandlers', () => {
throw new Error('SSH_SESSION_EXPIRED: remote-pty')
})
const store = {
markSshRemotePtyLease: vi.fn()
markSshRemotePtyLease: vi.fn(),
clearSshRemotePtyKillIntent: vi.fn()
}
registerSshPtyProvider('ssh-1', {
spawn: sshSpawn,
@@ -514,7 +515,8 @@ describe('registerPtyHandlers', () => {
throw new Error('SSH_SESSION_EXPIRED: remote-pty')
})
const store = {
markSshRemotePtyLease: vi.fn()
markSshRemotePtyLease: vi.fn(),
clearSshRemotePtyKillIntent: vi.fn()
}
registerSshPtyProvider('ssh-1', {
spawn: sshSpawn,
@@ -72,7 +72,8 @@ describe('registerPtyHandlers', () => {
)
})
const store = {
markSshRemotePtyLease: vi.fn()
markSshRemotePtyLease: vi.fn(),
clearSshRemotePtyKillIntent: vi.fn()
}
registerSshPtyProvider('ssh-1', {
spawn: sshSpawn,
@@ -136,7 +137,8 @@ describe('registerPtyHandlers', () => {
})
it('does not tombstone an SSH lease when explicit kill shutdown fails transiently', async () => {
const store = {
markSshRemotePtyLease: vi.fn()
markSshRemotePtyLease: vi.fn(),
clearSshRemotePtyKillIntent: vi.fn()
}
registerSshPtyProvider('ssh-1', {
spawn: vi.fn(),
@@ -188,7 +190,8 @@ describe('registerPtyHandlers', () => {
it('marks an SSH lease terminated after runtime controller kill succeeds', async () => {
const shutdown = vi.fn(async () => undefined)
const store = {
markSshRemotePtyLease: vi.fn()
markSshRemotePtyLease: vi.fn(),
clearSshRemotePtyKillIntent: vi.fn()
}
const runtime = {
setPtyController: vi.fn(),
@@ -408,7 +411,8 @@ describe('registerPtyHandlers', () => {
vi.useFakeTimers()
const shutdown = vi.fn(async () => undefined)
const store = {
markSshRemotePtyLease: vi.fn()
markSshRemotePtyLease: vi.fn(),
clearSshRemotePtyKillIntent: vi.fn()
}
const runtime = {
setPtyController: vi.fn(),
@@ -119,7 +119,8 @@ describe('registerPtyHandlers', () => {
persistPtyBinding: vi.fn(),
upsertSshRemotePtyLease: vi.fn(),
removeSshRemotePtyLease: vi.fn(),
markSshRemotePtyLease: vi.fn()
markSshRemotePtyLease: vi.fn(),
clearSshRemotePtyKillIntent: vi.fn()
}
const runtime = {
setPtyController: vi.fn(),
@@ -477,7 +478,8 @@ describe('registerPtyHandlers', () => {
upsertSshRemotePtyLease: vi.fn(),
persistPtyBinding: vi.fn(),
removeSshRemotePtyLease: vi.fn(),
markSshRemotePtyLease: vi.fn()
markSshRemotePtyLease: vi.fn(),
clearSshRemotePtyKillIntent: vi.fn()
}
let controller: RuntimeSpawnController | null = null
const runtime = {
@@ -156,7 +156,8 @@ describe('registerPtyHandlers', () => {
upsertSshRemotePtyLease: vi.fn(),
persistPtyBinding: vi.fn(),
removeSshRemotePtyLease: vi.fn(),
markSshRemotePtyLease: vi.fn()
markSshRemotePtyLease: vi.fn(),
clearSshRemotePtyKillIntent: vi.fn()
}
let controller: RuntimeSpawnController | null = null
const runtime = {
@@ -373,7 +374,8 @@ describe('registerPtyHandlers', () => {
throw new Error('disk full')
}),
removeSshRemotePtyLease: vi.fn(),
markSshRemotePtyLease: vi.fn()
markSshRemotePtyLease: vi.fn(),
clearSshRemotePtyKillIntent: vi.fn()
}
let controller: RuntimeSpawnController | null = null
const runtime = {
@@ -469,7 +471,8 @@ describe('registerPtyHandlers', () => {
upsertSshRemotePtyLease: vi.fn(),
persistPtyBinding: vi.fn(),
removeSshRemotePtyLease: vi.fn(),
markSshRemotePtyLease: vi.fn()
markSshRemotePtyLease: vi.fn(),
clearSshRemotePtyKillIntent: vi.fn()
}
let controller: RuntimeSpawnController | null = null
const runtime = {
@@ -110,7 +110,8 @@ describe('registerPtyHandlers', () => {
upsertSshRemotePtyLease: vi.fn(),
persistPtyBinding: vi.fn(),
removeSshRemotePtyLease: vi.fn(),
markSshRemotePtyLease: vi.fn()
markSshRemotePtyLease: vi.fn(),
clearSshRemotePtyKillIntent: vi.fn()
}
let controller: RuntimeSpawnController | null = null
const runtime = {
@@ -215,7 +216,8 @@ describe('registerPtyHandlers', () => {
throw new Error('disk full')
}),
removeSshRemotePtyLease: vi.fn(),
markSshRemotePtyLease: vi.fn()
markSshRemotePtyLease: vi.fn(),
clearSshRemotePtyKillIntent: vi.fn()
}
try {
@@ -302,7 +302,7 @@ describe('registerPtyHandlers', () => {
getProfiles: vi.fn()
} as never)
const sshShutdown = vi.fn(async () => undefined)
const store = { markSshRemotePtyLease: vi.fn() }
const store = { markSshRemotePtyLease: vi.fn(), clearSshRemotePtyKillIntent: vi.fn() }
registerSshPtyProvider('ssh-1', {
spawn: vi.fn(),
write: vi.fn(),
@@ -367,7 +367,7 @@ describe('registerPtyHandlers', () => {
getDefaultShell: vi.fn(),
getProfiles: vi.fn()
} as never)
const store = { markSshRemotePtyLease: vi.fn() }
const store = { markSshRemotePtyLease: vi.fn(), clearSshRemotePtyKillIntent: vi.fn() }
registerPtyHandlers(
mainWindow as never,
undefined,
@@ -386,7 +386,8 @@ describe('registerPtyHandlers', () => {
const store = {
upsertSshRemotePtyLease: vi.fn(),
persistPtyBinding: vi.fn(),
markSshRemotePtyLease: vi.fn()
markSshRemotePtyLease: vi.fn(),
clearSshRemotePtyKillIntent: vi.fn()
}
const provider = {
spawn: vi.fn(async () => ({ id: 'remote-pty' })),
@@ -0,0 +1,351 @@
import { describe, expect, it, vi } from 'vitest'
import { setupPtyIpcSuite } from './pty-ipc-test-harness'
import { SSH_SESSION_EXPIRED_ERROR } from '../providers/ssh-pty-errors'
import {
registerPtyHandlers,
registerSshPtyProvider,
unregisterSshPtyProvider,
deletePtyOwnership,
setPtyOwnership,
restorePtyIncarnation
} from './pty'
vi.mock('electron', () => import('./pty-ipc-mock-registry').then((m) => m.electronModuleMock()))
vi.mock('fs', () => import('./pty-ipc-mock-registry').then((m) => m.fsModuleMock()))
vi.mock('node-pty', () => import('./pty-ipc-mock-registry').then((m) => m.nodePtyModuleMock()))
vi.mock('node:child_process', async (importOriginal) =>
(await import('./pty-ipc-mock-registry')).childProcessModuleMock(await importOriginal())
)
vi.mock('../opencode/hook-service', () =>
import('./pty-ipc-mock-registry').then((m) => m.openCodeHookServiceModuleMock())
)
vi.mock('../mimo/hook-service', () =>
import('./pty-ipc-mock-registry').then((m) => m.mimoHookServiceModuleMock())
)
vi.mock('../agent-hooks/server', () =>
import('./pty-ipc-mock-registry').then((m) => m.agentHookServerModuleMock())
)
vi.mock('../pi/titlebar-extension-service', () =>
import('./pty-ipc-mock-registry').then((m) => m.piTitlebarExtensionModuleMock())
)
vi.mock('../pwsh', () => import('./pty-ipc-mock-registry').then((m) => m.pwshModuleMock()))
vi.mock('../wsl', async (importOriginal) =>
(await import('./pty-ipc-mock-registry')).wslModuleMock(await importOriginal())
)
vi.mock('../telemetry/client', () =>
import('./pty-ipc-mock-registry').then((m) => m.telemetryClientModuleMock())
)
vi.mock('../telemetry/classify-error', () =>
import('./pty-ipc-mock-registry').then((m) => m.classifyErrorModuleMock())
)
vi.mock('../cli/linux-terminal-orca-cli-shim', () =>
import('./pty-ipc-mock-registry').then((m) => m.linuxCliShimModuleMock())
)
vi.mock('../memory/pty-registry', () =>
import('./pty-ipc-mock-registry').then((m) => m.ptyRegistryModuleMock())
)
vi.mock('../agent-hooks/migration-unsupported-pty-state', () =>
import('./pty-ipc-mock-registry').then((m) => m.migrationUnsupportedPtyModuleMock())
)
vi.mock('../codex/codex-pane-account-registry', () =>
import('./pty-ipc-mock-registry').then((m) => m.codexPaneAccountRegistryModuleMock())
)
vi.mock('../codex/codex-state-db-backfill-recovery', () =>
import('./pty-ipc-mock-registry').then((m) => m.codexBackfillRecoveryModuleMock())
)
const SCOPED_PTY_ID = 'ssh:ssh-1@@pty-7'
function createKillStore() {
return {
markSshRemotePtyLease: vi.fn(),
recordSshRemotePtyKillIntent: vi.fn(),
clearSshRemotePtyKillIntent: vi.fn()
}
}
function sshProviderStub(shutdown: () => Promise<void>) {
return {
shutdown: vi.fn(shutdown),
onExit: vi.fn(() => () => {}),
onData: vi.fn(() => () => {}),
onReplay: vi.fn(() => () => {}),
listProcesses: vi.fn(async () => [])
} as never
}
function installController(handlers: Map<string, never>) {
const runtime = {
setPtyController: vi.fn(),
markPtyStopRequested: vi.fn(),
markPtyLivenessUnverifiable: vi.fn(),
onPtyExit: vi.fn()
}
handlers.clear()
return { runtime }
}
describe('undelivered SSH stops', () => {
const { handlers, mainWindow } = setupPtyIpcSuite()
function install(store: ReturnType<typeof createKillStore>): {
kill: (ptyId: string) => boolean
stopAndWait: (ptyId: string, opts?: { keepHistory?: boolean }) => Promise<boolean>
markReversibleStops: (ptyIds: readonly string[]) => () => void
runtime: ReturnType<typeof installController>['runtime']
} {
const { runtime } = installController(handlers as never)
registerPtyHandlers(
mainWindow as never,
runtime as never,
undefined,
undefined,
undefined,
store as never
)
const controller = runtime.setPtyController.mock.calls[0]?.[0] as {
kill: (ptyId: string) => boolean
stopAndWait: (ptyId: string, opts?: { keepHistory?: boolean }) => Promise<boolean>
markReversibleStops: (ptyIds: readonly string[]) => () => void
}
return {
kill: controller.kill,
stopAndWait: controller.stopAndWait,
markReversibleStops: controller.markReversibleStops,
runtime
}
}
// The leak: the shutdown died on the transport, the verdict is correctly `unverifiable`, and
// before this fix nothing retried — the remote shell survived forever.
it('records the stop when the shutdown RPC rejects with a transport failure', async () => {
const store = createKillStore()
registerSshPtyProvider(
'ssh-1',
sshProviderStub(async () => {
throw new Error('socket closed')
})
)
setPtyOwnership(SCOPED_PTY_ID, 'ssh-1')
restorePtyIncarnation(SCOPED_PTY_ID, 'inc-a')
const { kill, runtime } = install(store)
try {
expect(kill(SCOPED_PTY_ID)).toBe(true)
await vi.waitFor(() => expect(store.recordSshRemotePtyKillIntent).toHaveBeenCalled())
expect(store.recordSshRemotePtyKillIntent).toHaveBeenCalledWith(
'ssh-1',
'pty-7',
expect.objectContaining({ incarnationId: 'inc-a', attempts: 0 })
)
expect(runtime.markPtyLivenessUnverifiable).toHaveBeenCalledWith(
SCOPED_PTY_ID,
'socket closed'
)
} finally {
unregisterSshPtyProvider('ssh-1')
deletePtyOwnership(SCOPED_PTY_ID)
}
})
// The offline close: no provider is registered, so the relay is never even asked. The lease is
// tombstoned locally, and without the record the remote shell has nothing left to retire it.
it('records the stop when no provider is registered to deliver it', async () => {
const store = createKillStore()
setPtyOwnership(SCOPED_PTY_ID, 'ssh-1')
restorePtyIncarnation(SCOPED_PTY_ID, 'inc-b')
const { kill } = install(store)
try {
expect(kill(SCOPED_PTY_ID)).toBe(false)
expect(store.recordSshRemotePtyKillIntent).toHaveBeenCalledWith(
'ssh-1',
'pty-7',
expect.objectContaining({ incarnationId: 'inc-b' })
)
} finally {
deletePtyOwnership(SCOPED_PTY_ID)
}
})
it('records nothing when the shutdown RPC is delivered', async () => {
const store = createKillStore()
registerSshPtyProvider(
'ssh-1',
sshProviderStub(async () => {})
)
setPtyOwnership(SCOPED_PTY_ID, 'ssh-1')
restorePtyIncarnation(SCOPED_PTY_ID, 'inc-c')
const { kill } = install(store)
try {
expect(kill(SCOPED_PTY_ID)).toBe(true)
await vi.waitFor(() => expect(store.markSshRemotePtyLease).toHaveBeenCalled())
expect(store.recordSshRemotePtyKillIntent).not.toHaveBeenCalled()
// And it does not retire one either. A resolved RPC is not a death certificate — the relay
// answers identically for a PTY it never had — so retirement is left to the replay, which
// only acts on a live inventory. Clearing here would be the mechanism by which a still-valid
// order is destroyed on an unconfirmed success.
expect(store.clearSshRemotePtyKillIntent).not.toHaveBeenCalled()
} finally {
unregisterSshPtyProvider('ssh-1')
deletePtyOwnership(SCOPED_PTY_ID)
}
})
// Worktree sleep stops through stopAndWait and marks those stops reversible: when one does not
// land, the pane stays live and the user keeps using it. A durable order recorded here would come
// back on a later handshake and kill that terminal out from under them.
it('records nothing for a reversible stopAndWait that could not be delivered', async () => {
const store = createKillStore()
registerSshPtyProvider(
'ssh-1',
sshProviderStub(async () => {
throw new Error('socket closed')
})
)
setPtyOwnership(SCOPED_PTY_ID, 'ssh-1')
restorePtyIncarnation(SCOPED_PTY_ID, 'inc-e')
const { stopAndWait } = install(store)
try {
await expect(stopAndWait(SCOPED_PTY_ID, { keepHistory: true })).resolves.toBe(false)
expect(store.recordSshRemotePtyKillIntent).not.toHaveBeenCalled()
} finally {
unregisterSshPtyProvider('ssh-1')
deletePtyOwnership(SCOPED_PTY_ID)
}
})
// The path the UI actually takes. `window.api.pty.kill` -> `pty:kill`, a separate implementation
// from the runtime controller, and the one #12447 describes.
it('records the stop when the renderer pty:kill shutdown rejects', async () => {
const store = createKillStore()
registerSshPtyProvider(
'ssh-1',
sshProviderStub(async () => {
throw new Error('socket closed')
})
)
setPtyOwnership(SCOPED_PTY_ID, 'ssh-1')
restorePtyIncarnation(SCOPED_PTY_ID, 'inc-r1')
install(store)
try {
await expect(handlers.get('pty:kill')!(null, { id: SCOPED_PTY_ID })).rejects.toThrow(
'socket closed'
)
expect(store.recordSshRemotePtyKillIntent).toHaveBeenCalledWith(
'ssh-1',
'pty-7',
expect.objectContaining({ incarnationId: 'inc-r1' })
)
} finally {
unregisterSshPtyProvider('ssh-1')
deletePtyOwnership(SCOPED_PTY_ID)
}
})
it('records the stop when renderer pty:kill finds no provider to deliver it', async () => {
const store = createKillStore()
setPtyOwnership(SCOPED_PTY_ID, 'ssh-1')
restorePtyIncarnation(SCOPED_PTY_ID, 'inc-r2')
install(store)
try {
await handlers.get('pty:kill')!(null, { id: SCOPED_PTY_ID })
expect(store.recordSshRemotePtyKillIntent).toHaveBeenCalledWith(
'ssh-1',
'pty-7',
expect.objectContaining({ incarnationId: 'inc-r2' })
)
} finally {
deletePtyOwnership(SCOPED_PTY_ID)
}
})
// Pane hibernation is the reversible caller on this IPC and is the only one passing keepHistory.
it('records nothing for a renderer pty:kill that keeps history', async () => {
const store = createKillStore()
registerSshPtyProvider(
'ssh-1',
sshProviderStub(async () => {
throw new Error('socket closed')
})
)
setPtyOwnership(SCOPED_PTY_ID, 'ssh-1')
restorePtyIncarnation(SCOPED_PTY_ID, 'inc-r3')
install(store)
try {
await expect(
handlers.get('pty:kill')!(null, { id: SCOPED_PTY_ID, keepHistory: true })
).rejects.toThrow('socket closed')
expect(store.recordSshRemotePtyKillIntent).not.toHaveBeenCalled()
} finally {
unregisterSshPtyProvider('ssh-1')
deletePtyOwnership(SCOPED_PTY_ID)
}
})
it('records nothing while a reversible stop owns the PTY', async () => {
const store = createKillStore()
registerSshPtyProvider(
'ssh-1',
sshProviderStub(async () => {
throw new Error('socket closed')
})
)
setPtyOwnership(SCOPED_PTY_ID, 'ssh-1')
restorePtyIncarnation(SCOPED_PTY_ID, 'inc-f')
const { kill, markReversibleStops } = install(store)
const release = markReversibleStops([SCOPED_PTY_ID])
try {
kill(SCOPED_PTY_ID)
await new Promise((resolve) => setTimeout(resolve, 0))
expect(store.recordSshRemotePtyKillIntent).not.toHaveBeenCalled()
} finally {
release()
unregisterSshPtyProvider('ssh-1')
deletePtyOwnership(SCOPED_PTY_ID)
}
})
it('records nothing for a local PTY, which has no later host to ask', async () => {
const store = createKillStore()
setPtyOwnership('local-pty', null)
restorePtyIncarnation('local-pty', 'inc-d')
const { kill } = install(store)
try {
kill('local-pty')
await new Promise((resolve) => setTimeout(resolve, 0))
expect(store.recordSshRemotePtyKillIntent).not.toHaveBeenCalled()
} finally {
deletePtyOwnership('local-pty')
}
})
// No incarnation means no fence, and an unfenced order can only be discarded or guessed at.
it('records nothing when the PTY incarnation was never learned', async () => {
const store = createKillStore()
registerSshPtyProvider(
'ssh-1',
sshProviderStub(async () => {
throw new Error(`${SSH_SESSION_EXPIRED_ERROR}-transport`)
})
)
setPtyOwnership('ssh:ssh-1@@pty-8', 'ssh-1')
const { kill } = install(store)
try {
kill('ssh:ssh-1@@pty-8')
await new Promise((resolve) => setTimeout(resolve, 0))
expect(store.recordSshRemotePtyKillIntent).not.toHaveBeenCalled()
} finally {
unregisterSshPtyProvider('ssh-1')
deletePtyOwnership('ssh:ssh-1@@pty-8')
}
})
})
+1 -80
View File
@@ -1,7 +1,4 @@
import { getPtyIpc } from '../../pty-host-bindings'
import type { Store } from '../../../persistence'
import type { OrcaRuntimeService } from '../../../runtime/orca-runtime'
import type { IPtyProvider } from '../../../providers/types'
import { parseAppSshPtyId } from '../../../providers/ssh-pty-id'
import { inspectPtyProviderProcessForRenderer } from '../../../providers/pty-process-inspection'
import {
@@ -9,7 +6,6 @@ import {
visitPtyProcessListingsInBatches
} from '../../../providers/pty-process-list-admission'
import type { PtyListedSession } from '../../../../shared/pty-listed-session'
import { SSH_PROVIDER_UNREGISTERED_REASON } from '../../../../shared/pty-liveness-verdict'
import { ptyOwnership } from '../provider/ownership-state'
import {
getProviderForPty,
@@ -18,7 +14,6 @@ import {
sshProviders,
tryGetProviderForPty
} from '../provider/registry'
import { finishPtyShutdown, isPtyAlreadyGoneError } from '../provider/liveness'
import { ptySizes } from '../delivery/visibility-state'
import { isValidPaneKey } from '../pane/key-state'
import {
@@ -29,84 +24,10 @@ import {
} from '../pane/serializer-state'
export function installPtyInspectIpcHandlers(deps: {
store?: Store
runtime?: OrcaRuntimeService
getLocalPtyProviderStartupPromise: (connectionId?: string | null) => Promise<void> | undefined
shutdownProviderAndDetectExit: (
provider: IPtyProvider,
id: string,
opts: { immediate?: boolean; keepHistory?: boolean; deadlineMs?: number }
) => Promise<boolean>
rememberSyntheticKillExit: (id: string) => void
sendPtyExitToRenderer: (payload: { id: string; code: number; incarnationId?: string }) => void
}): void {
const ipcMain = getPtyIpc()
const {
store,
runtime,
getLocalPtyProviderStartupPromise,
shutdownProviderAndDetectExit,
rememberSyntheticKillExit,
sendPtyExitToRenderer
} = deps
ipcMain.handle('pty:kill', async (_event, args: { id: string; keepHistory?: boolean }) => {
if (typeof args?.id !== 'string' || !args.id || args.id.startsWith('remote:')) {
// Why: runtime terminal handles belong to terminal.close; unowned PTY routing could target the local provider.
throw new Error('Invalid PTY provider id')
}
runtime?.markPtyStopRequested?.(args.id)
const ownedConnectionId = ptyOwnership.get(args.id)
const parsedSshId = ownedConnectionId === undefined ? parseAppSshPtyId(args.id) : null
const connectionId = ownedConnectionId ?? parsedSshId?.connectionId
// Why: wait for daemon startup before selecting the local provider, else a fallback shutdown falsely succeeds and orphans a restored daemon PTY (#7742).
const startupPromise = getLocalPtyProviderStartupPromise(connectionId)
if (startupPromise) {
await startupPromise
}
const provider = connectionId ? sshProviders.get(connectionId) : tryGetProviderForPty(args.id)
if (!provider && connectionId) {
// Why: detached SSH PTYs intentionally keep ownership after their
// provider is unregistered; hydrated app-scoped ids can also arrive
// before ownership is rebuilt. Tombstone instead of falling back local.
const incarnationId = finishPtyShutdown(args.id, connectionId, store)
runtime?.markPtyLivenessUnverifiable?.(args.id, SSH_PROVIDER_UNREGISTERED_REASON)
runtime?.onPtyExit(args.id, -1, incarnationId)
rememberSyntheticKillExit(args.id)
sendPtyExitToRenderer({
id: args.id,
code: -1,
...(incarnationId ? { incarnationId } : {})
})
return
}
const shutdownProvider = provider ?? getProviderForPty(args.id)
let providerExitObserved = false
try {
providerExitObserved = await shutdownProviderAndDetectExit(shutdownProvider, args.id, {
immediate: true,
keepHistory: args.keepHistory ?? false
})
} catch (err) {
if (!isPtyAlreadyGoneError(err)) {
// Why: a failed shutdown can leave the process alive (SSH relay grace window / local daemon); keep ownership/lease state so the user can retry.
throw err
}
/* session already dead — cleanup below handles the rest */
}
// Why: some shutdown paths do not emit onExit through the provider listener.
// Explicit cleanup is idempotent and covers already-dead PTYs.
const incarnationId = finishPtyShutdown(args.id, connectionId, store)
if (!providerExitObserved) {
runtime?.onPtyExit(args.id, -1, incarnationId)
rememberSyntheticKillExit(args.id)
sendPtyExitToRenderer({
id: args.id,
code: -1,
...(incarnationId ? { incarnationId } : {})
})
}
})
const { getLocalPtyProviderStartupPromise } = deps
ipcMain.handle('pty:listSessions', async (): Promise<PtyListedSession[]> => {
const deduped = new Map<string, PtyListedSession>()
+111
View File
@@ -0,0 +1,111 @@
import { getPtyIpc } from '../../pty-host-bindings'
import type { Store } from '../../../persistence'
import type { OrcaRuntimeService } from '../../../runtime/orca-runtime'
import type { IPtyProvider } from '../../../providers/types'
import { parseAppSshPtyId } from '../../../providers/ssh-pty-id'
import { SSH_PROVIDER_UNREGISTERED_REASON } from '../../../../shared/pty-liveness-verdict'
import { ptyOwnership } from '../provider/ownership-state'
import { getProviderForPty, sshProviders, tryGetProviderForPty } from '../provider/registry'
import { finishPtyShutdown, isPtyAlreadyGoneError } from '../provider/liveness'
import { recordUndeliveredSshPtyKill } from '../runtime/undelivered-ssh-kill'
export type PtyKillIpcDeps = {
store?: Store
runtime?: OrcaRuntimeService
getLocalPtyProviderStartupPromise: (connectionId?: string | null) => Promise<void> | undefined
shutdownProviderAndDetectExit: (
provider: IPtyProvider,
id: string,
opts: { immediate?: boolean; keepHistory?: boolean; deadlineMs?: number }
) => Promise<boolean>
rememberSyntheticKillExit: (id: string) => void
sendPtyExitToRenderer: (payload: { id: string; code: number; incarnationId?: string }) => void
}
/** `pty:kill` — the route the renderer takes when the user closes a terminal tab, and a separate
* implementation from `killPtyFromRuntimeController`. Both have to record an undelivered SSH stop,
* and this is the one ordinary tab close actually reaches. */
export function installPtyKillIpcHandler(deps: PtyKillIpcDeps): void {
const ipcMain = getPtyIpc()
const {
store,
runtime,
getLocalPtyProviderStartupPromise,
shutdownProviderAndDetectExit,
rememberSyntheticKillExit,
sendPtyExitToRenderer
} = deps
ipcMain.handle('pty:kill', async (_event, args: { id: string; keepHistory?: boolean }) => {
if (typeof args?.id !== 'string' || !args.id || args.id.startsWith('remote:')) {
// Why: runtime terminal handles belong to terminal.close; unowned PTY routing could target the local provider.
throw new Error('Invalid PTY provider id')
}
runtime?.markPtyStopRequested?.(args.id)
const ownedConnectionId = ptyOwnership.get(args.id)
const parsedSshId = ownedConnectionId === undefined ? parseAppSshPtyId(args.id) : null
const connectionId = ownedConnectionId ?? parsedSshId?.connectionId
// Why: wait for daemon startup before selecting the local provider, else a fallback shutdown falsely succeeds and orphans a restored daemon PTY (#7742).
const startupPromise = getLocalPtyProviderStartupPromise(connectionId)
if (startupPromise) {
await startupPromise
}
// Why stated rather than inferred: this IPC serves both the ordinary tab close and pane
// hibernation, and only hibernation passes keepHistory. Recording a replayable kill for a
// hibernating pane would destroy it on the next handshake.
const reversible = args.keepHistory === true
const provider = connectionId ? sshProviders.get(connectionId) : tryGetProviderForPty(args.id)
if (!provider && connectionId) {
// Why: detached SSH PTYs intentionally keep ownership after their
// provider is unregistered; hydrated app-scoped ids can also arrive
// before ownership is rebuilt. Tombstone instead of falling back local.
const incarnationId = finishPtyShutdown(args.id, connectionId, store)
// The relay was never asked, so the remote shell is still running. Keep the order.
recordUndeliveredSshPtyKill({
store,
ptyId: args.id,
connectionId,
reversible,
incarnationId
})
runtime?.markPtyLivenessUnverifiable?.(args.id, SSH_PROVIDER_UNREGISTERED_REASON)
runtime?.onPtyExit(args.id, -1, incarnationId)
rememberSyntheticKillExit(args.id)
sendPtyExitToRenderer({
id: args.id,
code: -1,
...(incarnationId ? { incarnationId } : {})
})
return
}
const shutdownProvider = provider ?? getProviderForPty(args.id)
let providerExitObserved = false
try {
providerExitObserved = await shutdownProviderAndDetectExit(shutdownProvider, args.id, {
immediate: true,
keepHistory: args.keepHistory ?? false
})
} catch (err) {
if (!isPtyAlreadyGoneError(err)) {
// Why: a failed shutdown can leave the process alive (SSH relay grace window / local daemon); keep ownership/lease state so the user can retry.
// The renderer has already discarded the tab, so nothing here retries — the durable order
// is what lets the next handshake to this host finish the close.
recordUndeliveredSshPtyKill({ store, ptyId: args.id, connectionId, reversible })
throw err
}
/* session already dead — cleanup below handles the rest */
}
// Why: some shutdown paths do not emit onExit through the provider listener.
// Explicit cleanup is idempotent and covers already-dead PTYs.
const incarnationId = finishPtyShutdown(args.id, connectionId, store)
if (!providerExitObserved) {
runtime?.onPtyExit(args.id, -1, incarnationId)
rememberSyntheticKillExit(args.id)
sendPtyExitToRenderer({
id: args.id,
code: -1,
...(incarnationId ? { incarnationId } : {})
})
}
})
}
+4
View File
@@ -143,6 +143,10 @@ export function finishPtyShutdown(
const incarnationId = ptyIncarnationById.get(id)
clearProviderPtyState(id)
if (connectionId) {
// Deliberately does NOT retire a recorded undelivered stop. Some callers reach here having
// asked the host and some having never asked it, so retiring from this one place would be a
// contract every call site has to know about — and the one that forgot would silently drop a
// kill order. Retirement is left to the replay, which only acts on host evidence.
store?.markSshRemotePtyLease(connectionId, getRelayPtyId(connectionId, id), 'terminated')
}
ptyOwnership.delete(id)
+3 -1
View File
@@ -12,6 +12,7 @@ import { localProvider } from './provider/registry'
import { finishPtyShutdown } from './provider/liveness'
import type { GetSelectedCodexHomePath, PrepareClaudeAuth } from './host-env/types'
import { installPtyInspectIpcHandlers } from './ipc/inspect'
import { installPtyKillIpcHandler } from './ipc/renderer-kill'
import { installPtyWriteIpcHandlers } from './ipc/write'
import { installPtySpawnIpcHandler } from './ipc/spawn'
import { installPtyRuntimeController } from './runtime/controller'
@@ -256,7 +257,8 @@ export function registerPtyHandlers(
clearHiddenRendererResizeOutput: session.clearHiddenRendererResizeOutput
})
installPtyResizeVisibilityIpc(session)
installPtyInspectIpcHandlers({
installPtyInspectIpcHandlers({ getLocalPtyProviderStartupPromise })
installPtyKillIpcHandler({
store,
runtime,
getLocalPtyProviderStartupPromise,
+26 -1
View File
@@ -4,6 +4,7 @@ import { parseAppSshPtyId } from '../../../providers/ssh-pty-id'
import { ptyOwnership, ptyIncarnationById } from '../provider/ownership-state'
import { getProvider, getProviderForPty } from '../provider/registry'
import { isPtyAlreadyGoneError, delay, verifyPtyStopped } from '../provider/liveness'
import { recordUndeliveredSshPtyKill } from './undelivered-ssh-kill'
import type { PtyRuntimeControllerDeps } from './controller-deps'
export function killPtyFromRuntimeController(
@@ -18,12 +19,22 @@ export function killPtyFromRuntimeController(
rememberSyntheticKillExit,
sendPtyExitToRenderer,
finishPtyShutdown,
retiredRejectedPtyIds
retiredRejectedPtyIds,
reversibleStopOwnersByPtyId
} = deps
runtime?.markPtyStopRequested?.(ptyId)
let connectionId: string | null | undefined = ptyOwnership.get(ptyId)
const parsedSshId = connectionId === undefined ? parseAppSshPtyId(ptyId) : null
connectionId ??= parsedSshId?.connectionId
const recordUndelivered = (incarnationId?: string): void => {
recordUndeliveredSshPtyKill({
store,
ptyId,
connectionId,
reversible: reversibleStopOwnersByPtyId.has(ptyId),
incarnationId
})
}
const killWithCurrentProvider = (): boolean => {
let provider: IPtyProvider
try {
@@ -34,6 +45,8 @@ export function killPtyFromRuntimeController(
// provider was unregistered. Tombstone the lease so reconnect does
// not revive a terminal the user explicitly closed.
const incarnationId = finishPtyShutdown(ptyId, connectionId, store)
// The relay was never asked, so the remote shell is still running. Keep the order.
recordUndelivered(incarnationId)
runtime?.onPtyExit(ptyId, -1, incarnationId)
rememberSyntheticKillExit(ptyId)
sendPtyExitToRenderer({
@@ -90,6 +103,9 @@ export function killPtyFromRuntimeController(
}
runtime?.onPtyExit(ptyId, -1, ptyIncarnationById.get(ptyId))
}
// Outside the `retired` guard: the remote process outlives this client's bookkeeping
// either way, and the intent is what the next handshake replays.
recordUndelivered()
})
return true
}
@@ -109,6 +125,7 @@ export function killPtyFromRuntimeController(
}
runtime?.onPtyExit(ptyId, -1, ptyIncarnationById.get(ptyId))
}
recordUndelivered()
})
return true
}
@@ -191,6 +208,14 @@ export function markReversibleStopsFromRuntimeController(
}
}
/**
* Deliberately records no undelivered-stop intent, unlike `killPtyFromRuntimeController`.
*
* Only a caller that gives the PTY up for good may leave a replayable kill order behind. This one
* hands its failures back instead: worktree sleep marks these stops reversible and leaves the pane
* live and usable when one does not land, so a durable order recorded here would come back on a
* later handshake and destroy a terminal the user had gone back to using.
*/
export async function stopAndWaitPtyFromRuntimeController(
deps: PtyRuntimeControllerDeps,
ptyId: string,
@@ -0,0 +1,53 @@
import type { Store } from '../../../persistence'
import { ptyIncarnationById } from '../provider/ownership-state'
import { getRelayPtyId } from '../provider/registry'
export type UndeliveredSshPtyKill = {
store: Store | undefined
ptyId: string
connectionId: string | null | undefined
/** Whether this caller may still undo the stop. Every caller states it rather than having it
* inferred, because both directions are bugs: `true` on a real close re-opens the leak, and
* `false` on a reversible one lets a later handshake kill a pane the user went back to using.
* Worktree sleep is the reversible case, on both the runtime and the renderer route. */
reversible: boolean
/** Pass what `finishPtyShutdown` returned: it clears the live map, so a caller that already ran
* it can no longer look the incarnation up. */
incarnationId?: string
now?: number
}
/** Durably records a stop this client asked for on an SSH host and could not confirm, so the next
* handshake to that host can replay it. Without this the remote PTY — a child of the detached
* relay daemon, not of the ssh channel — outlives the failed RPC forever.
*
* The silent no-ops are the safety rules, not defensive padding:
* - **reversible**: see above.
* - **no `connectionId`**: a local PTY's owner is this process, so a failed kill has no later host
* to ask; there is nothing to replay against.
* - **no incarnation**: the replay fence is the host-minted PTY incarnation, and a relay renumbers
* from `pty-1` on every start. An order we could never safely aim can only be discarded later,
* or worse, guessed at.
* - **an id naming another connection**: `getRelayPtyId` throws on those, and this runs inside
* promise `.catch` handlers where that would surface as an unhandled rejection. */
export function recordUndeliveredSshPtyKill(args: UndeliveredSshPtyKill): void {
const { store, ptyId, connectionId } = args
if (!store || !connectionId || args.reversible) {
return
}
const incarnationId = args.incarnationId ?? ptyIncarnationById.get(ptyId)
if (!incarnationId) {
return
}
let relayPtyId: string
try {
relayPtyId = getRelayPtyId(connectionId, ptyId)
} catch {
return
}
store.recordSshRemotePtyKillIntent(connectionId, relayPtyId, {
requestedAt: args.now ?? Date.now(),
incarnationId,
attempts: 0
})
}
+11 -1
View File
@@ -31,6 +31,11 @@ export type SshLeaseStoreMock = {
markSshRemotePtyLeasesForShutdown: Mock
markSshRemotePtyLeasesAttachedAsync: Mock
removeSshRemotePtyLeases: Mock
getSshRemotePtyKillIntents: Mock
pruneExpiredSshRemotePtyKillIntents: Mock
recordSshRemotePtyKillIntent: Mock
clearSshRemotePtyKillIntent: Mock
noteSshRemotePtyKillReplayAttempt: Mock
}
export type MockBrowserWindow = { isDestroyed: () => boolean; webContents: { send: Mock } }
@@ -102,7 +107,12 @@ export function createSshIpcHarness(mocks: SshIpcMocks): SshIpcHarness {
markSshRemotePtyLeasesAsync: vi.fn(),
markSshRemotePtyLeasesForShutdown: vi.fn(),
markSshRemotePtyLeasesAttachedAsync: vi.fn(),
removeSshRemotePtyLeases: vi.fn()
removeSshRemotePtyLeases: vi.fn(),
getSshRemotePtyKillIntents: vi.fn().mockReturnValue([]),
pruneExpiredSshRemotePtyKillIntents: vi.fn(),
recordSshRemotePtyKillIntent: vi.fn(),
clearSshRemotePtyKillIntent: vi.fn(),
noteSshRemotePtyKillReplayAttempt: vi.fn()
}
const mockWindow = {
isDestroyed: () => false,
@@ -0,0 +1,209 @@
import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'
import { rmSync, mkdtempSync } from 'node:fs'
import { join } from 'node:path'
import { tmpdir } from 'node:os'
import { testState, createStore } from './persistence-test-harness'
import {
MAX_SSH_PENDING_PTY_KILLS_PER_TARGET,
SSH_PENDING_PTY_KILL_TTL_MS
} from '../shared/ssh-pending-pty-kill'
vi.mock('electron', () => ({
app: { getPath: () => testState.dir },
safeStorage: {
isEncryptionAvailable: () => false,
encryptString: (plaintext: string) => Buffer.from(plaintext, 'utf-8'),
decryptString: (ciphertext: Buffer) => ciphertext.toString('utf-8')
}
}))
vi.mock('./telemetry/client', () => ({ track: vi.fn() }))
vi.mock('./telemetry/cohort-classifier', () => ({ getCohortAtEmit: vi.fn(() => ({})) }))
const NOW = 1_800_000_000_000
describe('Store SSH pending PTY kills', () => {
beforeEach(() => {
testState.dir = mkdtempSync(join(tmpdir(), 'orca-test-'))
})
afterEach(() => {
rmSync(testState.dir, { recursive: true, force: true })
})
// The whole point of the record: a laptop closed mid-failure must not lose the kill order.
it('survives an app restart', async () => {
const store = await createStore()
store.upsertSshRemotePtyLease({ targetId: 'ssh-1', ptyId: 'pty-1', state: 'attached' })
store.recordSshRemotePtyKillIntent('ssh-1', 'pty-1', {
requestedAt: NOW,
incarnationId: 'inc-a',
attempts: 0
})
store.flush()
const reloaded = await createStore()
expect(reloaded.getSshRemotePtyKillIntents('ssh-1', NOW)).toEqual([
{ ptyId: 'pty-1', intent: { requestedAt: NOW, incarnationId: 'inc-a', attempts: 0 } }
])
})
// A kill issued while the provider was already unregistered writes no lease of its own, and that
// offline close is the case most likely to strand a remote shell.
it('records an intent for a PTY that has no lease row yet', async () => {
const store = await createStore()
store.recordSshRemotePtyKillIntent('ssh-1', 'pty-9', {
requestedAt: NOW,
incarnationId: 'inc-z',
attempts: 0
})
store.flush()
const reloaded = await createStore()
expect(reloaded.getSshRemotePtyKillIntents('ssh-1', NOW).map((e) => e.ptyId)).toEqual(['pty-9'])
})
// The row it invents must not be reattachable: the user closed this PTY, and reattach fences only
// on paneKey/tabId — neither of which a row created here carries.
it('creates the missing lease terminated so reattach cannot adopt it', async () => {
const store = await createStore()
store.recordSshRemotePtyKillIntent('ssh-1', 'pty-9', {
requestedAt: NOW,
incarnationId: 'inc-z',
attempts: 0
})
const lease = store.getSshRemotePtyLeases('ssh-1')[0]
expect(lease?.state).toBe('terminated')
expect(lease?.tabId).toBeUndefined()
expect(lease?.leafId).toBeUndefined()
})
it('deletes expired intents durably rather than only filtering them on read', async () => {
const store = await createStore()
store.upsertSshRemotePtyLease({ targetId: 'ssh-1', ptyId: 'pty-1', state: 'attached' })
store.recordSshRemotePtyKillIntent('ssh-1', 'pty-1', {
requestedAt: NOW,
incarnationId: 'inc-a',
attempts: 0
})
store.pruneExpiredSshRemotePtyKillIntents('ssh-1', NOW + SSH_PENDING_PTY_KILL_TTL_MS + 1)
store.flush()
const reloaded = await createStore()
expect(reloaded.getSshRemotePtyLeases('ssh-1')[0]?.pendingKill).toBeUndefined()
// Ageing out observes nothing about the process, so the lease state is left exactly as it was.
expect(reloaded.getSshRemotePtyLeases('ssh-1')[0]?.state).toBe('attached')
})
it('leaves an intent that is still inside its TTL alone', async () => {
const store = await createStore()
store.recordSshRemotePtyKillIntent('ssh-1', 'pty-1', {
requestedAt: NOW,
incarnationId: 'inc-a',
attempts: 0
})
store.pruneExpiredSshRemotePtyKillIntents('ssh-1', NOW + SSH_PENDING_PTY_KILL_TTL_MS)
expect(store.getSshRemotePtyKillIntents('ssh-1', NOW)).toHaveLength(1)
})
// The replay reads every lease state on purpose: the close path tombstones the lease locally and
// the remote process is still running, so a terminated lease is exactly where an order lives.
it('keeps an intent on a lease its own kill path tombstoned as terminated', async () => {
const store = await createStore()
store.upsertSshRemotePtyLease({ targetId: 'ssh-1', ptyId: 'pty-1', state: 'attached' })
store.markSshRemotePtyLease('ssh-1', 'pty-1', 'terminated')
store.recordSshRemotePtyKillIntent('ssh-1', 'pty-1', {
requestedAt: NOW,
incarnationId: 'inc-a',
attempts: 0
})
expect(store.getSshRemotePtyKillIntents('ssh-1', NOW)).toHaveLength(1)
})
it('retires an intent without touching the lease state', async () => {
const store = await createStore()
store.upsertSshRemotePtyLease({ targetId: 'ssh-1', ptyId: 'pty-1', state: 'attached' })
store.recordSshRemotePtyKillIntent('ssh-1', 'pty-1', {
requestedAt: NOW,
incarnationId: 'inc-a',
attempts: 0
})
store.clearSshRemotePtyKillIntent('ssh-1', 'pty-1')
store.flush()
const reloaded = await createStore()
expect(reloaded.getSshRemotePtyKillIntents('ssh-1', NOW)).toEqual([])
expect(reloaded.getSshRemotePtyLeases('ssh-1')[0]?.state).toBe('attached')
})
it('drops intents past the TTL on read', async () => {
const store = await createStore()
store.recordSshRemotePtyKillIntent('ssh-1', 'pty-1', {
requestedAt: NOW,
incarnationId: 'inc-a',
attempts: 0
})
expect(
store.getSshRemotePtyKillIntents('ssh-1', NOW + SSH_PENDING_PTY_KILL_TTL_MS)
).toHaveLength(1)
expect(
store.getSshRemotePtyKillIntents('ssh-1', NOW + SSH_PENDING_PTY_KILL_TTL_MS + 1)
).toEqual([])
})
it('caps pending kills per target so an unreachable host cannot grow the store', async () => {
const store = await createStore()
const total = MAX_SSH_PENDING_PTY_KILLS_PER_TARGET + 50
for (let index = 0; index < total; index++) {
store.recordSshRemotePtyKillIntent('ssh-1', `pty-${index}`, {
requestedAt: NOW + index,
incarnationId: `inc-${index}`,
attempts: 0
})
}
store.flush()
const reloaded = await createStore()
const persisted = reloaded
.getSshRemotePtyLeases('ssh-1')
.filter((lease) => lease.pendingKill !== undefined)
expect(persisted).toHaveLength(MAX_SSH_PENDING_PTY_KILLS_PER_TARGET)
// Newest kept: the oldest orders are the ones least likely to still name a live process.
expect(persisted.some((lease) => lease.ptyId === `pty-${total - 1}`)).toBe(true)
expect(persisted.some((lease) => lease.ptyId === 'pty-0')).toBe(false)
})
it('does not let a repeated close extend the TTL, and carries attempts forward', async () => {
const store = await createStore()
store.recordSshRemotePtyKillIntent('ssh-1', 'pty-1', {
requestedAt: NOW,
incarnationId: 'inc-a',
attempts: 0
})
store.noteSshRemotePtyKillReplayAttempt('ssh-1', 'pty-1')
store.recordSshRemotePtyKillIntent('ssh-1', 'pty-1', {
requestedAt: NOW + 5000,
incarnationId: 'inc-a',
attempts: 0
})
expect(store.getSshRemotePtyKillIntents('ssh-1', NOW)[0]?.intent).toEqual({
requestedAt: NOW,
incarnationId: 'inc-a',
attempts: 1
})
})
it('scopes intents to their own target', async () => {
const store = await createStore()
store.recordSshRemotePtyKillIntent('ssh-1', 'pty-1', {
requestedAt: NOW,
incarnationId: 'inc-a',
attempts: 0
})
expect(store.getSshRemotePtyKillIntents('ssh-2', NOW)).toEqual([])
})
})
@@ -4,6 +4,7 @@ import type {
SshTarget
} from '../../../shared/ssh-types'
import { LEGACY_DEFAULT_SSH_RELAY_GRACE_PERIOD_SECONDS } from '../../../shared/ssh-types'
import { normalizeSshPendingPtyKill } from '../../../shared/ssh-pending-pty-kill'
export type LegacySshTarget = SshTarget & {
remoteWorkspaceSyncEnabled?: unknown
@@ -60,9 +61,11 @@ export function normalizeSshRemotePtyLease(value: unknown): SshRemotePtyLease |
return null
}
const now = Date.now()
const pendingKill = normalizeSshPendingPtyKill(raw.pendingKill)
return {
targetId: raw.targetId,
ptyId: raw.ptyId,
...(pendingKill ? { pendingKill } : {}),
...(typeof raw.worktreeId === 'string' ? { worktreeId: raw.worktreeId } : {}),
...(typeof raw.tabId === 'string' ? { tabId: raw.tabId } : {}),
...(typeof raw.leafId === 'string' && raw.leafId.length <= 256 ? { leafId: raw.leafId } : {}),
@@ -0,0 +1,150 @@
import type { PersistedState } from '../../../shared/persisted-state-types'
import {
MAX_SSH_PENDING_PTY_KILLS_PER_TARGET,
isSshPendingPtyKillExpired,
pendingSshPtyKillEntries,
prunePendingSshPtyKills,
type SshPendingPtyKill,
type SshPendingPtyKillEntry
} from '../../../shared/ssh-pending-pty-kill'
import type { SshRemotePtyLease } from '../../../shared/ssh-types'
import type { SshPtyLeaseOperations } from './ssh-pty-lease-operations'
/** Every recorded-but-undelivered stop for a target, newest first, TTL-filtered and capped.
* Returned with stored (target-local) relay pty ids, which is what `pty.shutdown` takes. */
export function getSshRemotePtyKillIntents(
state: PersistedState,
targetId: string,
now: number
): SshPendingPtyKillEntry[] {
const leases = (state.sshRemotePtyLeases ?? []).filter((lease) => lease.targetId === targetId)
return prunePendingSshPtyKills(pendingSshPtyKillEntries(leases), now)
}
/** Deletes orders past their TTL, durably.
*
* The read path filters them out too, but filtering alone would leave the field on disk forever
* and make the cap the only thing that ever reclaimed it. This is the TTL retirement path, and it
* deliberately leaves `state` alone: an order aging out observes nothing about the process. */
export function pruneExpiredSshRemotePtyKillIntents(
operations: SshPtyLeaseOperations,
targetId: string,
now: number
): void {
let changed = false
for (const lease of operations.state.sshRemotePtyLeases ?? []) {
if (
lease.targetId === targetId &&
lease.pendingKill &&
isSshPendingPtyKillExpired(lease.pendingKill, now)
) {
delete lease.pendingKill
lease.updatedAt = now
changed = true
}
}
if (changed) {
operations.flush()
}
}
/** Drops the oldest intents past the cap so an unreachable target cannot grow the store. Runs over
* the whole target rather than the arriving id, because the cap is what bounds the target. */
function capPendingKillsForTarget(
leases: SshRemotePtyLease[],
targetId: string,
now: number
): void {
const scoped = leases.filter((lease) => lease.targetId === targetId && lease.pendingKill)
if (scoped.length <= MAX_SSH_PENDING_PTY_KILLS_PER_TARGET) {
return
}
const kept = new Set(
prunePendingSshPtyKills(pendingSshPtyKillEntries(scoped), now).map((entry) => entry.ptyId)
)
for (const lease of scoped) {
if (!kept.has(lease.ptyId)) {
delete lease.pendingKill
lease.updatedAt = now
}
}
}
/** Records a stop this client asked for and could not confirm.
*
* Creates the lease when none exists — a kill that found no provider registered writes no lease of
* its own, and that offline close is the case most likely to strand a remote process. It is
* created `terminated`, carrying no pane identity, because the user closed this PTY: reattach must
* not adopt it. That is a routing tombstone and not a claim the process died — the process
* question is exactly what `pendingKill` is now tracking, and the replay reads every lease state. */
export function recordSshRemotePtyKillIntent(
operations: SshPtyLeaseOperations,
targetId: string,
ptyId: string,
intent: SshPendingPtyKill
): void {
const relayPtyId = operations.toStoredPtyId(targetId, ptyId)
const now = intent.requestedAt
operations.state.sshRemotePtyLeases ??= []
const leases = operations.state.sshRemotePtyLeases
const existing = leases.find((entry) => entry.targetId === targetId && entry.ptyId === relayPtyId)
if (existing) {
// Why keep the earliest requestedAt: the TTL bounds how long the intent may chase the host, and
// a repeated close must not extend it. Attempts carry over so replays stay countable.
existing.pendingKill = {
...intent,
requestedAt: Math.min(existing.pendingKill?.requestedAt ?? now, now),
attempts: existing.pendingKill?.attempts ?? intent.attempts
}
existing.updatedAt = now
} else {
leases.push({
targetId,
ptyId: relayPtyId,
state: 'terminated',
createdAt: now,
updatedAt: now,
pendingKill: intent
})
}
capPendingKillsForTarget(leases, targetId, now)
operations.flush()
}
/** Retires the intent. Deliberately does not touch `state`: the caller decides whether it earned a
* `terminated` tombstone, because only some retirement paths observed the host. */
export function clearSshRemotePtyKillIntent(
operations: SshPtyLeaseOperations,
targetId: string,
ptyId: string
): void {
const relayPtyId = operations.toStoredPtyId(targetId, ptyId)
const lease = (operations.state.sshRemotePtyLeases ?? []).find(
(entry) => entry.targetId === targetId && entry.ptyId === relayPtyId
)
if (!lease?.pendingKill) {
return
}
delete lease.pendingKill
lease.updatedAt = Date.now()
operations.flush()
}
/** Counts one replay against the intent so a target that never answers stays visible in the record
* without changing what it is allowed to do. */
export function noteSshRemotePtyKillReplayAttempt(
operations: SshPtyLeaseOperations,
targetId: string,
ptyId: string
): void {
const relayPtyId = operations.toStoredPtyId(targetId, ptyId)
const lease = (operations.state.sshRemotePtyLeases ?? []).find(
(entry) => entry.targetId === targetId && entry.ptyId === relayPtyId
)
if (!lease?.pendingKill) {
return
}
lease.pendingKill = { ...lease.pendingKill, attempts: lease.pendingKill.attempts + 1 }
lease.updatedAt = Date.now()
operations.flush()
}
@@ -1,4 +1,15 @@
import type {
SshPendingPtyKill,
SshPendingPtyKillEntry
} from '../../../shared/ssh-pending-pty-kill'
import type { SshPtyConsumerRecovery, SshRemotePtyLease } from '../../../shared/ssh-types'
import {
clearSshRemotePtyKillIntent as clearSshRemotePtyKillIntentOperation,
getSshRemotePtyKillIntents as getSshRemotePtyKillIntentsOperation,
noteSshRemotePtyKillReplayAttempt as noteSshRemotePtyKillReplayAttemptOperation,
pruneExpiredSshRemotePtyKillIntents as pruneExpiredSshRemotePtyKillIntentsOperation,
recordSshRemotePtyKillIntent as recordSshRemotePtyKillIntentOperation
} from '../leasing-ssh-ptys/ssh-pty-kill-intent-operations'
import {
getSshRemotePtyLeases as getSshRemotePtyLeasesOperation,
markSshRemotePtyLease as markSshRemotePtyLeaseOperation,
@@ -120,6 +131,30 @@ export class SshLeaseRecoveryOperations {
removeSshRemotePtyLeases(targetId: string): void {
removeSshRemotePtyLeasesOperation(getSshPtyLeaseOperations(this), targetId)
}
getSshRemotePtyKillIntents(targetId: string, now = Date.now()): SshPendingPtyKillEntry[] {
return getSshRemotePtyKillIntentsOperation(
this[sshLeaseRecoveryOperationsContext].runtime.state,
targetId,
now
)
}
recordSshRemotePtyKillIntent(targetId: string, ptyId: string, intent: SshPendingPtyKill): void {
recordSshRemotePtyKillIntentOperation(getSshPtyLeaseOperations(this), targetId, ptyId, intent)
}
clearSshRemotePtyKillIntent(targetId: string, ptyId: string): void {
clearSshRemotePtyKillIntentOperation(getSshPtyLeaseOperations(this), targetId, ptyId)
}
noteSshRemotePtyKillReplayAttempt(targetId: string, ptyId: string): void {
noteSshRemotePtyKillReplayAttemptOperation(getSshPtyLeaseOperations(this), targetId, ptyId)
}
pruneExpiredSshRemotePtyKillIntents(targetId: string, now = Date.now()): void {
pruneExpiredSshRemotePtyKillIntentsOperation(getSshPtyLeaseOperations(this), targetId, now)
}
}
export function getSshPtyConsumerRecoveryOperations(
@@ -0,0 +1,372 @@
import { describe, expect, it, vi, beforeEach } from 'vitest'
import { replayPendingSshPtyKills } from './ssh-pending-pty-kill-replay'
import { SshPtyAbsentFromRelayError } from '../providers/ssh-pty-errors'
import type { SshPendingPtyKillEntry } from '../../shared/ssh-pending-pty-kill'
import {
isSshPendingPtyKillExpired,
prunePendingSshPtyKills,
SSH_PENDING_PTY_KILL_TTL_MS
} from '../../shared/ssh-pending-pty-kill'
import type { Store } from '../persistence'
import type { IPtyProvider } from '../providers/types'
const TARGET = 'ssh-1'
const NOW = 1_800_000_000_000
type HostPty = { relayPtyId: string; incarnationId?: string }
/** Models the real store rather than returning a fixed list: `getSshRemotePtyKillIntents` applies
* the same TTL filter production does, and the prune actually deletes. A stub that ignored `now`
* would make every TTL assertion below pass for the wrong reason. */
function createStoreStub(entries: SshPendingPtyKillEntry[]): {
store: Store
cleared: string[]
terminated: string[]
expired: string[]
attempts: string[]
remaining: () => string[]
} {
let backing = [...entries]
const cleared: string[] = []
const terminated: string[] = []
const expired: string[] = []
const attempts: string[] = []
const store = {
getSshRemotePtyKillIntents: vi.fn((_target: string, now: number) =>
prunePendingSshPtyKills(backing, now)
),
pruneExpiredSshRemotePtyKillIntents: vi.fn((_target: string, now: number) => {
backing = backing.filter((item) => !isSshPendingPtyKillExpired(item.intent, now))
}),
clearSshRemotePtyKillIntent: vi.fn((_target: string, ptyId: string) => {
backing = backing.filter((item) => item.ptyId !== ptyId)
cleared.push(ptyId)
}),
markSshRemotePtyLease: vi.fn((_target: string, ptyId: string, state: string) => {
if (state === 'terminated') {
terminated.push(ptyId)
} else if (state === 'expired') {
expired.push(ptyId)
}
}),
noteSshRemotePtyKillReplayAttempt: vi.fn((_target: string, ptyId: string) => {
attempts.push(ptyId)
})
} as unknown as Store
return {
store,
cleared,
terminated,
expired,
attempts,
remaining: () => backing.map((item) => item.ptyId)
}
}
/** A relay that lists `hostPtys`, and drops an id from that listing once it is shut down. */
function createProviderStub(hostPtys: HostPty[], overrides: Partial<IPtyProvider> = {}) {
const live = new Map(hostPtys.map((pty) => [pty.relayPtyId, pty.incarnationId]))
const shutdown = vi.fn(async (appPtyId: string) => {
live.delete(appPtyId.replace(`ssh:${TARGET}@@`, ''))
})
const listProcesses = vi.fn(async () =>
Array.from(live, ([relayPtyId, incarnationId]) => ({
id: `ssh:${TARGET}@@${relayPtyId}`,
...(incarnationId ? { incarnationId } : {})
}))
)
return {
provider: { listProcesses, shutdown, ...overrides } as unknown as IPtyProvider,
listProcesses,
shutdown
}
}
function entry(relayPtyId: string, incarnationId: string, requestedAt = NOW) {
return { ptyId: relayPtyId, intent: { requestedAt, incarnationId, attempts: 0 } }
}
describe('replayPendingSshPtyKills', () => {
beforeEach(() => {
vi.spyOn(console, 'warn').mockImplementation(() => {})
vi.spyOn(console, 'log').mockImplementation(() => {})
})
it('does not talk to the host when nothing is pending', async () => {
const { store } = createStoreStub([])
const { provider, listProcesses } = createProviderStub([])
await replayPendingSshPtyKills({
targetId: TARGET,
store,
provider,
shouldContinue: () => true,
now: () => NOW
})
expect(listProcesses).not.toHaveBeenCalled()
})
// This is the leak: yesterday nothing retried, and the shell stayed up on the user's box.
it('replays the undelivered stop against the same PTY the kill was aimed at', async () => {
const { store, cleared, terminated } = createStoreStub([entry('pty-1', 'inc-a')])
const { provider, shutdown } = createProviderStub([
{ relayPtyId: 'pty-1', incarnationId: 'inc-a' }
])
await replayPendingSshPtyKills({
targetId: TARGET,
store,
provider,
shouldContinue: () => true,
now: () => NOW
})
expect(shutdown).toHaveBeenCalledWith('ssh:ssh-1@@pty-1', { immediate: true })
expect(cleared).toEqual(['pty-1'])
expect(terminated).toEqual(['pty-1'])
})
// #16970: a redeployed relay renumbers from pty-1, so this id now names someone else's shell.
it('refuses to kill a recycled relay id and expires the lease that named it', async () => {
const { store, cleared, terminated, expired } = createStoreStub([entry('pty-1', 'inc-a')])
const { provider, shutdown } = createProviderStub([
{ relayPtyId: 'pty-1', incarnationId: 'inc-fresh' }
])
await replayPendingSshPtyKills({
targetId: TARGET,
store,
provider,
shouldContinue: () => true,
now: () => NOW
})
expect(shutdown).not.toHaveBeenCalled()
expect(cleared).toEqual(['pty-1'])
// Declining to kill is only half of it: the reattach one step later fences on paneKey/tabId and
// never on incarnation, so an untouched lease would bind the user's old pane to that stranger.
expect(expired).toEqual(['pty-1'])
// `expired`, not `terminated` — losing the route is not evidence the shell died.
expect(terminated).toEqual([])
})
it('retires on the host reporting the PTY absent, with the tombstone that earns', async () => {
const { store, cleared, terminated } = createStoreStub([entry('pty-1', 'inc-a')])
const { provider, shutdown } = createProviderStub([
{ relayPtyId: 'pty-2', incarnationId: 'inc-b' }
])
await replayPendingSshPtyKills({
targetId: TARGET,
store,
provider,
shouldContinue: () => true,
now: () => NOW
})
expect(shutdown).not.toHaveBeenCalled()
expect(cleared).toEqual(['pty-1'])
expect(terminated).toEqual(['pty-1'])
})
// The TTL is owned by the durable prune, not by a branch in the decision function. This asserts
// the order is actually deleted and never aimed at whatever holds the id now — and it can only
// pass if the prune honours `now`, because the store stub applies the real TTL rules.
it('deletes an order past its TTL instead of replaying it, and never touches the host', async () => {
const { store, cleared, terminated, expired, remaining } = createStoreStub([
entry('pty-1', 'inc-a')
])
const { provider, shutdown, listProcesses } = createProviderStub([
{ relayPtyId: 'pty-1', incarnationId: 'inc-a' }
])
await replayPendingSshPtyKills({
targetId: TARGET,
store,
provider,
shouldContinue: () => true,
now: () => NOW + SSH_PENDING_PTY_KILL_TTL_MS + 1
})
expect(remaining()).toEqual([])
expect(shutdown).not.toHaveBeenCalled()
expect(listProcesses).not.toHaveBeenCalled()
// Ageing out observes nothing, so it may not move the lease either way.
expect(cleared).toEqual([])
expect(terminated).toEqual([])
expect(expired).toEqual([])
})
// #16763's lesson: `isPtyAlreadyGoneError` matches message text such as /Session not found/i,
// which a transport failure could wear. A tombstone must never rest on that — only on a listing.
it('does not tombstone on a shutdown error that merely looks like absence', async () => {
const { store, cleared, terminated, remaining } = createStoreStub([entry('pty-1', 'inc-a')])
const { provider } = createProviderStub([{ relayPtyId: 'pty-1', incarnationId: 'inc-a' }], {
shutdown: vi.fn(async () => {
throw new SshPtyAbsentFromRelayError('SSH_SESSION_EXPIRED: pty-1')
})
})
await replayPendingSshPtyKills({
targetId: TARGET,
store,
provider,
shouldContinue: () => true,
now: () => NOW
})
expect(cleared).toEqual([])
expect(terminated).toEqual([])
expect(remaining()).toEqual(['pty-1'])
})
// Best-effort background work on the connect path: a disk hiccup must not fail the connection.
it('never rejects into its caller when persistence throws', async () => {
const { store } = createStoreStub([entry('pty-1', 'inc-a')])
vi.mocked(store.noteSshRemotePtyKillReplayAttempt).mockImplementation(() => {
throw new Error('disk full')
})
const { provider } = createProviderStub([{ relayPtyId: 'pty-1', incarnationId: 'inc-a' }])
await expect(
replayPendingSshPtyKills({
targetId: TARGET,
store,
provider,
shouldContinue: () => true,
now: () => NOW
})
).resolves.toBeUndefined()
})
it('never rejects into its caller when the very first store read throws', async () => {
const { store } = createStoreStub([entry('pty-1', 'inc-a')])
vi.mocked(store.pruneExpiredSshRemotePtyKillIntents).mockImplementation(() => {
throw new Error('disk full')
})
const { provider } = createProviderStub([{ relayPtyId: 'pty-1', incarnationId: 'inc-a' }])
await expect(
replayPendingSshPtyKills({
targetId: TARGET,
store,
provider,
shouldContinue: () => true,
now: () => NOW
})
).resolves.toBeUndefined()
})
it('keeps the order when the replayed stop is itself unverifiable', async () => {
const { store, cleared, terminated, attempts } = createStoreStub([entry('pty-1', 'inc-a')])
const { provider } = createProviderStub([{ relayPtyId: 'pty-1', incarnationId: 'inc-a' }], {
shutdown: vi.fn(async () => {
throw new Error('socket closed')
})
})
await replayPendingSshPtyKills({
targetId: TARGET,
store,
provider,
shouldContinue: () => true,
now: () => NOW
})
expect(attempts).toEqual(['pty-1'])
expect(cleared).toEqual([])
expect(terminated).toEqual([])
})
// `pty.shutdown` returns the same empty success for a PTY the relay never had, so only a fresh
// inventory that omits the id is a death certificate.
it('keeps the order when the host still lists the PTY after a resolved shutdown', async () => {
const { store, cleared, terminated } = createStoreStub([entry('pty-1', 'inc-a')])
const { provider } = createProviderStub([{ relayPtyId: 'pty-1', incarnationId: 'inc-a' }], {
shutdown: vi.fn(async () => {})
})
await replayPendingSshPtyKills({
targetId: TARGET,
store,
provider,
shouldContinue: () => true,
now: () => NOW
})
expect(cleared).toEqual([])
expect(terminated).toEqual([])
})
// Wire skew: a host predating the published PTY incarnation cannot answer the fence.
it('defers against a host that publishes no PTY incarnation', async () => {
const { store, cleared, terminated } = createStoreStub([entry('pty-1', 'inc-a')])
const { provider, shutdown } = createProviderStub([{ relayPtyId: 'pty-1' }])
await replayPendingSshPtyKills({
targetId: TARGET,
store,
provider,
shouldContinue: () => true,
now: () => NOW
})
expect(shutdown).not.toHaveBeenCalled()
expect(cleared).toEqual([])
expect(terminated).toEqual([])
})
it('keeps every order when the inventory itself fails', async () => {
const { store, cleared, terminated } = createStoreStub([
entry('pty-1', 'inc-a'),
entry('pty-2', 'inc-b')
])
const { provider } = createProviderStub([], {
listProcesses: vi.fn(async () => {
throw new Error('relay unreachable')
})
})
await replayPendingSshPtyKills({
targetId: TARGET,
store,
provider,
shouldContinue: () => true,
now: () => NOW
})
expect(cleared).toEqual([])
expect(terminated).toEqual([])
})
it('delivers nothing once the connection attempt is superseded', async () => {
const { store, cleared } = createStoreStub([entry('pty-1', 'inc-a'), entry('pty-2', 'inc-b')])
const { provider, shutdown } = createProviderStub([
{ relayPtyId: 'pty-1', incarnationId: 'inc-a' },
{ relayPtyId: 'pty-2', incarnationId: 'inc-b' }
])
await replayPendingSshPtyKills({
targetId: TARGET,
store,
provider,
shouldContinue: () => false,
now: () => NOW
})
expect(shutdown).not.toHaveBeenCalled()
expect(cleared).toEqual([])
})
// Bounded by wave, not by order: one inventory per wave of 4 plus one to prove the batch. The
// per-wave read is what keeps a stop's identity evidence within one round trip of the stop.
it('reads one inventory per wave plus one to confirm, not two per pending stop', async () => {
const pending = Array.from({ length: 12 }, (_, index) => entry(`pty-${index}`, `inc-${index}`))
const { store, terminated } = createStoreStub(pending)
const { provider, listProcesses, shutdown } = createProviderStub(
pending.map((item) => ({ relayPtyId: item.ptyId, incarnationId: item.intent.incarnationId }))
)
await replayPendingSshPtyKills({
targetId: TARGET,
store,
provider,
shouldContinue: () => true,
now: () => NOW
})
expect(shutdown).toHaveBeenCalledTimes(12)
expect(listProcesses).toHaveBeenCalledTimes(12 / 4 + 1)
expect(terminated).toHaveLength(12)
})
it('costs one inventory and one confirmation for a single pending stop', async () => {
const { store } = createStoreStub([entry('pty-1', 'inc-a')])
const { provider, listProcesses } = createProviderStub([
{ relayPtyId: 'pty-1', incarnationId: 'inc-a' }
])
await replayPendingSshPtyKills({
targetId: TARGET,
store,
provider,
shouldContinue: () => true,
now: () => NOW
})
expect(listProcesses).toHaveBeenCalledTimes(2)
})
})
+208
View File
@@ -0,0 +1,208 @@
import type { Store } from '../persistence'
import type { IPtyProvider } from '../providers/types'
import { toAppSshPtyId, toRelaySshPtyId } from '../providers/ssh-pty-id'
import {
decideSshPendingPtyKill,
type SshPendingPtyKillEntry,
type SshPendingPtyKillObservation,
type SshPendingPtyKillRetirement
} from '../../shared/ssh-pending-pty-kill'
export type SshPendingPtyKillReplayArgs = {
targetId: string
store: Store
provider: IPtyProvider
shouldContinue: () => boolean
now?: () => number
}
/** Stops issued per inventory read. Every stop in a wave is dispatched concurrently, immediately
* after the read that fenced it, so no kill is ever aimed with evidence older than one round trip. */
const REPLAY_WAVE_SIZE = 4
type HostInventory = Map<string, string | undefined>
/** Relay pty id -> the incarnation the host published for it, `undefined` when it published none. */
async function readHostInventory(args: SshPendingPtyKillReplayArgs): Promise<HostInventory> {
const processes = await args.provider.listProcesses()
const inventory: HostInventory = new Map()
for (const process of processes) {
inventory.set(toRelaySshPtyId(args.targetId, process.id), process.incarnationId)
}
return inventory
}
/** Retires one order.
*
* Both tombstoning reasons are derived from a successful `pty.listProcesses`, never from an error
* predicate: `isPtyAlreadyGoneError` matches on message text (`/Session not found/i`), which a
* transport failure could wear, and a tombstone written on that would bury a live remote shell.
* A recycled id gets `expired`, not `terminated` — the client has lost its route to that lease,
* which is all that was observed; nothing says the shell it named ever died. */
function retire(
args: SshPendingPtyKillReplayArgs,
relayPtyId: string,
reason: SshPendingPtyKillRetirement
): void {
args.store.clearSshRemotePtyKillIntent(args.targetId, relayPtyId)
if (reason === 'host-reports-absent' || reason === 'stop-confirmed') {
args.store.markSshRemotePtyLease(args.targetId, relayPtyId, 'terminated')
} else if (reason === 'relay-id-recycled') {
// Why this must happen: the reattach one step later filters on lease state and fences only on
// paneKey/tabId, never on incarnation. Leaving this lease active hands the user's old pane to
// whatever process now holds the recycled id.
args.store.markSshRemotePtyLease(args.targetId, relayPtyId, 'expired')
}
console.log(
`[ssh-pending-kill] retired stop for ${args.targetId}/${relayPtyId} (${reason.replace(/-/g, ' ')})`
)
}
function observe(inventory: HostInventory, relayPtyId: string): SshPendingPtyKillObservation {
return {
hostListsPty: inventory.has(relayPtyId),
hostIncarnationId: inventory.get(relayPtyId)
}
}
/** Applies every recorded order against one inventory, and returns the ones it says are safe to
* stop. Retirement side effects happen here, against that same fresh evidence. */
function selectReplayTargets(
args: SshPendingPtyKillReplayArgs,
inventory: HostInventory,
now: number,
/** Orders already dispatched this pass. Skipped whole: the shutdown they were issued is what
* removes them from the next inventory, so re-deciding would retire them a second time. */
attempted: ReadonlySet<string>
): SshPendingPtyKillEntry[] {
const replayable: SshPendingPtyKillEntry[] = []
for (const entry of args.store.getSshRemotePtyKillIntents(args.targetId, now)) {
if (attempted.has(entry.ptyId)) {
continue
}
const decision = decideSshPendingPtyKill(entry.intent, observe(inventory, entry.ptyId), now)
if (decision.action === 'retire') {
retire(args, entry.ptyId, decision.reason)
} else if (decision.action === 'defer') {
console.warn(
`[ssh-pending-kill] deferring stop for ${args.targetId}/${entry.ptyId}: ${decision.reason}`
)
} else {
replayable.push(entry)
}
}
return replayable
}
/** Issues one recorded stop. Returns false when it did not reach the host, in which case the order
* stays: a rejected RPC observes nothing, and the next handshake's inventory will answer.
*
* Re-checks the fence here rather than trusting the selection pass, so the identity proof and the
* irreversible call sit next to each other and cannot drift apart if this loop is ever reshaped.
* It still cannot be made atomic — `pty.shutdown` carries no incarnation, so only the host could
* refuse a stale kill. See the residual risk note in the PR. */
async function deliverReplay(
args: SshPendingPtyKillReplayArgs,
entry: SshPendingPtyKillEntry,
inventory: HostInventory,
now: number
): Promise<boolean> {
if (
decideSshPendingPtyKill(entry.intent, observe(inventory, entry.ptyId), now).action !== 'replay'
) {
return false
}
args.store.noteSshRemotePtyKillReplayAttempt(args.targetId, entry.ptyId)
try {
await args.provider.shutdown(toAppSshPtyId(args.targetId, entry.ptyId), { immediate: true })
return true
} catch (err) {
console.warn(
`[ssh-pending-kill] replay for ${args.targetId}/${entry.ptyId} is unverifiable: ${
err instanceof Error ? err.message : String(err)
}`
)
return false
}
}
/** Confirms delivered stops against one fresh inventory.
*
* Why re-list at all: the relay answers `pty.shutdown` for a PTY it never had with the same empty
* success, so a resolved RPC alone is not a death certificate. Absence from a live listing is. */
async function confirmDelivered(
args: SshPendingPtyKillReplayArgs,
awaitingProof: readonly string[]
): Promise<void> {
if (awaitingProof.length === 0) {
return
}
const inventory = await readHostInventory(args)
for (const relayPtyId of awaitingProof) {
if (!inventory.has(relayPtyId)) {
retire(args, relayPtyId, 'stop-confirmed')
} else {
console.warn(
`[ssh-pending-kill] ${args.targetId}/${relayPtyId} is still live after a replayed stop`
)
}
}
}
/** Replays every stop this client could not deliver to `targetId`, now that it is reachable again.
*
* Runs before reattach so a PTY that dies here is never re-adopted as a live pane. Costs nothing
* when nothing is pending. When something is, it re-reads the inventory once per wave rather than
* once per batch: the fence and the stops it authorises are then never more than one round trip
* apart, which is as tight as this can get while `pty.shutdown` carries no incarnation of its own.
*
* Never throws. It is best-effort work on the connect path, and `establish()` treats a throw here
* as a failed connection. */
export async function replayPendingSshPtyKills(args: SshPendingPtyKillReplayArgs): Promise<void> {
const now = args.now ?? Date.now
try {
// Inside the guard with everything else: these touch persistence, and a disk hiccup must not
// turn best-effort cleanup into a failed SSH connection.
args.store.pruneExpiredSshRemotePtyKillIntents(args.targetId, now())
if (args.store.getSshRemotePtyKillIntents(args.targetId, now()).length === 0) {
return
}
const attempted = new Set<string>()
const awaitingProof: string[] = []
while (args.shouldContinue()) {
// No inventory means no fence, and an unfenced replay can kill a shell nobody asked to close.
const inventory = await readHostInventory(args)
const selected = selectReplayTargets(args, inventory, now(), attempted)
const wave = selected.slice(0, REPLAY_WAVE_SIZE)
if (wave.length === 0) {
break
}
for (const entry of wave) {
attempted.add(entry.ptyId)
}
const delivered = await Promise.all(
wave.map(async (entry) =>
(await deliverReplay(args, entry, inventory, now())) ? entry.ptyId : null
)
)
for (const relayPtyId of delivered) {
if (relayPtyId !== null) {
awaitingProof.push(relayPtyId)
}
}
if (selected.length <= REPLAY_WAVE_SIZE) {
// This wave took everything the inventory offered, so another read would only confirm that.
break
}
}
if (args.shouldContinue()) {
await confirmDelivered(args, awaitingProof)
}
} catch (err) {
console.warn(
`[ssh-pending-kill] replay pass on ${args.targetId} stopped early; stops stay pending: ${
err instanceof Error ? err.message : String(err)
}`
)
}
}
@@ -160,7 +160,12 @@ function createSession(targetId: string): InstanceType<typeof SshRelaySession> {
markSshRemotePtyLease: vi.fn(),
markSshRemotePtyLeases: vi.fn(),
markSshRemotePtyLeasesAsync: vi.fn(),
markSshRemotePtyLeasesAttachedAsync: vi.fn()
markSshRemotePtyLeasesAttachedAsync: vi.fn(),
getSshRemotePtyKillIntents: vi.fn().mockReturnValue([]),
pruneExpiredSshRemotePtyKillIntents: vi.fn(),
recordSshRemotePtyKillIntent: vi.fn(),
clearSshRemotePtyKillIntent: vi.fn(),
noteSshRemotePtyKillReplayAttempt: vi.fn()
} as unknown as Store
const portForwardManager = {
removeAllForwards: vi.fn().mockResolvedValue(undefined)
@@ -0,0 +1,144 @@
import { describe, it, expect, vi, beforeEach } from 'vitest'
import { SshRelaySession } from './ssh-relay-session'
import { createMockDeps, mockDeploySuccess } from './ssh-relay-session-test-fixtures'
const { muxRequestMock, openConsumerSessionMock, listProcessesMock, shutdownMock } = vi.hoisted(
() => ({
muxRequestMock: vi.fn(),
openConsumerSessionMock: vi.fn(),
listProcessesMock: vi.fn(),
shutdownMock: vi.fn()
})
)
vi.mock('./ssh-relay-deploy', () => ({ deployAndLaunchRelay: vi.fn() }))
vi.mock('./ssh-pty-consumer-session', () => ({
openSshPtyConsumerSession: openConsumerSessionMock
}))
vi.mock('../ipc/ssh-pty-output-intake-registry', () => ({
acceptSshPtyOutputData: vi.fn().mockResolvedValue(undefined),
acceptSshPtyOutputExit: vi.fn().mockResolvedValue(undefined),
allocateSshPtyProviderGeneration: vi.fn(() => 41),
beginSshPtyOutputGenerationMigration: vi.fn(() => ({
byPty: new Map(),
completion: Promise.resolve()
})),
closeSshPtyOutputGeneration: vi.fn(),
getSshPtyAcceptedSourceCheckpoints: vi.fn(() => []),
applySshPtySourceCancellationProof: vi.fn(() => true),
applySshPtySourceRecoveryCancellationProof: vi.fn(() => true),
installSshPtySourceAckPublisher: vi.fn(() => () => {}),
installSshPtySourceCancellationPublisher: vi.fn(() => () => {})
}))
vi.mock('./ssh-relay-deploy-helpers', () => ({ execCommand: vi.fn().mockResolvedValue('') }))
vi.mock('./ssh-channel-multiplexer', () => ({
SshChannelMultiplexer: class MockSshChannelMultiplexer {
notify = vi.fn()
notifyWithSettlement = vi.fn()
request = muxRequestMock
onNotification = vi.fn().mockReturnValue(() => {})
onNotificationByMethod = vi.fn().mockReturnValue(() => {})
onRequest = vi.fn().mockReturnValue(() => {})
onDispose = vi.fn().mockReturnValue(() => {})
dispose = vi.fn()
isDisposed = vi.fn().mockReturnValue(false)
}
}))
vi.mock('../providers/ssh-pty-provider', () => ({
SshPtyProvider: class MockSshPtyProvider {
onData = vi.fn().mockReturnValue(() => {})
onReplay = vi.fn().mockReturnValue(() => {})
onExit = vi.fn().mockReturnValue(() => {})
attach = vi.fn().mockResolvedValue(undefined)
attachForReconnect = vi.fn().mockResolvedValue({})
setPtyDeliveryPauseAdapter = vi.fn()
dispose = vi.fn()
}
}))
vi.mock('../providers/ssh-filesystem-provider', () => ({
SshFilesystemProvider: class MockSshFilesystemProvider {
dispose = vi.fn()
}
}))
vi.mock('../providers/ssh-git-provider', () => ({ SshGitProvider: class MockSshGitProvider {} }))
vi.mock('../ipc/pty', () => ({
registerSshPtyProvider: vi.fn(),
unregisterSshPtyProvider: vi.fn(),
getSshPtyProvider: vi.fn().mockReturnValue({
dispose: vi.fn(),
attach: vi.fn().mockResolvedValue(undefined),
attachForReconnect: vi.fn().mockResolvedValue({}),
listProcesses: listProcessesMock,
shutdown: shutdownMock
}),
getPtyIdsForConnection: vi.fn().mockReturnValue([]),
clearPtyOwnershipForConnection: vi.fn(),
clearProviderPtyState: vi.fn(),
deletePtyOwnership: vi.fn(),
setPtyOwnership: vi.fn(),
restorePtyIncarnation: vi.fn(),
isCurrentPtyExit: vi.fn(() => true)
}))
vi.mock('../providers/ssh-filesystem-dispatch', () => ({
registerSshFilesystemProvider: vi.fn(),
unregisterSshFilesystemProvider: vi.fn(),
getSshFilesystemProvider: vi.fn().mockReturnValue({ dispose: vi.fn() })
}))
vi.mock('../providers/ssh-git-dispatch', () => ({
registerSshGitProvider: vi.fn(),
unregisterSshGitProvider: vi.fn()
}))
const NOW = 1_800_000_000_000
describe('SshRelaySession pending PTY kill replay', () => {
beforeEach(() => {
vi.clearAllMocks()
vi.spyOn(console, 'log').mockImplementation(() => {})
openConsumerSessionMock.mockImplementation(async (_mux, options) => ({
mode: 'legacy-fallback',
clientInstanceId: options.clientInstanceId,
serverBuildId: 'test-relay-build'
}))
muxRequestMock.mockReset()
muxRequestMock.mockResolvedValue([])
mockDeploySuccess()
})
it('replays a stop the client could not deliver, before reattach reads the leases', async () => {
const { mockConn, mockStore, mockPortForward, getMainWindow } = createMockDeps()
const order: string[] = []
vi.mocked(mockStore.getSshRemotePtyKillIntents).mockReturnValue([
{ ptyId: 'pty-3', intent: { requestedAt: NOW, incarnationId: 'inc-a', attempts: 0 } }
])
vi.mocked(mockStore.getSshRemotePtyLeases).mockImplementation(() => {
order.push('read-leases')
return []
})
let live = true
listProcessesMock.mockImplementation(async () =>
live ? [{ id: 'ssh:target-1@@pty-3', incarnationId: 'inc-a' }] : []
)
shutdownMock.mockImplementation(async (id: string) => {
order.push(`shutdown:${id}`)
live = false
})
const session = new SshRelaySession('target-1', getMainWindow, mockStore, mockPortForward)
await session.establish(mockConn)
expect(shutdownMock).toHaveBeenCalledWith('ssh:target-1@@pty-3', { immediate: true })
expect(order).toEqual(['shutdown:ssh:target-1@@pty-3', 'read-leases'])
expect(mockStore.clearSshRemotePtyKillIntent).toHaveBeenCalledWith('target-1', 'pty-3')
expect(mockStore.markSshRemotePtyLease).toHaveBeenCalledWith('target-1', 'pty-3', 'terminated')
})
it('costs no host round trip when nothing is pending', async () => {
const { mockConn, mockStore, mockPortForward, getMainWindow } = createMockDeps()
const session = new SshRelaySession('target-1', getMainWindow, mockStore, mockPortForward)
await session.establish(mockConn)
expect(listProcessesMock).not.toHaveBeenCalled()
expect(shutdownMock).not.toHaveBeenCalled()
})
})
@@ -107,7 +107,12 @@ function createMockDeps(): {
markSshRemotePtyLease: vi.fn(),
markSshRemotePtyLeases: vi.fn(),
markSshRemotePtyLeasesAsync: vi.fn(),
markSshRemotePtyLeasesAttachedAsync: vi.fn()
markSshRemotePtyLeasesAttachedAsync: vi.fn(),
getSshRemotePtyKillIntents: vi.fn().mockReturnValue([]),
pruneExpiredSshRemotePtyKillIntents: vi.fn(),
recordSshRemotePtyKillIntent: vi.fn(),
clearSshRemotePtyKillIntent: vi.fn(),
noteSshRemotePtyKillReplayAttempt: vi.fn()
} as unknown as Store
const mockPortForward = {
removeAllForwards: vi.fn()
@@ -26,6 +26,11 @@ export function createMockDeps(): SshRelaySessionTestDeps {
markSshRemotePtyLeasesAsync: vi.fn(),
markSshRemotePtyLeasesForShutdown: vi.fn(),
markSshRemotePtyLeasesAttachedAsync: vi.fn(),
getSshRemotePtyKillIntents: vi.fn().mockReturnValue([]),
pruneExpiredSshRemotePtyKillIntents: vi.fn(),
recordSshRemotePtyKillIntent: vi.fn(),
clearSshRemotePtyKillIntent: vi.fn(),
noteSshRemotePtyKillReplayAttempt: vi.fn(),
persistPtyBinding: vi.fn()
} as unknown as Store
const mockPortForward = {
+18 -5
View File
@@ -6,6 +6,7 @@ import type { BrowserWindow } from 'electron'
import { deployAndLaunchRelay } from './ssh-relay-deploy'
import { execCommand } from './ssh-relay-deploy-helpers'
import { isRelayVersionMismatchError } from './ssh-relay-version-mismatch-error'
import { replayPendingSshPtyKills } from './ssh-pending-pty-kill-replay'
import { SshChannelMultiplexer } from './ssh-channel-multiplexer'
import { SshPtyProvider } from '../providers/ssh-pty-provider'
import type { SshPtyAttachResult } from '../providers/ssh-pty-session-reattach'
@@ -2252,6 +2253,23 @@ export class SshRelaySession {
mux: SshChannelMultiplexer,
shouldContinue: () => boolean
): Promise<void> {
const ptyProvider = getSshPtyProvider(this.targetId) as SshPtyProvider | undefined
const providerGeneration = this.activePtyProviderGeneration
if (!ptyProvider || providerGeneration === null || this.mux !== mux) {
return
}
// Why before the lease read: a stop the user asked for and this client could not deliver is
// replayed here, and a confirmed one tombstones its lease — so it must land before the filter
// below decides what to reattach, or the reattach revives a PTY that is about to die.
await replayPendingSshPtyKills({
targetId: this.targetId,
store: this.store,
provider: ptyProvider,
shouldContinue
})
if (!shouldContinue()) {
return
}
const activeLeases = this.store
.getSshRemotePtyLeases(this.targetId)
.filter((lease) => lease.state !== 'terminated' && lease.state !== 'expired')
@@ -2276,11 +2294,6 @@ export class SshRelaySession {
...leasedPtyIds
])
)
const ptyProvider = getSshPtyProvider(this.targetId) as SshPtyProvider | undefined
const providerGeneration = this.activePtyProviderGeneration
if (!ptyProvider || providerGeneration === null || this.mux !== mux) {
return
}
let nextPtyIndex = 0
const worker = async (): Promise<void> => {
while (shouldContinue()) {
+9 -1
View File
@@ -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<TerminalSideEffectBatch | null>
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: (
+18 -3
View File
@@ -1061,6 +1061,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
@@ -1315,10 +1317,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)
},
@@ -0,0 +1,60 @@
import type { AgentHookEventPayload } from '../shared/agent-hook-listener/listener-event'
import type { AgentHookSource } from '../shared/agent-hook-relay'
export type CachedPaneEnvelopeMeta = { source: AgentHookSource; env?: string; version?: string }
export type CachedPaneReplaySelection = {
event: AgentHookEventPayload
meta: CachedPaneEnvelopeMeta
}
/**
* Chooses which cached pane statuses a reconnecting client may be re-told about, and drops the rest.
*
* Two panes are refused, for opposite reasons:
* - a pane whose surface has been retired — the tab is gone, so replaying its status would hand the
* client a live agent pane nobody owns, which is what drives an auto-resume onto a transcript an
* orphaned agent is still writing;
* - a pane whose envelope metadata is missing — the status cache and the metadata map are populated
* and cleared in lockstep, so a key present in one and not the other is drift. Skip rather than
* guess a source that mis-tags the replay downstream.
*/
export function selectReplayableCachedPanes(input: {
cachedByPaneKey: ReadonlyMap<string, AgentHookEventPayload>
metaByPaneKey: ReadonlyMap<string, CachedPaneEnvelopeMeta>
isPaneSurfaceRetired: (paneKey: string) => boolean
dropPane: (paneKey: string) => void
}): CachedPaneReplaySelection[] {
const selected: CachedPaneReplaySelection[] = []
// Snapshot: dropping a retired pane mutates the map being read.
for (const [paneKey, event] of Array.from(input.cachedByPaneKey.entries())) {
if (input.isPaneSurfaceRetired(paneKey)) {
input.dropPane(paneKey)
continue
}
const meta = input.metaByPaneKey.get(paneKey)
if (meta) {
selected.push({ event, meta })
}
}
return selected
}
/** How many panes keep a cached status. Bounds a long-lived relay's per-pane state. */
export const MAX_CACHED_PANES = 256
/** Drop the longest-idle panes until the cache is back under its cap. Map insertion order is
* recency (writers delete-then-set), so the first key is always the oldest. */
export function evictCachedPanesOverCap(
cachedByPaneKey: ReadonlyMap<string, unknown>,
dropPane: (paneKey: string) => void,
maxPanes: number = MAX_CACHED_PANES
): void {
while (cachedByPaneKey.size > maxPanes) {
const oldest = cachedByPaneKey.keys().next().value
if (oldest === undefined) {
return
}
dropPane(oldest)
}
}
@@ -0,0 +1,206 @@
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
import { mkdtempSync, rmSync } from 'node:fs'
import { tmpdir } from 'node:os'
import { join } from 'node:path'
import { RelayAgentHookServer } from './agent-hook-server'
import { RelayAgentHookRuntime } from './relay-agent-hook-runtime'
import { RetiredPaneSurfaceRegistry } from './retired-pane-surfaces'
import type { AgentHookRelayEnvelope } from '../shared/agent-hook-relay'
import { makePaneKey } from '../shared/stable-pane-id'
import type { PtyHandler, PtySurfaceRetiredListener } from './pty-handler'
import type { RelayDispatcher } from './dispatcher'
const LEAF_ID = '11111111-1111-4111-8111-111111111111'
const PANE_KEY = makePaneKey('tab-1', LEAF_ID)
function hookMeta(paneKey: string): string {
return Buffer.from([paneKey, 'tab-1', '', 'wt-1', 'remote', '1'].join('\x1f')).toString('base64')
}
async function postHook(
server: RelayAgentHookServer,
paneKey: string,
prompt = 'hi'
): Promise<number> {
const { port, token } = server.getCoordinates()
const res = await fetch(`http://127.0.0.1:${port}/hook/claude`, {
method: 'POST',
headers: {
'Content-Type': 'application/json',
'X-Orca-Agent-Hook-Token': token,
'X-Orca-Agent-Hook-Meta-Encoding': 'base64',
'X-Orca-Agent-Hook-Meta': hookMeta(paneKey)
},
body: JSON.stringify({ hook_event_name: 'UserPromptSubmit', prompt })
})
return res.status
}
type CachedPanes = { state: { lastStatusByPaneKey: Map<string, unknown> } }
function cachedPaneKeys(server: RelayAgentHookServer): string[] {
return [...(server as unknown as CachedPanes).state.lastStatusByPaneKey.keys()]
}
describe('relay hook forwarding for a retired pane surface', () => {
let dir: string
let servers: RelayAgentHookServer[]
beforeEach(() => {
dir = mkdtempSync(join(tmpdir(), 'relay-retired-pane-'))
servers = []
})
afterEach(() => {
for (const server of servers) {
server.stop()
}
rmSync(dir, { recursive: true, force: true })
})
async function startServer(options: {
forward: (envelope: AgentHookRelayEnvelope) => void
isPaneSurfaceRetired?: (paneKey: string) => boolean
}): Promise<RelayAgentHookServer> {
const server = new RelayAgentHookServer({ endpointDir: dir, ...options })
servers.push(server)
await server.start({ publishEndpoint: false })
return server
}
it('forwards and caches a post from a pane that still has a tab', async () => {
const retired = new RetiredPaneSurfaceRegistry()
const forward = vi.fn<(envelope: AgentHookRelayEnvelope) => void>()
const server = await startServer({
forward,
isPaneSurfaceRetired: (paneKey) => retired.isRetired(paneKey)
})
expect(await postHook(server, PANE_KEY)).toBe(204)
expect(forward).toHaveBeenCalledTimes(1)
expect(cachedPaneKeys(server)).toEqual([PANE_KEY])
})
it('does not advertise an orphan agent whose tab was closed', async () => {
const retired = new RetiredPaneSurfaceRegistry()
const forward = vi.fn<(envelope: AgentHookRelayEnvelope) => void>()
const server = await startServer({
forward,
isPaneSurfaceRetired: (paneKey) => retired.isRetired(paneKey)
})
expect(await postHook(server, PANE_KEY, 'first')).toBe(204)
forward.mockClear()
// The user closes the tab; the agent inside the pane shell survives and keeps posting.
retired.retire(PANE_KEY)
expect(await postHook(server, PANE_KEY, 'orphan')).toBe(204)
expect(forward).not.toHaveBeenCalled()
// The pre-close status must go too, or a reconnect replays it as a live agent pane.
expect(cachedPaneKeys(server)).toEqual([])
})
it('does not replay a cached status for a pane retired after it was cached', async () => {
const retired = new RetiredPaneSurfaceRegistry()
const forward = vi.fn<(envelope: AgentHookRelayEnvelope) => void>()
const server = await startServer({
forward,
isPaneSurfaceRetired: (paneKey) => retired.isRetired(paneKey)
})
expect(await postHook(server, PANE_KEY)).toBe(204)
forward.mockClear()
retired.retire(PANE_KEY)
expect(server.replayCachedPayloadsForPanes()).toBe(0)
expect(forward).not.toHaveBeenCalled()
expect(cachedPaneKeys(server)).toEqual([])
})
it('forwards again once the pane key is bound by a new PTY', async () => {
const retired = new RetiredPaneSurfaceRegistry()
const forward = vi.fn<(envelope: AgentHookRelayEnvelope) => void>()
const server = await startServer({
forward,
isPaneSurfaceRetired: (paneKey) => retired.isRetired(paneKey)
})
retired.retire(PANE_KEY)
expect(await postHook(server, PANE_KEY, 'orphan')).toBe(204)
expect(forward).not.toHaveBeenCalled()
retired.restore(PANE_KEY)
expect(await postHook(server, PANE_KEY, 'reopened')).toBe(204)
expect(forward).toHaveBeenCalledTimes(1)
})
it('an embedder that supplies no retirement source keeps forwarding everything', async () => {
const forward = vi.fn<(envelope: AgentHookRelayEnvelope) => void>()
const server = await startServer({ forward })
expect(await postHook(server, PANE_KEY)).toBe(204)
expect(forward).toHaveBeenCalledTimes(1)
expect(cachedPaneKeys(server)).toEqual([PANE_KEY])
})
})
describe('RelayAgentHookRuntime wiring', () => {
let dir: string
beforeEach(() => {
dir = mkdtempSync(join(tmpdir(), 'relay-retired-pane-runtime-'))
})
afterEach(() => {
rmSync(dir, { recursive: true, force: true })
})
it('routes hook admission through the PTY handler and drops the cache on retirement', async () => {
const retired = new RetiredPaneSurfaceRegistry()
const surfaceRetiredListeners: PtySurfaceRetiredListener[] = []
const ptyHandler = {
addEnvAugmenter: vi.fn(),
setExitListener: vi.fn(),
setSurfaceRetiredListener: vi.fn((listener: PtySurfaceRetiredListener | null) => {
if (listener) {
surfaceRetiredListeners.push(listener)
}
}),
isPaneSurfaceRetired: (paneKey: string) => retired.isRetired(paneKey)
} as unknown as PtyHandler
// Zero attached clients: publishAgentHookEnvelope returns before touching any sink.
const dispatcher = {
onRequest: vi.fn(),
activeClientIds: () => [] as number[]
} as unknown as RelayDispatcher
const runtime = new RelayAgentHookRuntime(
dispatcher,
ptyHandler,
join(dir, 'relay.sock'),
join(dir, 'hooks')
)
await runtime.start()
try {
const server = (runtime as unknown as { hookServer: RelayAgentHookServer }).hookServer
expect(await postHook(server, PANE_KEY)).toBe(204)
expect(cachedPaneKeys(server)).toEqual([PANE_KEY])
// The PTY handler retires the surface: the runtime must drop the cached status immediately,
// rather than waiting for a process exit a surviving shell never produces.
retired.retire(PANE_KEY)
expect(surfaceRetiredListeners).toHaveLength(1)
surfaceRetiredListeners[0]({ id: 'pty-1', paneKey: PANE_KEY })
expect(cachedPaneKeys(server)).toEqual([])
expect(await postHook(server, PANE_KEY, 'orphan')).toBe(204)
expect(cachedPaneKeys(server)).toEqual([])
} finally {
runtime.stop()
}
})
})
+30 -18
View File
@@ -41,11 +41,13 @@ import {
import { buildRelayHookPtyEnv, defaultEndpointDir } from './agent-hook-endpoint-coordinates'
import { buildRelayHookEnvelope, hookBodyEnv, hookBodyVersion } from './agent-hook-envelope-build'
import { AgentHookResultRetryScheduler } from './agent-hook-result-retry-scheduler'
import {
evictCachedPanesOverCap,
selectReplayableCachedPanes
} from './agent-hook-cached-pane-status'
export type RelayHookForward = (envelope: AgentHookRelayEnvelope) => void
const MAX_CACHED_PANES = 256
export type RelayHookServerOptions = {
/** Where to put endpoint.env / endpoint.cmd. Defaults to `$HOME/.orca-relay/agent-hooks`. */
endpointDir?: string
@@ -56,6 +58,13 @@ export type RelayHookServerOptions = {
/** Preferred bind port. WSL relay passes the Windows listener's port so env-sourced client coords stay truthful; falls back to :0 if occupied. Defaults to :0. */
preferredPort?: number
forward: RelayHookForward
/**
* True when the host has been told this pane's tab is gone and no PTY has re-bound the paneKey.
* Posts from such a pane come from a process the user already closed, so they describe no surface
* any client owns. Defaults to "never retired", which is the pre-existing behaviour — a listener
* with no PTY handler behind it (the WSL relay) keeps forwarding everything.
*/
isPaneSurfaceRetired?: (paneKey: string) => boolean
}
export type RelayHookServerStartOptions = {
@@ -81,6 +90,7 @@ export class RelayAgentHookServer {
{ source: AgentHookSource; env?: string; version?: string }
>()
private forward: RelayHookForward
private isPaneSurfaceRetired: (paneKey: string) => boolean
private fixedToken: string | undefined
private preferredPort: number
private portFallbackApplied = false
@@ -93,6 +103,7 @@ export class RelayAgentHookServer {
this.fixedToken = options.token
this.preferredPort = options.preferredPort ?? 0
this.forward = options.forward
this.isPaneSurfaceRetired = options.isPaneSurfaceRetired ?? (() => false)
this.retryScheduler = new AgentHookResultRetryScheduler({
state: this.state,
env: this.env,
@@ -198,19 +209,18 @@ export class RelayAgentHookServer {
/** Request-driven replay: re-forwards each cached paneKey payload as a fresh notification. Forwards are
* issued before the request handler returns, so the response trails all replayed notifications. */
replayCachedPayloadsForPanes(): number {
let count = 0
for (const [paneKey, event] of this.state.lastStatusByPaneKey.entries()) {
const meta = this.lastEnvelopeMetaByPaneKey.get(paneKey)
// Why: invariant — status-cache keys always have meta; if it drifted, skip rather than guess a source that mis-tags downstream.
if (!meta) {
continue
}
const replayable = selectReplayableCachedPanes({
cachedByPaneKey: this.state.lastStatusByPaneKey,
metaByPaneKey: this.lastEnvelopeMetaByPaneKey,
isPaneSurfaceRetired: this.isPaneSurfaceRetired,
dropPane: (paneKey) => this.clearPaneState(paneKey)
})
for (const { event, meta } of replayable) {
this.forward(
buildRelayHookEnvelope(event, meta.source, meta.env, meta.version, { isReplay: true })
)
count++
}
return count
return replayable.length
}
/** Drop a paneKey's cached entries on PTY exit so a terminated pane can't resurface as a ghost event on reconnect. */
@@ -301,6 +311,14 @@ export class RelayAgentHookServer {
version?: string,
options: { isReplay?: boolean } = {}
): void {
// Why: this post came from a process still running inside a pane whose tab the user closed.
// Caching or forwarding it makes every connected client advertise a live, resumable agent pane
// that no tab owns — the advertisement that ends up auto-typing a second `--resume` onto a
// transcript the orphan is still writing (#12447). Drop the stale cache with it.
if (this.isPaneSurfaceRetired(event.paneKey)) {
this.clearPaneState(event.paneKey)
return
}
if (event.payload.state !== 'done' || event.payload.lastAssistantMessage) {
this.retryScheduler.clearAssistantMessageRetry(event.paneKey)
}
@@ -313,13 +331,7 @@ export class RelayAgentHookServer {
this.state.lastStatusByPaneKey.set(event.paneKey, cachedEvent)
this.lastEnvelopeMetaByPaneKey.delete(event.paneKey)
this.lastEnvelopeMetaByPaneKey.set(event.paneKey, { source, env, version })
while (this.state.lastStatusByPaneKey.size > MAX_CACHED_PANES) {
const oldest = this.state.lastStatusByPaneKey.keys().next().value
if (oldest === undefined) {
break
}
this.clearPaneState(oldest)
}
evictCachedPanesOverCap(this.state.lastStatusByPaneKey, (key) => this.clearPaneState(key))
this.forward(buildRelayHookEnvelope(event, source, env, version, options))
}
@@ -0,0 +1,230 @@
import { describe, expect, it, vi, beforeEach, afterEach } from 'vitest'
const { mockPtySpawn, mockPtyInstance, mockCreateShellPromptReadinessProbe } = vi.hoisted(() => ({
mockPtySpawn: vi.fn(),
mockCreateShellPromptReadinessProbe: vi.fn(),
mockPtyInstance: {
pid: process.pid,
onData: vi.fn(),
onExit: vi.fn(),
write: vi.fn(),
resize: vi.fn(),
kill: vi.fn(),
clear: vi.fn(),
pause: vi.fn(),
resume: vi.fn()
}
}))
vi.mock('node-pty', () => ({
spawn: mockPtySpawn
}))
vi.mock('../main/pty/posix-pty-process-groups', () => ({
forceKillPosixPtyProcessGroups: vi.fn((_pid: number, fallback: () => void) => fallback())
}))
vi.mock('../main/shell-prompt-readiness-probe', () => ({
createShellPromptReadinessProbe: mockCreateShellPromptReadinessProbe
}))
import * as ptyShellUtils from './pty-shell-utils'
import type { PtyHandler } from './pty-handler'
import {
IMMEDIATE_PTY_EXIT_TIMEOUT_MS,
SHUTDOWN_REAP_MAX_SWEEPS,
SHUTDOWN_REAP_VERIFY_DELAY_MS
} from './pty-handler'
import {
beginPtyHandlerTest,
createPtyRequestHelpers,
endPtyHandlerTest
} from './pty-handler-test-harness'
import type { MockDispatcher } from './pty-handler-test-harness'
const PANE_KEY = 'tab-agent:22222222-2222-4222-8222-222222222222'
const AGENT_SESSION_ENSURE = {
claim: {
digestVersion: 1,
keyId: 'claim-key',
identityDigest: 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa',
worktreeScopeDigest: 'bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb',
agent: 'claude'
},
surface: {
worktreeId: 'repo::/tmp/worktree',
tabId: '11111111-1111-4111-8111-111111111111',
leafId: '22222222-2222-4222-8222-222222222222',
terminalHandle: 'term_claimed'
}
}
type ProcessSummary = {
id: string
agentSessionOwners?: unknown[]
}
describe('PtyHandler retires a closed pane surface', () => {
let dispatcher: MockDispatcher
let handler: PtyHandler
let originalPlatform: PropertyDescriptor | undefined
const { spawnPty } = createPtyRequestHelpers(() => dispatcher)
async function spawnAgentPane(
params: Record<string, unknown> = {}
): Promise<{ id: string; term: typeof mockPtyInstance }> {
const term = { ...mockPtyInstance, kill: vi.fn(), onData: vi.fn(), onExit: vi.fn() }
mockPtySpawn.mockReturnValue(term)
const spawned = await spawnPty({
env: { ORCA_PANE_KEY: PANE_KEY },
agentSessionEnsure: AGENT_SESSION_ENSURE,
...params
})
return { id: spawned.id, term }
}
async function listProcesses(): Promise<ProcessSummary[]> {
return (await dispatcher.callRequest('pty.listProcesses', {})) as ProcessSummary[]
}
beforeEach(() => {
;({ dispatcher, handler, originalPlatform } = beginPtyHandlerTest({
mockPtySpawn,
mockPtyInstance,
mockCreateShellPromptReadinessProbe
}))
})
afterEach(async () => {
await endPtyHandlerTest(handler, originalPlatform)
})
it('marks the pane surface retired as soon as its tab is closed', async () => {
const retired: { id: string; paneKey: string }[] = []
handler.setSurfaceRetiredListener((event) => retired.push(event))
const { id } = await spawnAgentPane()
expect(handler.isPaneSurfaceRetired(PANE_KEY)).toBe(false)
await dispatcher.callRequest('pty.shutdown', { id })
expect(handler.isPaneSurfaceRetired(PANE_KEY)).toBe(true)
expect(retired).toEqual([{ id, paneKey: PANE_KEY }])
})
it('retires the surface even when the shell survives the kill request', async () => {
vi.spyOn(ptyShellUtils, 'isProcessAlive').mockReturnValue(true)
const { id } = await spawnAgentPane()
await dispatcher.callRequest('pty.shutdown', { id })
await vi.advanceTimersByTimeAsync(
SHUTDOWN_REAP_VERIFY_DELAY_MS * (SHUTDOWN_REAP_MAX_SWEEPS + 2)
)
// The process could not be verified gone, so the claim is deliberately kept — releasing it is
// what would let a reopened project fork a second agent onto the same transcript.
expect(handler.isPaneSurfaceRetired(PANE_KEY)).toBe(true)
const sessions = await listProcesses()
expect(sessions.map((session) => session.id)).toEqual([id])
})
it('re-issues the force kill instead of letting a detached shell outlive its tab', async () => {
vi.spyOn(ptyShellUtils, 'isProcessAlive').mockReturnValue(true)
const { id, term } = await spawnAgentPane()
await dispatcher.callRequest('pty.shutdown', { id })
// The graceful path's own armed fallback fires first; count only what the sweep adds.
await vi.advanceTimersByTimeAsync(SHUTDOWN_REAP_VERIFY_DELAY_MS - 1)
const killsBeforeSweep = term.kill.mock.calls.filter(([signal]) => signal === 'SIGKILL').length
await vi.advanceTimersByTimeAsync(
SHUTDOWN_REAP_VERIFY_DELAY_MS * (SHUTDOWN_REAP_MAX_SWEEPS + 2)
)
const killsAfterSweep = term.kill.mock.calls.filter(([signal]) => signal === 'SIGKILL').length
expect(killsAfterSweep - killsBeforeSweep).toBe(SHUTDOWN_REAP_MAX_SWEEPS)
})
it('never re-closes a ConPTY handle on Windows, where the first kill is already final', async () => {
Object.defineProperty(process, 'platform', { configurable: true, value: 'win32' })
vi.spyOn(ptyShellUtils, 'isProcessAlive').mockReturnValue(true)
const { id, term } = await spawnAgentPane()
// The immediate path is the one that reaches ConPTY's kill without setting gracefulKillSent.
const shutdown = dispatcher.callRequest('pty.shutdown', { id, immediate: true })
const rejected = expect(shutdown).rejects.toThrow('Timed out waiting for PTY process exit')
await vi.advanceTimersByTimeAsync(IMMEDIATE_PTY_EXIT_TIMEOUT_MS)
await rejected
await vi.advanceTimersByTimeAsync(
SHUTDOWN_REAP_VERIFY_DELAY_MS * (SHUTDOWN_REAP_MAX_SWEEPS + 2)
)
expect(term.kill.mock.calls).toHaveLength(1)
})
it('retires the relay session once the sweep proves the process is gone', async () => {
const exits: { id: string; paneKey?: string }[] = []
handler.setExitListener((event) => exits.push(event))
const alive = vi.spyOn(ptyShellUtils, 'isProcessAlive').mockReturnValue(true)
const { id } = await spawnAgentPane()
expect((await listProcesses())[0]?.agentSessionOwners).toHaveLength(1)
await dispatcher.callRequest('pty.shutdown', { id })
alive.mockReturnValue(false)
await vi.advanceTimersByTimeAsync(SHUTDOWN_REAP_VERIFY_DELAY_MS + 1)
// The sweep itself must retire it — node-pty produced no exit, and nothing here polls a listing.
expect(exits).toEqual([{ id, paneKey: PANE_KEY }])
expect(await listProcesses()).toEqual([])
})
it('stops advertising an agent session whose process died without a node-pty exit', async () => {
const alive = vi.spyOn(ptyShellUtils, 'isProcessAlive').mockReturnValue(true)
await spawnAgentPane()
expect((await listProcesses())[0]?.agentSessionOwners).toHaveLength(1)
// No shutdown, no onExit — the shell just went away. The relay has to look.
alive.mockReturnValue(false)
expect(await listProcesses()).toEqual([])
})
it('restores the surface when a new PTY binds the same pane key', async () => {
const { id } = await spawnAgentPane()
await dispatcher.callRequest('pty.shutdown', { id })
expect(handler.isPaneSurfaceRetired(PANE_KEY)).toBe(true)
const term = { ...mockPtyInstance, kill: vi.fn(), onData: vi.fn(), onExit: vi.fn() }
mockPtySpawn.mockReturnValue(term)
await spawnPty({ env: { ORCA_PANE_KEY: PANE_KEY } })
expect(handler.isPaneSurfaceRetired(PANE_KEY)).toBe(false)
})
it('restores the surface when a client reattaches to a shut-down PTY that survived', async () => {
vi.spyOn(ptyShellUtils, 'isProcessAlive').mockReturnValue(true)
const { id } = await spawnAgentPane()
await dispatcher.callRequest('pty.shutdown', { id })
expect(handler.isPaneSurfaceRetired(PANE_KEY)).toBe(true)
await dispatcher.callRequest('pty.attach', { id })
expect(handler.isPaneSurfaceRetired(PANE_KEY)).toBe(false)
})
it('shutting down a pane with no pane key retires nothing', async () => {
const retired: { id: string; paneKey: string }[] = []
handler.setSurfaceRetiredListener((event) => retired.push(event))
const term = { ...mockPtyInstance, kill: vi.fn(), onData: vi.fn(), onExit: vi.fn() }
mockPtySpawn.mockReturnValue(term)
const spawned = await spawnPty({})
await dispatcher.callRequest('pty.shutdown', { id: spawned.id })
expect(retired).toEqual([])
})
})
+152 -9
View File
@@ -17,6 +17,7 @@ import {
listShellProfiles
} from './pty-shell-utils'
import { getRelayShellLaunchConfig, isRelayWslShell } from './pty-shell-launch'
import { RetiredPaneSurfaceRegistry } from './retired-pane-surfaces'
import { addWslEnvKeys } from '../shared/wsl-env'
import { SHELL_STARTUP_FEATURE_ENV } from '../main/shell-startup-features'
import { DEFAULT_SSH_RELAY_GRACE_PERIOD_SECONDS } from '../shared/ssh-types'
@@ -151,6 +152,9 @@ type ManagedPty = {
buffered: RecentPtyOutputBuffer
/** Timer for SIGKILL fallback after a graceful SIGTERM shutdown. */
killTimer?: ReturnType<typeof setTimeout>
/** Timer for the post-shutdown reap sweep: re-probes liveness after a kill request instead of
* assuming the request landed. */
reapTimer?: ReturnType<typeof setTimeout>
/** True once disposeManagedPty has run; blocks double-dispose and makes post-dispose calls fail "not found" not silently. */
disposed?: boolean
/** True once external cleanup observers have been notified. */
@@ -242,6 +246,10 @@ function disposeManagedPty(managed: ManagedPty): void {
clearTimeout(managed.killTimer)
managed.killTimer = undefined
}
if (managed.reapTimer) {
clearTimeout(managed.reapTimer)
managed.reapTimer = undefined
}
// Why: neutralize pty.kill before destroy() so UnixTerminal's async 'close' SIGHUP can't hit a recycled pid.
// Windows exempt: its destroy() IS a kill() (via _deferNoArgs), so neutralizing leaks the ConPTY agent.
if (process.platform !== 'win32') {
@@ -258,6 +266,9 @@ function disposeManagedPty(managed: ManagedPty): void {
}
const DEFAULT_GRACE_TIME_MS = DEFAULT_SSH_RELAY_GRACE_PERIOD_SECONDS * 1000
export const IMMEDIATE_PTY_EXIT_TIMEOUT_MS = 8_000
/** Longer than the 5s armed SIGKILL fallback, so the first sweep observes the post-kill state. */
export const SHUTDOWN_REAP_VERIFY_DELAY_MS = 6_000
export const SHUTDOWN_REAP_MAX_SWEEPS = 3
export const MAX_RELAY_PTY_SESSIONS = 50
export const REPLAY_BUFFER_MAX = 100 * 1024
const PTY_OUTPUT_BATCH_INTERVAL_MS = 8
@@ -389,6 +400,10 @@ function sanitizeEnvToDelete(value: unknown): string[] {
export type PtyExitListener = (event: { id: string; paneKey?: string }) => void
/** Notified when a client retires a pane's surface (`pty.shutdown`). Deliberately separate from
* {@link PtyExitListener}: retirement says the tab is gone, never that the process exited. */
export type PtySurfaceRetiredListener = (event: { id: string; paneKey: string }) => void
type PtyIdentity = { paneKey?: string; tabId?: string }
/**
@@ -443,6 +458,8 @@ export class PtyHandler {
private reloadPtyModuleFromDisk = false
// Why: single optional slot is intentional — callers compose externally; a throw is swallowed so it can't block cleanup.
private exitListener: PtyExitListener | null = null
private surfaceRetiredListener: PtySurfaceRetiredListener | null = null
private readonly retiredPaneSurfaces = new RetiredPaneSurfaceRegistry()
private ptyPoolEmptyListener: (() => void) | null = null
private ptyPoolActiveListener: (() => void) | null = null
// Why: augment environment on every spawn so PTYs receive current hook coordinates.
@@ -571,6 +588,19 @@ export class PtyHandler {
this.exitListener = listener
}
/** Subscribe to pane-surface retirement (relay-hook server uses this to drop the pane's cached
* agent status the moment the tab goes away, rather than waiting for a process exit that a
* surviving shell may never produce). */
setSurfaceRetiredListener(listener: PtySurfaceRetiredListener | null): void {
this.surfaceRetiredListener = listener
}
/** True when the client has told this host the pane's tab is gone and no PTY has re-bound the
* paneKey since. Nothing this pane emits can belong to a surface any client still owns. */
isPaneSurfaceRetired(paneKey: string): boolean {
return this.retiredPaneSurfaces.isRetired(paneKey)
}
/** Notified when the last PTY leaves the pool, so the relay can re-arm its idle grace. */
onPtyPoolEmpty(listener: () => void): () => void {
this.ptyPoolEmptyListener = listener
@@ -786,6 +816,12 @@ export class PtyHandler {
private wireAndStore(managed: ManagedPty): void {
managed.physicalExit = new PhysicalExitTracker()
this.ptys.set(managed.id, managed)
// Why: a PTY joining the pool under this paneKey means the surface exists again (reopened pane
// or revive), so a prior retirement no longer describes anything and must not mute its hooks.
const boundPaneKey = managed.paneKey ?? managed.attachIdentity?.paneKey
if (boundPaneKey) {
this.retiredPaneSurfaces.restore(boundPaneKey)
}
// Why: a second announce covers any store whose admission window has already closed.
this.notifyPoolListener(this.ptyPoolActiveListener, 'pty-pool-active')
const emitIngressData = (emission: PtyIngressEmission): void => {
@@ -1778,14 +1814,7 @@ export class PtyHandler {
// Why: verify liveness because shells can exit without node-pty onExit.
if (managed.pty.pid && !isProcessAlive(managed.pty.pid)) {
managed.physicalExit?.markExited()
this.releaseRelayIngress(managed)
this.flushPtyOutput(id)
this.notifyExitListener(managed)
this.agentSessionOwners.release(managed.id)
disposeManagedPty(managed)
this.removePty(id)
this.clearPtyFlowState(id)
this.reapExitedPty(managed)
throw new Error(`PTY "${id}" not found`)
}
@@ -1801,6 +1830,14 @@ export class PtyHandler {
throw new Error(`PTY "${id}" not found (identity mismatch)`)
}
// Why: an accepted attach is a client surface for this pane, so a prior retirement no longer
// describes anything — without this a reattach to a shut-down-but-surviving PTY would keep the
// pane's agent hooks muted for the rest of the daemon's life.
const attachedPaneKey = managed.paneKey ?? managed.attachIdentity?.paneKey
if (attachedPaneKey) {
this.retiredPaneSurfaces.restore(attachedPaneKey)
}
managed.startupIngress?.snapshotBarrier()
let sourceRecovery = parseSourceRecoveryRequest(params.sourceRecovery)
if (
@@ -1917,6 +1954,17 @@ export class PtyHandler {
if (!managed) {
return
}
// Why: `pty.shutdown` is the only authoritative statement this host ever gets that a tab is
// gone. Record it before the kill request, because the kill is the part that can fail: an agent
// that survives teardown otherwise keeps posting hooks the relay forwards as a live agent pane
// with no tab, and that advertisement is what drives a second `--resume` onto its transcript.
// Why gated on an actual retirement: the teardown below is fire-and-forget — one SIGTERM, one
// armed SIGKILL, and nobody ever looks again. Re-probing is what stops a retired pane's shell
// outliving its tab; a PTY with no pane surface has nothing to outlive, so its long-standing
// teardown contract is left exactly as it was.
if (this.retirePaneSurface(managed)) {
this.armShutdownReapSweep(managed, SHUTDOWN_REAP_MAX_SWEEPS)
}
if (immediate) {
this.releaseStartupCommand(managed)
@@ -1930,6 +1978,93 @@ export class PtyHandler {
}
}
/** Record that this pane's client surface is gone, and tell the hook server so the pane's cached
* agent status stops being replayed to reconnecting clients. Returns false when there is no pane
* surface to retire. */
private retirePaneSurface(managed: ManagedPty): boolean {
const paneKey = managed.paneKey ?? managed.attachIdentity?.paneKey
if (!paneKey) {
return false
}
this.retiredPaneSurfaces.retire(paneKey)
const listener = this.surfaceRetiredListener
if (!listener) {
return true
}
try {
listener({ id: managed.id, paneKey })
} catch (err) {
process.stderr.write(
`[pty-handler] surface-retired listener threw: ${err instanceof Error ? err.message : String(err)}\n`
)
}
return true
}
/**
* After a retirement, verify the host actually reaped the shell rather than trusting the kill
* request. Two outcomes matter and both are bookkeeping the relay previously never did:
*
* - the pid is gone but node-pty never produced `onExit` — retire the record here, so the entry
* and its agent-session owners stop being published by `pty.listProcesses`;
* - the pid is still alive — re-issue the force kill instead of leaving a detached shell (and the
* agent inside it) outliving its tab for the life of the daemon.
*
* Bounded: after {@link SHUTDOWN_REAP_MAX_SWEEPS} the owner claim is deliberately *retained*.
* Releasing a claim we cannot prove dead is what lets a reopened project spawn a second agent
* over one transcript; keeping it makes the next `pty.spawn` adopt this PTY instead.
*/
private armShutdownReapSweep(managed: ManagedPty, attemptsRemaining: number): void {
if (managed.reapTimer) {
return
}
const timer = setTimeout(() => {
managed.reapTimer = undefined
if (this.ptys.get(managed.id) !== managed || managed.disposed) {
return
}
const pid = managed.pty.pid
if (pid && !isProcessAlive(pid)) {
this.reapExitedPty(managed)
return
}
if (attemptsRemaining <= 0) {
process.stderr.write(
`[pty-handler] retired pane PTY ${managed.id} still alive after force kill; ownership retained as unverifiable\n`
)
return
}
// Why POSIX-only: a SIGKILL that returned success is not proof of death there — the group
// probe can degrade to a root-pid kill, leaving the agent running under a shell nobody is
// watching. On Windows ConPTY's kill is already force-final and closing its handle twice is
// the hazard disposeManagedPty guards against, so the probe above is the whole sweep.
if (process.platform !== 'win32') {
managed.forceKillSent = false
try {
this.requestForceKill(managed)
} catch {
/* Re-probed on the next sweep; a transient failure must not end the escalation. */
}
}
this.armShutdownReapSweep(managed, attemptsRemaining - 1)
}, SHUTDOWN_REAP_VERIFY_DELAY_MS)
timer.unref?.()
managed.reapTimer = timer
}
/** Retire every record for a PTY whose process is proven gone. Shared by the attach probe, the
* listing probe and the post-shutdown sweep so the three cannot drift on what "gone" retires. */
private reapExitedPty(managed: ManagedPty): void {
managed.physicalExit?.markExited()
this.releaseRelayIngress(managed)
this.flushPtyOutput(managed.id)
this.notifyExitListener(managed)
this.agentSessionOwners.release(managed.id)
disposeManagedPty(managed)
this.removePty(managed.id)
this.clearPtyFlowState(managed.id)
}
private async sendSignal(params: Record<string, unknown>): Promise<void> {
const id = params.id as string
const signal = params.signal as string
@@ -2101,7 +2236,15 @@ export class PtyHandler {
private async listProcesses(): Promise<PtyProcessSummary[]> {
const results: PtyProcessSummary[] = []
for (const [id, managed] of this.ptys) {
// Why (SSH-v3 P2 — the host is the authoritative liveness source, so it has to look): this
// listing is what publishes `agentSessionOwners`, i.e. "there is a live agent session here you
// can adopt". A shell can exit without node-pty's onExit, and an unverified entry advertised
// that session forever. Snapshot the map because reaping mutates it.
for (const [id, managed] of Array.from(this.ptys)) {
if (managed.disposed || (managed.pty.pid && !isProcessAlive(managed.pty.pid))) {
this.reapExitedPty(managed)
continue
}
const title =
(await getForegroundProcessName(managed.pty.pid, managed.pty.process || null)) || 'shell'
results.push({
+10 -1
View File
@@ -30,7 +30,10 @@ export class RelayAgentHookRuntime {
) {
this.hookServer = new RelayAgentHookServer({
endpointDir: endpointDir ?? endpointDirForRelaySocket(sockPath),
forward: (envelope) => publishAgentHookEnvelope(dispatcher, envelope)
forward: (envelope) => publishAgentHookEnvelope(dispatcher, envelope),
// Why: the PTY handler is the only component that knows which panes still have a client
// surface, so it — not the client — decides whether a hook post describes a live pane.
isPaneSurfaceRetired: (paneKey) => ptyHandler.isPaneSurfaceRetired(paneKey)
})
}
@@ -63,6 +66,12 @@ export class RelayAgentHookRuntime {
}
this.pluginOverlay.clearOverlay(paneKey ?? id)
})
// Why: the exit listener above only fires on proof of process death, which a shell that
// survives teardown never produces. Drop the pane's cached status the moment its tab goes, so a
// reconnecting client cannot be handed a replay of an agent nobody owns.
this.ptyHandler.setSurfaceRetiredListener(({ paneKey }) => {
this.hookServer.clearPaneState(paneKey)
})
}
private buildPluginEnvironment(context: Parameters<PtyEnvAugmenter>[0]): Record<string, string> {
+51
View File
@@ -0,0 +1,51 @@
import { describe, expect, it } from 'vitest'
import { RETIRED_PANE_SURFACE_LIMIT, RetiredPaneSurfaceRegistry } from './retired-pane-surfaces'
describe('RetiredPaneSurfaceRegistry', () => {
it('records a retired pane and forgets it once the surface exists again', () => {
const registry = new RetiredPaneSurfaceRegistry()
expect(registry.isRetired('pane-a')).toBe(false)
registry.retire('pane-a')
expect(registry.isRetired('pane-a')).toBe(true)
registry.restore('pane-a')
expect(registry.isRetired('pane-a')).toBe(false)
})
it('ignores empty pane keys rather than retiring a catch-all entry', () => {
const registry = new RetiredPaneSurfaceRegistry()
registry.retire('')
expect(registry.size).toBe(0)
expect(registry.isRetired('')).toBe(false)
})
it('caps growth by evicting the longest-retired pane', () => {
const registry = new RetiredPaneSurfaceRegistry()
for (let index = 0; index < RETIRED_PANE_SURFACE_LIMIT + 25; index += 1) {
registry.retire(`pane-${index}`)
}
expect(registry.size).toBe(RETIRED_PANE_SURFACE_LIMIT)
expect(registry.isRetired('pane-0')).toBe(false)
expect(registry.isRetired('pane-24')).toBe(false)
expect(registry.isRetired('pane-25')).toBe(true)
expect(registry.isRetired(`pane-${RETIRED_PANE_SURFACE_LIMIT + 24}`)).toBe(true)
})
it('re-retiring refreshes recency so an active pane is not evicted first', () => {
const registry = new RetiredPaneSurfaceRegistry()
registry.retire('pane-oldest')
for (let index = 0; index < RETIRED_PANE_SURFACE_LIMIT - 1; index += 1) {
registry.retire(`pane-${index}`)
}
registry.retire('pane-oldest')
registry.retire('pane-overflow')
expect(registry.isRetired('pane-oldest')).toBe(true)
expect(registry.isRetired('pane-0')).toBe(false)
})
})
+55
View File
@@ -0,0 +1,55 @@
/**
* PaneKeys whose client surface the host has been told is gone — a `pty.shutdown` arrived for the
* PTY bound to them — but whose host-side process the relay has not proven dead.
*
* Retirement is deliberately NOT a liveness verdict. Per docs/reference/ssh-execution-boundary.md
* the vocabulary stays `live` / `unverifiable` / `exited`, and this registry asserts none of them
* about the process: it records only that no client tab can own this pane any more. That is the
* fact the relay needs to tell an orphaned agent's hook post apart from a live agent pane, and it
* is knowable without guessing, because the client stated it.
*/
/** Bounded so a long-lived relay cannot accumulate one entry per pane ever closed. Insertion order
* is age, so eviction drops the longest-retired pane first; an evicted pane simply degrades to the
* pre-fix behaviour (its posts are forwarded again) rather than failing. */
export const RETIRED_PANE_SURFACE_LIMIT = 512
export class RetiredPaneSurfaceRegistry {
private readonly retired = new Set<string>()
retire(paneKey: string): void {
if (!paneKey) {
return
}
// Delete-then-add makes iteration order = recency of retirement for the cap below.
this.retired.delete(paneKey)
this.retired.add(paneKey)
while (this.retired.size > RETIRED_PANE_SURFACE_LIMIT) {
const oldest = this.retired.values().next().value
if (oldest === undefined) {
break
}
this.retired.delete(oldest)
}
}
/** A PTY joining the pool under this paneKey means the surface exists again — the user reopened
* the pane, or a revive re-bound it — so the retirement no longer describes anything. */
restore(paneKey: string): void {
if (paneKey) {
this.retired.delete(paneKey)
}
}
isRetired(paneKey: string): boolean {
return Boolean(paneKey) && this.retired.has(paneKey)
}
get size(): number {
return this.retired.size
}
clear(): void {
this.retired.clear()
}
}
@@ -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 }
}
@@ -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
@@ -2,11 +2,19 @@ import type { IpcPtyTransportOptions, PtyConnectResult, PtyTransport } from './p
type PtyConnectOptions = Parameters<PtyTransport['connect']>[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<PtyConnectResult & { isReattach?: boolean }> {
): Promise<IpcPtySpawnResponse> {
const {
cwd,
cwdFallback,
@@ -70,5 +78,5 @@ export async function spawnIpcPty(
...(projectRuntime ? { projectRuntime } : {}),
...(terminalColorQueryReplies ? { terminalColorQueryReplies } : {}),
...(telemetry ? { telemetry } : {})
}) as Promise<PtyConnectResult & { isReattach?: boolean }>
}) as Promise<IpcPtySpawnResponse>
}
@@ -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)
}
@@ -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
@@ -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)
@@ -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<string, BufferedPreHandlerPtyState>()
const preHandlerPtyExit = new Map<string, BufferedPreHandlerPtyExit>()
// 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<string, BufferedPreHandlerPtyExit[]>()
const consumedPreHandlerPtyExits = new Map<string, true>()
const discardedPreHandlerPtyStates = new Map<string, ReturnType<typeof setTimeout>>()
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<V>(map: Map<string, V>, 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
}
@@ -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<typeof vi.fn> }> {
const { createIpcPtyTransport } = await import('./pty-transport')
const spawn = window.api.pty.spawn as unknown as ReturnType<typeof vi.fn>
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)
})
})
@@ -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 = {
@@ -7,6 +7,7 @@ import {
TERMINAL_TAB_PARK_FLIP_WINDOW_MS,
getParkVerdictUnparkPinUntilMs,
recordParkVerdictFlips,
selectParkVerdictPinnedTabIds,
type ParkVerdictFlipRecord
} from './terminal-park-verdict-flip-telemetry'
@@ -336,4 +337,28 @@ describe('expired pins stop gating breadcrumbs', () => {
expect.objectContaining({ trigger: 'window' })
)
})
// Why: the pin is set from flips on the rendered verdict, so it has to be
// readable — and expirable — for any live tab, not only cold-park candidates
// (issue #15136: the driver was the worktree-level park prop).
it('selects and expires pins for tabs the cold-park selector never proposed', () => {
const records = new Map<string, ParkVerdictFlipRecord>()
for (let i = 0; i < 40; i += 1) {
observe({ records, parked: i % 2 === 0, nowMs: 1_000 + i * 10 })
}
const pinned = selectParkVerdictPinnedTabIds({ records, tabIds: [TAB], nowMs: 1_500 })
expect(pinned.pinnedTabIds).toEqual(new Set([TAB]))
expect(pinned.earliestPinExpiryMs).toBe(records.get(TAB)?.pinnedUntilMs)
// Past the deadline the pin lapses in place, so damping never latches on.
const lapsed = selectParkVerdictPinnedTabIds({
records,
tabIds: [TAB],
nowMs: 1_000 + TERMINAL_TAB_PARK_FLIP_WINDOW_MS * 3
})
expect(lapsed.pinnedTabIds.size).toBe(0)
expect(lapsed.earliestPinExpiryMs).toBeNull()
expect(records.get(TAB)?.pinnedUntilMs).toBeNull()
})
})
@@ -3,11 +3,12 @@
* Field breadcrumbs prove render-cadence flips, but not which eligibility input
* oscillates; burst damping keeps the pane mounted before React reaches #185.
*
* Scope: flips are counted on the rendered verdict, but the pin can only
* subtract from the cold-park candidate set. Churn driven by the other parked
* inputs (forced parking, portal ownership, deferred activation mounts) is
* observed and breadcrumbed, not damped — read a repeating `burst` crumb for
* one tab as "damping did not reach the oscillating input".
* Scope: flips are counted on the rendered verdict and the pin subtracts from
* that same verdict (selectParkVerdictPinnedTabIds), so churn driven by any
* parked input — worktree-level park, portal ownership, deferred activation
* mounts — is damped, not only the cold-park candidate set. A repeating `burst`
* crumb for one tab therefore means the driver keeps re-proposing the park once
* each pin lapses, not that damping never reached it.
*/
import { recordRendererCrashBreadcrumb } from '@/lib/crash-breadcrumb-recorder'
import { REACT_NESTED_UPDATE_LIMIT } from '../../../../shared/react-update-depth-attribution'
@@ -169,3 +170,41 @@ export function recordParkVerdictFlips(args: {
}
}
}
export type ParkVerdictPinSelection = {
pinnedTabIds: Set<string>
/** Earliest live pin deadline, so a caller can wake exactly when damping lapses. */
earliestPinExpiryMs: number | null
}
/**
* Tab ids a flip burst pinned unparked, expiring lapsed pins in place.
*
* Why every live tab and not only cold-park candidates: flips are counted on
* the rendered verdict, so the oscillating input can be one the cold-park
* selector never sees (worktree-level park, activation-deferred mounts). A pin
* consulted only through the cold set then damps nothing and never lapses —
* it just silences its own breadcrumb for the window and re-arms forever.
*/
export function selectParkVerdictPinnedTabIds(args: {
records: Map<string, ParkVerdictFlipRecord>
tabIds: Iterable<string>
nowMs: number
}): ParkVerdictPinSelection {
const pinnedTabIds = new Set<string>()
let earliestPinExpiryMs: number | null = null
for (const tabId of args.tabIds) {
const pinnedUntilMs = getParkVerdictUnparkPinUntilMs({
records: args.records,
tabId,
nowMs: args.nowMs
})
if (pinnedUntilMs === null) {
continue
}
pinnedTabIds.add(tabId)
earliestPinExpiryMs =
earliestPinExpiryMs === null ? pinnedUntilMs : Math.min(earliestPinExpiryMs, pinnedUntilMs)
}
return { pinnedTabIds, earliestPinExpiryMs }
}
@@ -0,0 +1,197 @@
/** @vitest-environment happy-dom */
/**
* Park-verdict oscillation driven by an input the cold-park selector never sees.
*
* Why this shape (issue #15136): Terminal.tsx vetoes a WORKTREE park with
* worktreeTabsAreWatcherCovered → canWatcherCoverParkedTerminalTab, which a
* mounted pane grants and a parked pane withdraws. That verdict arrives here as
* the coldParkTerminalPanes prop, which short-circuits the cold-park branch of
* the rendered verdict — so withholdUnparkableTerminalTabs, which only filters
* the cold-park candidate list, can never damp it.
*
* The single tab is deliberate: selectIdsBeyondHotRetain spares the lone
* most-recently-hidden candidate, so the cold-park set stays empty and
* coldParkTerminalPanes is provably the only driver of the rendered verdict.
*
* Field signature this reproduces: one tab, `trigger: burst` crumbs recurring
* 60 008 / 60 020 / 60 012 ms apart (the pin window), i.e. flips continuing at
* commit cadence underneath a live pin instead of stopping.
*/
import { act, useEffect, useState } from 'react'
import { createRoot, type Root } from 'react-dom/client'
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
import type { TerminalTab } from '../../../../shared/terminal-tab-types'
;(globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true
const park = vi.hoisted(() => ({ worktreeId: 'repo::/wt-worktree-driver' }))
vi.mock('../../store', async () => {
const { create } = await import('zustand')
const useAppStore = create(() => ({
pendingStartupByTabId: {} as Record<string, unknown>,
ptyIdsByTabId: {} as Record<string, string[]>,
runtimeStatusByEnvironmentId: new Map<string, unknown>(),
settings: {} as Record<string, unknown>,
terminalLayoutsByTabId: {} as Record<string, unknown>,
runtimePaneTitlesByTabId: {} as Record<string, unknown>,
tabsByWorktree: {} as Record<string, TerminalTab[]>
}))
return { useAppStore }
})
vi.mock('./terminal-parked-tab-watchers', () => ({
canWatcherCoverParkedTerminalTab: () => true,
disposeParkedTerminalWatchersForWorktree: () => {},
resolveParkedTerminalPaneCandidates: () => [],
syncParkedTerminalTabWatchers: () => {}
}))
vi.mock('./terminal-parking-e2e-overrides', () => ({
getTerminalParkingPolicyOverrides: () => ({ coldParkDelayMs: 0, hotRetainMs: 0 })
}))
import { useAppStore } from '../../store'
import { TERMINAL_TAB_PARK_FLIP_BURST_LIMIT } from './terminal-park-verdict-flip-telemetry'
import { useTerminalTabColdParking } from './use-terminal-tab-cold-parking'
const TAB_ID = 'tab-worktree-driven'
/** Stops an unfixed loop from hanging the run if React ever raises its bail. */
const RENDER_HARD_STOP = 400
/** +1: the pin lands in the passive effect observing the burst flip. */
const SETTLED_FLIP_BUDGET = TERMINAL_TAB_PARK_FLIP_BURST_LIMIT + 1
const EMPTY_ASSIGNMENTS = new Map<string, { groupId: string; isActiveInGroup: boolean }>()
const EMPTY_PORTALS: never[] = []
type ParkingStoreState = { tabsByWorktree: Record<string, TerminalTab[]> }
const parkingStore = useAppStore as unknown as {
setState: (partial: (state: ParkingStoreState) => Partial<ParkingStoreState>) => void
}
let hostRenderCount = 0
let parkVerdictFlipCount = 0
let lastParkVerdict = false
let paneMountCount = 0
/** The pane whose mount/unmount the park verdict authorizes. */
function TerminalPaneStandIn(): null {
useEffect(() => {
paneMountCount += 1
}, [])
return null
}
/**
* Terminal.tsx's worktree-level verdict, modelled: a mounted pane grants the
* byte-watcher coverage the worktree park requires, and a parked pane withdraws
* it. Runs in a passive effect, so each turn of the loop costs one commit.
*/
function WorktreeParkVerdict({
parked,
onVerdict
}: {
parked: boolean
onVerdict: (coldParkTerminalPanes: boolean) => void
}): null {
useEffect(() => {
onVerdict(!parked)
}, [parked, onVerdict])
return null
}
function OverlayHost(): React.JSX.Element | null {
hostRenderCount += 1
const [coldParkTerminalPanes, setColdParkTerminalPanes] = useState(false)
const terminalTabs = useAppStore(
(state) => (state as ParkingStoreState).tabsByWorktree[park.worktreeId]
) as TerminalTab[]
const parkedTerminalTabIds = useTerminalTabColdParking({
worktreeId: park.worktreeId,
terminalTabs,
assignments: EMPTY_ASSIGNMENTS,
isWorktreeActive: false,
activeTerminalTabId: null,
coldParkTerminalPanes,
shouldMeasureHiddenWorktree: false,
activityTerminalPortals: EMPTY_PORTALS,
activationDeferredMountTabIds: null
})
const parked = parkedTerminalTabIds.has(TAB_ID)
if (parked !== lastParkVerdict) {
parkVerdictFlipCount += 1
}
lastParkVerdict = parked
if (hostRenderCount > RENDER_HARD_STOP) {
return null
}
return (
<>
{parked ? null : <TerminalPaneStandIn />}
<WorktreeParkVerdict parked={parked} onVerdict={setColdParkTerminalPanes} />
</>
)
}
function renderOverlayHost(root: Root): unknown {
const consoleError = vi.spyOn(console, 'error').mockImplementation(() => {})
let thrown: unknown = null
try {
act(() => {
root.render(<OverlayHost />)
})
} catch (error) {
thrown = error
}
consoleError.mockRestore()
return thrown
}
describe('park-verdict damping reaches the worktree-level driver', () => {
let container: HTMLDivElement
let root: Root | undefined
beforeEach(() => {
hostRenderCount = 0
parkVerdictFlipCount = 0
lastParkVerdict = false
paneMountCount = 0
parkingStore.setState(() => ({
tabsByWorktree: {
[park.worktreeId]: [
{ id: TAB_ID, ptyId: `${park.worktreeId}@@session-1`, title: TAB_ID } as TerminalTab
]
}
}))
container = document.createElement('div')
document.body.appendChild(container)
})
afterEach(() => {
act(() => {
root?.unmount()
})
root = undefined
container.remove()
})
it('settles a verdict whose only driver is the worktree-level park prop', () => {
root = createRoot(container)
const thrown = renderOverlayHost(root)
expect(thrown).toBeNull()
// Without damping on the rendered verdict this runs to RENDER_HARD_STOP:
// the burst pin only subtracts from the cold-park candidate set, which is
// empty here, so it silences its breadcrumb and damps nothing.
expect(parkVerdictFlipCount).toBeLessThanOrEqual(SETTLED_FLIP_BUDGET)
// Settles on the safe side: the pane stays mounted, so bells/titles/
// completions never go silent behind a wedged verdict.
expect(lastParkVerdict).toBe(false)
// Measured: 4 flips / 3 mounts / 6 renders with damping, against
// 400 flips / 200 mounts before it — the pre-fix run only stops because
// RENDER_HARD_STOP stops it.
expect(paneMountCount).toBeGreaterThan(0)
expect(paneMountCount).toBeLessThanOrEqual(SETTLED_FLIP_BUDGET)
expect(hostRenderCount).toBeLessThan(RENDER_HARD_STOP)
})
})
@@ -0,0 +1,131 @@
/**
* Applies verdict-flip damping to the *rendered* park verdict.
*
* Why here and not inside the cold-park selector: every park input feeds one
* rendered set, and several of them (worktree-level park, activation-deferred
* mounts) bypass the cold-park candidate list entirely. Damping only that list
* left those drivers free to remount a pane at commit cadence while the burst
* pin silenced its own breadcrumb for the window — issue #15136, where one tab
* re-burst 60 008 / 60 020 / 60 012 ms apart, i.e. the instant each pin lapsed.
*
* The pin lags the verdict by one commit (it is set from the passive effect
* that observes the flip), which is what lets the loop settle instead of
* feeding itself: pinned tabs render unparked, so the next observation records
* no flip.
*/
import { useEffect, useMemo, useRef, useState } from 'react'
import type { TerminalTab } from '../../../../shared/terminal-tab-types'
import {
recordParkVerdictFlips,
selectParkVerdictPinnedTabIds,
type ParkVerdictFlipRecord
} from './terminal-park-verdict-flip-telemetry'
const EMPTY_TAB_IDS: ReadonlySet<string> = new Set()
export function haveSameTerminalTabIds(
left: ReadonlySet<string>,
right: ReadonlySet<string>
): boolean {
if (left.size !== right.size) {
return false
}
for (const id of left) {
if (!right.has(id)) {
return false
}
}
return true
}
/**
* Returns the park verdict with flip-pinned tabs removed, and records churn on
* that same (rendered) verdict.
*/
export function useTerminalParkVerdictPin(args: {
records: React.RefObject<Map<string, ParkVerdictFlipRecord>>
terminalTabs: readonly TerminalTab[]
/** Park verdict before damping — every input already merged. */
candidateParkedTabIds: ReadonlySet<string>
}): ReadonlySet<string> {
const { records, terminalTabs, candidateParkedTabIds } = args
const [pinnedTabIds, setPinnedTabIds] = useState<ReadonlySet<string>>(EMPTY_TAB_IDS)
const pinnedTabIdsRef = useRef(pinnedTabIds)
// Why a revision and not a raw timer callback: the pin lapse must re-run the
// observation effect, which is also what expires the record in place.
const [pinExpiryRevision, setPinExpiryRevision] = useState(0)
// Why armed-deadline bookkeeping: this effect re-runs on every tab-model
// write (runtime titles, unread bumps), which on a busy agent pane is many
// times a second. Re-arming an unchanged absolute deadline each time is pure
// churn — keep the live timer instead.
const armedPinExpiryRef = useRef<{ deadlineMs: number; timer: number } | null>(null)
const parkedTabIds = useMemo(() => {
if (pinnedTabIds.size === 0) {
return candidateParkedTabIds
}
const parked = new Set<string>()
for (const tabId of candidateParkedTabIds) {
if (!pinnedTabIds.has(tabId)) {
parked.add(tabId)
}
}
return parked
}, [candidateParkedTabIds, pinnedTabIds])
useEffect(() => {
const flipRecords = records.current
const liveTabIds = new Set(terminalTabs.map((terminalTab) => terminalTab.id))
const nowMs = Date.now()
recordParkVerdictFlips({
records: flipRecords,
liveTabIds,
nextParkedTabIds: parkedTabIds,
nowMs
})
const { pinnedTabIds: nextPinnedTabIds, earliestPinExpiryMs } = selectParkVerdictPinnedTabIds({
records: flipRecords,
tabIds: liveTabIds,
nowMs
})
if (!haveSameTerminalTabIds(pinnedTabIdsRef.current, nextPinnedTabIds)) {
pinnedTabIdsRef.current = nextPinnedTabIds
setPinnedTabIds(nextPinnedTabIds)
}
const armed = armedPinExpiryRef.current
if (earliestPinExpiryMs === null) {
if (armed) {
window.clearTimeout(armed.timer)
armedPinExpiryRef.current = null
}
return
}
if (armed?.deadlineMs === earliestPinExpiryMs) {
return
}
if (armed) {
window.clearTimeout(armed.timer)
}
armedPinExpiryRef.current = {
deadlineMs: earliestPinExpiryMs,
timer: window.setTimeout(
() => setPinExpiryRevision((revision) => revision + 1),
Math.max(1, earliestPinExpiryMs - nowMs)
)
}
}, [parkedTabIds, pinExpiryRevision, records, terminalTabs])
// Why separate from the observation effect: that effect intentionally keeps a
// live timer across its own re-runs, so its cleanup cannot own disposal.
useEffect(() => {
const armedPinExpiry = armedPinExpiryRef
return () => {
if (armedPinExpiry.current) {
window.clearTimeout(armedPinExpiry.current.timer)
armedPinExpiry.current = null
}
}
}, [])
return parkedTabIds
}
@@ -75,6 +75,7 @@ import {
} from './terminal-hidden-view-parking'
import {
TERMINAL_TAB_PARK_FLIP_BURST_LIMIT,
TERMINAL_TAB_PARK_FLIP_BURST_WINDOW_MS,
TERMINAL_TAB_PARK_FLIP_WINDOW_MS
} from './terminal-park-verdict-flip-telemetry'
import { useTerminalTabColdParking } from './use-terminal-tab-cold-parking'
@@ -459,6 +460,10 @@ describe('useTerminalTabColdParking measure-clock contract', () => {
}
}
act(() => {
// Why the clock advance: the verdict-flip burst limit is expressed per
// second, and fake timers freeze Date.now(), so back-to-back rerenders
// would read as an oscillation the damping is supposed to pin.
vi.advanceTimersByTime(TERMINAL_TAB_PARK_FLIP_BURST_WINDOW_MS)
rerender(hookArgs(false))
})
expect(result.current.size).toBe(0)
@@ -472,6 +477,7 @@ describe('useTerminalTabColdParking measure-clock contract', () => {
}
}
act(() => {
vi.advanceTimersByTime(TERMINAL_TAB_PARK_FLIP_BURST_WINDOW_MS)
rerender(hookArgs(false))
})
expect(result.current).toEqual(new Set(['tab-2']))
@@ -20,10 +20,11 @@ import {
selectPairedRuntimeParkingEnvironmentIdsFromState,
selectColdParkedTerminalTabs
} from './terminal-hidden-view-parking'
import type { ParkVerdictFlipRecord } from './terminal-park-verdict-flip-telemetry'
import {
recordParkVerdictFlips,
type ParkVerdictFlipRecord
} from './terminal-park-verdict-flip-telemetry'
haveSameTerminalTabIds,
useTerminalParkVerdictPin
} from './use-terminal-park-verdict-pin'
import { withholdUnparkableTerminalTabs } from './terminal-cold-park-withheld-tabs'
import { getTerminalParkingPolicyOverrides } from './terminal-parking-e2e-overrides'
import {
@@ -48,18 +49,6 @@ type TerminalOverlayTabAssignment = {
const EMPTY_TAB_IDS: ReadonlySet<string> = new Set()
function haveSameTerminalTabIds(left: ReadonlySet<string>, right: ReadonlySet<string>): boolean {
if (left.size !== right.size) {
return false
}
for (const id of left) {
if (!right.has(id)) {
return false
}
}
return true
}
export function useTerminalTabColdParking(args: {
worktreeId: string
terminalTabs: readonly TerminalTab[]
@@ -296,11 +285,11 @@ export function useTerminalTabColdParking(args: {
[evictionExemptLayoutKey, isForceParked, terminalTabs, worktreeId]
)
// Why: the rendered park verdict — worktree-level park (prop from
// Why: the park verdict before damping — worktree-level park (prop from
// Terminal.tsx) or per-tab cold park, never portal-hosted tabs. Render and
// the watcher-sync effect must share this exact set so watcher lifecycle
// tracks the committed unmounts.
const parkedTerminalTabIds = useMemo(() => {
// the watcher-sync effect must share the pinned result below so watcher
// lifecycle tracks the committed unmounts.
const candidateParkedTerminalTabIds = useMemo(() => {
const parked = new Set<string>()
for (const terminalTab of terminalTabs) {
const assignment = assignments.get(terminalTab.id)
@@ -357,19 +346,15 @@ export function useTerminalTabColdParking(args: {
worktreeId
])
// Why: observation only — records whether the *rendered* park verdict churns,
// so a crash bundle can confirm or refute a park-flip update loop. Watching
// the pre-gate cold set instead would miss loops driven by coldParkTerminalPanes
// or the portal/measuring gates. Changes no verdict; see
// terminal-park-verdict-flip-telemetry.ts.
useEffect(() => {
recordParkVerdictFlips({
records: parkVerdictRecordsRef.current,
liveTabIds: new Set(terminalTabs.map((terminalTab) => terminalTab.id)),
nextParkedTabIds: parkedTerminalTabIds,
nowMs: Date.now()
})
}, [parkedTerminalTabIds, terminalTabs])
// Why the last gate: flips are counted on the *rendered* verdict, so damping
// has to subtract from that same set — coldParkTerminalPanes and the
// activation-deferred branch never pass through the cold-park candidate list
// withholdUnparkableTerminalTabs filters (issue #15136).
const parkedTerminalTabIds = useTerminalParkVerdictPin({
records: parkVerdictRecordsRef,
terminalTabs,
candidateParkedTabIds: candidateParkedTerminalTabIds
})
// Why: runs in the same effect flush as the commit that parked/revealed the
// panes — watcher disposal therefore lands before any PTY data IPC can
@@ -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(
+84
View File
@@ -0,0 +1,84 @@
import { describe, expect, it } from 'vitest'
import {
decideSshPendingPtyKill,
MAX_SSH_PENDING_PTY_KILLS_PER_TARGET,
prunePendingSshPtyKills,
SSH_PENDING_PTY_KILL_TTL_MS,
type SshPendingPtyKill,
type SshPendingPtyKillEntry
} from './ssh-pending-pty-kill'
const NOW = 1_800_000_000_000
function intent(overrides: Partial<SshPendingPtyKill> = {}): SshPendingPtyKill {
return { requestedAt: NOW, incarnationId: 'inc-a', attempts: 0, ...overrides }
}
describe('decideSshPendingPtyKill', () => {
it('replays only when the host still holds the exact incarnation the kill was aimed at', () => {
expect(
decideSshPendingPtyKill(intent(), { hostListsPty: true, hostIncarnationId: 'inc-a' }, NOW)
).toEqual({ action: 'replay' })
})
// The #16970 collision: a redeployed relay renumbers from pty-1, so the same id can name a
// different shell. Replaying here would kill a terminal nobody asked to close.
it('retires without killing when the relay id was recycled onto another PTY', () => {
expect(
decideSshPendingPtyKill(intent(), { hostListsPty: true, hostIncarnationId: 'inc-b' }, NOW)
).toEqual({ action: 'retire', reason: 'relay-id-recycled' })
})
it('retires when the owning host answers and does not list the PTY', () => {
expect(
decideSshPendingPtyKill(
intent(),
{ hostListsPty: false, hostIncarnationId: undefined },
NOW + 1000
)
).toEqual({ action: 'retire', reason: 'host-reports-absent' })
})
// TTL retirement belongs to the durable prune, not to a branch here — a branch would be
// unreachable behind it and would only look tested. This guards the belt-and-braces refusal:
// an expired order that somehow reaches this function is never dispatched.
it('refuses to replay past the TTL, ahead of every other branch', () => {
const stale = intent()
const later = NOW + SSH_PENDING_PTY_KILL_TTL_MS + 1
expect(
decideSshPendingPtyKill(stale, { hostListsPty: true, hostIncarnationId: 'inc-a' }, later)
.action
).toBe('defer')
expect(
decideSshPendingPtyKill(stale, { hostListsPty: true, hostIncarnationId: 'inc-a' }, NOW + 1)
).toEqual({ action: 'replay' })
})
// Wire skew: a host predating the published PTY incarnation leaves the fence unanswerable.
// Absence must read as unknown, never as a match and never as a recycle.
it('defers rather than guessing when the host published no incarnation', () => {
expect(
decideSshPendingPtyKill(intent(), { hostListsPty: true, hostIncarnationId: undefined }, NOW)
.action
).toBe('defer')
})
})
describe('prunePendingSshPtyKills', () => {
it('drops expired entries and caps the rest newest-first', () => {
const entries: SshPendingPtyKillEntry[] = [
{
ptyId: 'pty-stale',
intent: intent({ requestedAt: NOW - SSH_PENDING_PTY_KILL_TTL_MS - 1 })
},
...Array.from({ length: MAX_SSH_PENDING_PTY_KILLS_PER_TARGET + 25 }, (_, index) => ({
ptyId: `pty-${index}`,
intent: intent({ requestedAt: NOW - index })
}))
]
const pruned = prunePendingSshPtyKills(entries, NOW)
expect(pruned).toHaveLength(MAX_SSH_PENDING_PTY_KILLS_PER_TARGET)
expect(pruned.map((entry) => entry.ptyId)).not.toContain('pty-stale')
expect(pruned[0]?.ptyId).toBe('pty-0')
})
})
+148
View File
@@ -0,0 +1,148 @@
import type { SshRemotePtyLease } from './ssh-types'
/** A `pty.shutdown` this client issued to an SSH host and could not confirm.
*
* Why it must exist: a relay PTY is a child of the detached relay daemon, not of the ssh channel,
* so a shutdown that dies on the transport leaves a live shell — and often a live agent — running
* on the user's remote machine with nothing left to retry it. Marking the attempt `unverifiable`
* is correct but is not a fix; the kill is an intent that has to outlive the transport failure and
* be replayed against the authoritative host when contact returns.
*
* Carried on the existing `SshRemotePtyLease` rather than in a second journal: the lease is
* already the durable, restart-surviving, per-`(targetId, relayPtyId)` record of a remote PTY. */
export type SshPendingPtyKill = {
requestedAt: number
/** The host-minted PTY incarnation this kill was aimed at, and the whole fence.
*
* A relay renumbers from `pty-1` on every start, so `(targetId, relayPtyId)` alone can name a
* DIFFERENT shell after a redeploy — the collision behind #16970. `pty.shutdown` carries no
* identity parameter and kills whatever holds the id, so the client has to prove identity before
* it replays. The relay mints this per PTY process and publishes it on `pty.listProcesses`,
* which makes it the one value that tells the two apart. */
incarnationId: string
/** Replays attempted since. Diagnostic; the TTL, not this, is the bound. */
attempts: number
}
/** Colocated with the type so the two cannot drift. The lease loader is a strict whitelist that
* drops anything it does not name, so a record omitted here would be silently stripped on every
* launch — the exact failure `closed-terminal-tab-tombstones.ts` records having shipped once. */
export function normalizeSshPendingPtyKill(value: unknown): SshPendingPtyKill | null {
if (!value || typeof value !== 'object') {
return null
}
const raw = value as Partial<SshPendingPtyKill>
if (typeof raw.requestedAt !== 'number' || !Number.isFinite(raw.requestedAt)) {
return null
}
// No incarnation, no fence, and an unfenced kill order is worse than none.
if (
typeof raw.incarnationId !== 'string' ||
!raw.incarnationId ||
raw.incarnationId.length > 128
) {
return null
}
return {
requestedAt: raw.requestedAt,
incarnationId: raw.incarnationId,
attempts: typeof raw.attempts === 'number' && raw.attempts >= 0 ? raw.attempts : 0
}
}
/** Backstop only — host acknowledgement is the normal exit. This covers a target the user never
* reconnects to, whose intent would otherwise be carried forever. Deliberately longer than the 7d
* ceiling on a relay grace window, so the intent outlives the longest window in which the host
* could still be holding the PTY it names. */
export const SSH_PENDING_PTY_KILL_TTL_MS = 30 * 24 * 60 * 60 * 1000
/** Per target, newest kept. A permanently unreachable host must not grow the store without bound. */
export const MAX_SSH_PENDING_PTY_KILLS_PER_TARGET = 200
/** The single definition of "too old to act on". Both the durable prune and the decision function
* read it, so a stale order cannot be replayed by one and kept by the other. */
export function isSshPendingPtyKillExpired(intent: SshPendingPtyKill, now: number): boolean {
return now - intent.requestedAt > SSH_PENDING_PTY_KILL_TTL_MS
}
/** What the authoritative host just said about this relay PTY id, from one `pty.listProcesses`. */
export type SshPendingPtyKillObservation = {
/** False only when the host answered and did not list the id: positive evidence of absence.
* A failed or timed-out listing is never expressed here — the caller defers instead. */
hostListsPty: boolean
/** The incarnation the host published for that id. `undefined` means the host published none,
* which reads as unknown — never as "no incarnation" and never as a match. */
hostIncarnationId: string | undefined
}
export type SshPendingPtyKillRetirement =
| 'host-reports-absent'
| 'relay-id-recycled'
| 'stop-confirmed'
export type SshPendingPtyKillDecision =
| { action: 'replay' }
| { action: 'retire'; reason: Exclude<SshPendingPtyKillRetirement, 'stop-confirmed'> }
| { action: 'defer'; reason: string }
/** Decides what to do with one recorded kill, given what the host just said about its id.
*
* The TTL is deliberately NOT a branch here. `isSshPendingPtyKillExpired` owns it, applied as a
* durable prune before any of this runs, so expired orders are actually deleted from the store
* rather than merely skipped — and so there is exactly one place that decides what "too old"
* means. A branch here would be unreachable behind that prune and would only look tested.
*
* `defer` is the `unverifiable` branch and asserts nothing: the record stays and the next
* handshake asks again. No branch concludes that a PTY exited — only `host-reports-absent` is an
* observation of absence, and it comes from the host that owns the process. */
export function decideSshPendingPtyKill(
intent: SshPendingPtyKill,
observation: SshPendingPtyKillObservation,
now: number
): SshPendingPtyKillDecision {
if (isSshPendingPtyKillExpired(intent, now)) {
// Unreachable behind the prune, but a stale order must never be aimed at whatever holds the
// id today if a future caller reaches this without pruning first.
return { action: 'defer', reason: 'order is past its TTL and awaiting prune' }
}
if (!observation.hostListsPty) {
return { action: 'retire', reason: 'host-reports-absent' }
}
if (observation.hostIncarnationId === undefined) {
// Why not replay: an unfenced kill against a renumbered relay id destroys a shell nobody asked
// to close, which is strictly worse than the leak. Hosts predating the published incarnation
// therefore degrade to no replay rather than to a guess.
return { action: 'defer', reason: 'host published no PTY incarnation for this id' }
}
if (observation.hostIncarnationId !== intent.incarnationId) {
return { action: 'retire', reason: 'relay-id-recycled' }
}
return { action: 'replay' }
}
export type SshPendingPtyKillEntry = { ptyId: string; intent: SshPendingPtyKill }
/** Newest-first, TTL-filtered and capped — the same shape as `pruneClosedTerminalTabTombstones`. */
export function prunePendingSshPtyKills(
entries: readonly SshPendingPtyKillEntry[],
now: number
): SshPendingPtyKillEntry[] {
return entries
.filter((entry) => !isSshPendingPtyKillExpired(entry.intent, now))
.sort((a, b) => b.intent.requestedAt - a.intent.requestedAt)
.slice(0, MAX_SSH_PENDING_PTY_KILLS_PER_TARGET)
}
/** Reads across every lease state on purpose: a kill issued while the provider was already gone
* tombstones its lease `terminated` and still leaves the remote process running. */
export function pendingSshPtyKillEntries(
leases: readonly SshRemotePtyLease[]
): SshPendingPtyKillEntry[] {
const entries: SshPendingPtyKillEntry[] = []
for (const lease of leases) {
if (lease.pendingKill) {
entries.push({ ptyId: lease.ptyId, intent: lease.pendingKill })
}
}
return entries
}
+5
View File
@@ -1,3 +1,5 @@
import type { SshPendingPtyKill } from './ssh-pending-pty-kill'
// ─── SSH Connection Types ───────────────────────────────────────────
export const MIN_SSH_RELAY_GRACE_PERIOD_SECONDS = 60
@@ -211,6 +213,9 @@ export type SshRemotePtyLease = {
updatedAt: number
lastAttachedAt?: number
lastDetachedAt?: number
/** A stop this client asked for and could not confirm, replayed on the next handshake to this
* same target. See `shared/ssh-pending-pty-kill.ts`. Never on the wire — client-local. */
pendingKill?: SshPendingPtyKill
}
/** Main-owned relay lease needed to reclaim PTY delivery after a desktop restart. */