route first teardown through retained owner

This commit is contained in:
Brennan Benson
2026-07-12 21:09:57 -07:00
parent 94b1ffee99
commit 28444bd984
3 changed files with 132 additions and 49 deletions
+1
View File
@@ -3965,6 +3965,7 @@
"persisted local shutdown waits for daemon startup instead of accepting unknown-id success from the fallback provider",
"an in-flight replaced local provider cannot complete or delete the newer shutdown owner",
"local and SSH stale-provider not-found rejections preserve replacement ownership and coalesced callers fail closed",
"direct local and SSH kill requests share the retained owner and re-drive after in-flight provider replacement",
"host-scoped liveness lists only the selected provider instead of fanning out to unrelated SSH hosts",
"relay generation reuse removes stale retry ownership without touching the new PTY"
]
+86
View File
@@ -3447,6 +3447,92 @@ describe('registerPtyHandlers', () => {
vi.useRealTimers()
})
it('re-drives a direct local kill when its provider is replaced in flight', async () => {
vi.useFakeTimers()
const oldShutdown = makeDeferred()
const makeProvider = (shutdown: (id: string) => Promise<void>) =>
({
shutdown,
onData: vi.fn(() => () => {}),
onReplay: vi.fn(() => () => {}),
onExit: vi.fn(() => () => {})
}) as never
const oldProviderShutdown = vi.fn(() => oldShutdown.promise)
const newShutdown = vi.fn().mockResolvedValue(undefined)
const removePendingLocalPtyShutdown = vi.fn()
setLocalPtyProvider(makeProvider(oldProviderShutdown))
handlers.clear()
registerPtyHandlers(mainWindow as never, undefined, undefined, undefined, undefined, {
getPendingLocalPtyShutdowns: () => [],
upsertPendingLocalPtyShutdown: vi.fn(),
removePendingLocalPtyShutdown
} as never)
const kill = handlers.get('pty:kill')!(null, { id: 'daemon-pty' }) as Promise<void>
expect(oldProviderShutdown).toHaveBeenCalledOnce()
setLocalPtyProvider(makeProvider(newShutdown))
oldShutdown.resolve()
await expect(kill).rejects.toThrow('PTY shutdown still pending')
await vi.advanceTimersByTimeAsync(0)
expect(newShutdown).toHaveBeenCalledOnce()
expect(removePendingLocalPtyShutdown).toHaveBeenCalledWith('daemon-pty')
await vi.advanceTimersByTimeAsync(30_000)
expect(vi.getTimerCount()).toBe(0)
vi.useRealTimers()
})
it('re-drives a direct SSH kill when its provider is replaced in flight', async () => {
vi.useFakeTimers()
const oldShutdown = makeDeferred()
const newShutdown = vi.fn().mockResolvedValue(undefined)
const lease: Record<string, unknown> = {
targetId: 'ssh-1',
ptyId: 'relay-pty',
relayInstanceId: 'boot-1',
state: 'detached',
createdAt: 1,
updatedAt: 1
}
const store = {
getSshRemotePtyLeases: vi.fn(() => [lease]),
markSshRemotePtyShutdownRequested: vi.fn(() => {
lease.shutdownRequestedAt = 2
}),
markSshRemotePtyLease: vi.fn()
}
registerSshPtyProvider('ssh-1', {
shutdown: vi.fn(() => oldShutdown.promise),
onExit: vi.fn(() => () => {})
} as never)
handlers.clear()
registerPtyHandlers(
mainWindow as never,
undefined,
undefined,
undefined,
undefined,
store as never
)
const id = 'ssh:ssh-1@@boot-1@@relay-pty'
const kill = handlers.get('pty:kill')!(null, { id }) as Promise<void>
registerSshPtyProvider('ssh-1', {
shutdown: newShutdown,
onExit: vi.fn(() => () => {})
} as never)
oldShutdown.resolve()
await expect(kill).rejects.toThrow('PTY shutdown still pending')
await vi.advanceTimersByTimeAsync(0)
expect(newShutdown).toHaveBeenCalledOnce()
expect(store.markSshRemotePtyLease).toHaveBeenCalledWith('ssh-1', id, 'terminated')
unregisterSshPtyProvider('ssh-1')
await vi.advanceTimersByTimeAsync(30_000)
expect(vi.getTimerCount()).toBe(0)
vi.useRealTimers()
})
it('rebinds a persisted local shutdown after async daemon startup replaces the provider', async () => {
vi.useFakeTimers()
const startup = makeDeferred()
+45 -49
View File
@@ -183,6 +183,7 @@ type RetainedPtyShutdown = {
attempts: number
nextRetryAt: number
inFlight: Promise<void> | null
lastError: Error | null
}
const pendingSshShutdownRetries = new Map<string, RetainedPtyShutdown>()
const pendingLocalShutdownRetries = new Map<string, RetainedPtyShutdown>()
@@ -632,6 +633,7 @@ function attemptRetainedPtyShutdown(retained: RetainedPtyShutdown): Promise<void
return Promise.resolve()
}
retained.provider = currentProvider
retained.lastError = null
const attempt = shutdownProviderAndDetectExit(currentProvider, retained.id, retained.options)
.then((providerExitObserved) => {
// Why: provider replacement can finish while an older shutdown is in flight.
@@ -654,6 +656,7 @@ function attemptRetainedPtyShutdown(retained: RetainedPtyShutdown): Promise<void
completeRetainedPtyShutdown(retained, false)
return
}
retained.lastError = error instanceof Error ? error : new Error(String(error))
retained.attempts += 1
retained.nextRetryAt =
Date.now() +
@@ -692,7 +695,8 @@ function retainLocalPtyShutdown(
: attempts === 0
? 0
: Date.now() + 250,
inFlight: null
inFlight: null,
lastError: null
}
pendingLocalShutdownRetries.set(id, retained)
}
@@ -708,6 +712,36 @@ function retainLocalPtyShutdown(
return retained
}
function retainSshPtyShutdown(
id: string,
connectionId: string,
provider: IPtyProvider,
options: PtyShutdownOptions
): RetainedPtyShutdown {
activePtyStore?.markSshRemotePtyShutdownRequested?.(connectionId, id)
let retained = pendingSshShutdownRetries.get(id)
if (!retained) {
retained = {
id,
connectionId,
options,
provider,
store: activePtyStore,
attempts: 0,
nextRetryAt: 0,
inFlight: null,
lastError: null
}
pendingSshShutdownRetries.set(id, retained)
} else if (retained.provider !== provider) {
retained.provider = provider
retained.options = options
retained.nextRetryAt = 0
}
scheduleRetainedShutdownRetries()
return retained
}
// ─── Host PTY env assembly ──────────────────────────────────────────
// Why: both the LocalPtyProvider.buildSpawnEnv closure and the daemon-active
// fallback in pty:spawn need the same set of host-local env injections
@@ -1289,6 +1323,7 @@ function retryPersistedSshShutdowns(connectionId: string, provider: IPtyProvider
attempts: 0,
nextRetryAt: 0,
inFlight: null,
lastError: null,
options
}
pendingSshShutdownRetries.set(id, retained)
@@ -5246,17 +5281,6 @@ export function registerPtyHandlers(
const ownedConnectionId = ptyOwnership.get(args.id)
const parsedSshId = ownedConnectionId === undefined ? parseAppSshPtyId(args.id) : null
const connectionId = ownedConnectionId ?? parsedSshId?.connectionId
const retainedRetry = connectionId
? pendingSshShutdownRetries.get(args.id)
: pendingLocalShutdownRetries.get(args.id)
if (retainedRetry?.inFlight) {
await retainedRetry.inFlight
const ownerMap = connectionId ? pendingSshShutdownRetries : pendingLocalShutdownRetries
if (ownerMap.get(args.id) === retainedRetry) {
throw new Error('PTY shutdown still pending')
}
return
}
const provider = connectionId ? sshProviders.get(connectionId) : tryGetProviderForPty(args.id)
if (!provider && connectionId) {
// Why: a disconnected provider is not proof the relay process exited.
@@ -5271,43 +5295,15 @@ export function registerPtyHandlers(
...(args.expectedPaneKey ? { expectedPaneKey: args.expectedPaneKey } : {}),
...(args.expectedTabId ? { expectedTabId: args.expectedTabId } : {})
}
let providerExitObserved = false
try {
providerExitObserved = await shutdownProviderAndDetectExit(
shutdownProvider,
args.id,
shutdownOptions
)
} catch (err) {
if (!isPtyAlreadyGoneError(err)) {
// Why: a failed SSH shutdown can leave the remote process alive in
// the relay grace window; daemon failures have the same risk locally.
// Keep ownership/lease state so the user can retry.
if (connectionId) {
store?.markSshRemotePtyShutdownRequested?.(connectionId, args.id)
retryPersistedSshShutdowns(connectionId, shutdownProvider)
} else {
// Why: renderer identity is already retired; persist exact daemon identity before retry.
retainLocalPtyShutdown(args.id, shutdownOptions, 1)
}
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.
finishPtyShutdown(args.id, connectionId, store)
if (!connectionId) {
pendingLocalShutdownRetries.delete(args.id)
if (typeof store?.removePendingLocalPtyShutdown === 'function') {
store.removePendingLocalPtyShutdown(args.id)
}
scheduleRetainedShutdownRetries()
}
if (!providerExitObserved) {
runtime?.onPtyExit(args.id, -1)
rememberSyntheticKillExit(args.id)
sendPtyExitToRenderer({ id: args.id, code: -1 })
// Why: the first request and every retry need one generation-fenced owner;
// a separate direct path can accept a stale provider's late completion.
const retained = connectionId
? retainSshPtyShutdown(args.id, connectionId, shutdownProvider, shutdownOptions)
: retainLocalPtyShutdown(args.id, shutdownOptions)
await attemptRetainedPtyShutdown(retained)
const ownerMap = connectionId ? pendingSshShutdownRetries : pendingLocalShutdownRetries
if (ownerMap.get(args.id) === retained) {
throw retained.lastError ?? new Error('PTY shutdown still pending')
}
}
)