fix(ssh): make a parked PTY delivery expire instead of going dark for good

Exhausting the per-generation recovery budget parks one PTY's delivery rather
than dropping the shared relay channel — right, because the channel is shared and
a retry count proves nothing. But the only escape it named was "the next relay
open", and a channel that stays healthy never gives it one. That leaves a pane
with no output and no way back, which is the exact state this change exists to
prevent; before this branch, exhaustion dropped the channel and the reconnect
ladder recovered the pane (loudly, at every sibling's expense).

The park is now a cooldown rather than a verdict: the next rejected frame after
it starts a fresh budget. Recovery is rate-limited, never abandoned, and the
containment that made parking right in the first place is untouched.

Elapsed time, not a timer, so there is nothing to cancel on teardown and no late
fire after dispose.
This commit is contained in:
Neil
2026-08-12 04:00:00 -07:00
parent 60ae0713da
commit 124e00e8a8
2 changed files with 72 additions and 12 deletions
@@ -427,6 +427,52 @@ describe('SshRelaySession rejected PTY delivery recovery', () => {
)
})
// The clause above pins the containment; this one pins that containment is not abandonment. The
// park's only stated escape is the next relay open, and a shared channel that stays healthy never
// gives it one — so a pane could sit with no output and no way back, which is the exact state
// this change exists to prevent. The park has to expire.
it('re-arms a parked PTY once its cooldown has passed rather than leaving it dark', async () => {
const { internals } = prepareSession()
const reattach = vi.fn().mockResolvedValue(false)
internals.reattachRejectedPty = reattach
getSshPtyProviderMock.mockReturnValue({ hasPty: () => true } as unknown as SshPtyProvider)
const warn = vi.spyOn(console, 'warn').mockImplementation(() => {})
await internals.acceptPtyData(rejectedPayload())
await vi.waitFor(
() =>
expect(warn).toHaveBeenCalledWith(
expect.stringContaining('PTY pty-bad delivery recovery exhausted')
),
{ timeout: 10_000 }
)
expect(reattach).toHaveBeenCalledTimes(12)
// Each later frame must carry its own delivery token — a retired delivery is dropped at the
// top of acceptPtyData, so reusing one would make both clauses below pass without proving
// anything. A re-announcing source is exactly what a new token looks like.
const nextFrame = (n: number) =>
rejectedPayload({
source: source({ spanId: `token-bad-${n}:0:4`, deliveryToken: `token-bad-${n}` })
})
// Parked: a further rejected frame must not retry yet, or the cooldown means nothing.
reattach.mockClear()
await internals.acceptPtyData(nextFrame(1))
expect(reattach).not.toHaveBeenCalled()
// Backdate the park instead of burning a minute of real time — the property under test is
// elapsed time, not which timer implementation measures it.
const parked = internals.rejectedPtyRecoveryAttempts.get('ssh:target-1@@pty-bad') as {
parkedAt: number
}
parked.parkedAt -= 60_000
await internals.acceptPtyData(nextFrame(2))
await vi.waitFor(() => expect(reattach).toHaveBeenCalled(), { timeout: 10_000 })
})
it('stops without an error when the rejected PTY exited during recovery', async () => {
const { internals, mux, session } = prepareSession()
const reattach = vi.fn().mockResolvedValue(false)
+26 -12
View File
@@ -142,6 +142,11 @@ const SSH_PTY_REATTACH_RETRY_JITTER_MS = 200
// forever — each one costs a store read, an attach round trip and a store write. Exhausting it parks
// this PTY's delivery; it never authorizes tearing anything down.
const SSH_REJECTED_PTY_RECOVERY_MAX_GENERATION_ATTEMPTS = 12
// Why a park expires: exhaustion parks one PTY's delivery instead of dropping the shared channel,
// and the escape it names — the next relay open — may never come while that channel stays healthy.
// A pane with no output and no way back is the state this change exists to prevent, so the park is
// a cooldown rather than a verdict: recovery is rate-limited, never abandoned.
const SSH_REJECTED_PTY_RECOVERY_PARK_MS = 60_000
const SSH_REJECTED_PTY_RECOVERY_RETRY_DELAY_MS = 150
const SSH_SOURCE_RECOVERY_CANCELLATION_FAILED = 'ssh_source_recovery_cancellation_failed'
@@ -324,6 +329,8 @@ export class SshRelaySession {
providerGeneration: number
generationAttempts: number
reported: boolean
/** When the budget ran out; 0 while it has not. Drives the park cooldown. */
parkedAt: number
}
>()
private readonly rejectedPtyRecoveryRetries = new Set<ReturnType<typeof setTimeout>>()
@@ -1858,20 +1865,27 @@ export class SshRelaySession {
const attempt =
previous?.providerGeneration === providerGeneration
? previous
: { providerGeneration, generationAttempts: 0, reported: false }
: { providerGeneration, generationAttempts: 0, reported: false, parkedAt: 0 }
if (attempt.generationAttempts >= SSH_REJECTED_PTY_RECOVERY_MAX_GENERATION_ATTEMPTS) {
if (!attempt.reported) {
attempt.reported = true
// Why park instead of dropping the relay channel: a retry count is not proof of anything, and
// the channel is shared — dropping it rotates provider authority, aborts every in-flight fs
// and git request on the target and stalls every sibling PTY over one PTY's delivery. The
// remote shell keeps running, its lease stands, and the next relay open reattaches it with a
// fresh delivery generation.
console.warn(
`[ssh-relay-session] PTY ${relayPtyId} delivery recovery exhausted for ${this.targetId}; parking its delivery until the next relay open`
)
// Why park instead of dropping the relay channel: a retry count is not proof of anything, and
// the channel is shared — dropping it rotates provider authority, aborts every in-flight fs
// and git request on the target and stalls every sibling PTY over one PTY's delivery. The
// remote shell keeps running and its lease stands.
if (attempt.parkedAt && Date.now() - attempt.parkedAt >= SSH_REJECTED_PTY_RECOVERY_PARK_MS) {
// The park has served its cooldown. Re-arm rather than leave the pane dark for good.
attempt.generationAttempts = 0
attempt.reported = false
attempt.parkedAt = 0
} else {
if (!attempt.reported) {
attempt.reported = true
attempt.parkedAt = Date.now()
console.warn(
`[ssh-relay-session] PTY ${relayPtyId} delivery recovery exhausted for ${this.targetId}; parking its delivery for ${SSH_REJECTED_PTY_RECOVERY_PARK_MS}ms or until the next relay open`
)
}
return
}
return
}
attempt.generationAttempts++
this.rejectedPtyRecoveryAttempts.set(appPtyId, attempt)