fix(ssh): actually write the shell identity the reconnect fence reads

The lease field was declared, preserved on load, and read on reconnect — and
never written. Both spawn writers omitted it, so every lease carried only
"pty-N", the reconnect always took the no-expectation path, and the fence added
for it could not fire. The main reconnect went on attaching by id alone, which
is the replaced-relay case the fence exists for.

Worse than inert: a successful unfenced attach durably binds the pane to
whatever answered, so the wrong identity would be recorded and carried forward.

The host attests the identity at spawn and it was already in scope one line
above both writers.

The oracle for this had to pin the WRITE. Every other clause — the type, the
loader, the reader, the reconnect forwarding — was green throughout, because
each was correct in isolation; only nothing joined them. The test that covers
the spawn now seeds a host incarnation and requires it on the persisted lease,
and removing either writer reddens it.
This commit is contained in:
Neil
2026-08-12 17:36:54 -07:00
parent 5793f186e9
commit be02c4e7f5
2 changed files with 21 additions and 2 deletions
+9 -2
View File
@@ -5296,7 +5296,10 @@ describe('registerPtyHandlers', () => {
paneKey?: string
tabId?: string
}) => ({
id: 'ssh-pty'
id: 'ssh-pty',
// The host attests the shell's identity at spawn; the lease has to carry it or the
// reconnect fence has nothing to compare and attaches by pty id alone.
incarnationId: 'inc-host-ssh-pty'
})
)
const store = {
@@ -5383,6 +5386,9 @@ describe('registerPtyHandlers', () => {
worktreeId: 'wt-1',
tabId: 'tab-1',
leafId,
// Without this the field exists, loads and is read — and is never written, so the
// reconnect fence silently permits everything while every other clause stays green.
incarnationId: 'inc-host-ssh-pty',
state: 'attached'
})
)
@@ -5393,7 +5399,8 @@ describe('registerPtyHandlers', () => {
worktreeId: 'wt-1',
tabId: 'tab-1',
leafId,
ptyId: 'ssh-pty'
ptyId: 'ssh-pty',
incarnationId: 'inc-host-ssh-pty'
})
expect(store.persistPtyBinding.mock.calls.at(-1)).toHaveLength(1)
+12
View File
@@ -88,6 +88,7 @@ import {
isSshPtyIdentityMismatchError,
isSshPtyNotFoundError
} from '../providers/ssh-pty-errors'
import { isRelayAttestedPtyIncarnationId } from '../../shared/pty-incarnation'
import { parseAppSshPtyId, toAppSshPtyId, toRelaySshPtyId } from '../providers/ssh-pty-id'
import { createPtySpawnTiming } from './pty-spawn-timing'
import {
@@ -5273,6 +5274,12 @@ export function registerPtyHandlers(
...(typeof args.leafId === 'string' && isTerminalLeafId(args.leafId)
? { leafId: args.leafId }
: {}),
// The shell this lease names, as the host just attested it. Without it the record says
// only "pty-N", and a replaced relay reissues that from 1 — so reconnect would have
// nothing to compare and would attach whatever now answers to the id.
...(isRelayAttestedPtyIncarnationId(result.incarnationId)
? { incarnationId: result.incarnationId }
: {}),
state: 'attached',
lastAttachedAt: Date.now()
})
@@ -6791,6 +6798,11 @@ export function registerPtyHandlers(
...(typeof args.worktreeId === 'string' ? { worktreeId: args.worktreeId } : {}),
...(typeof args.tabId === 'string' ? { tabId: args.tabId } : {}),
...(validatedLeafId ? { leafId: validatedLeafId } : {}),
// See the sibling writer: a lease that names only "pty-N" cannot survive a relay that
// reissues ids from 1.
...(isRelayAttestedPtyIncarnationId(result.incarnationId)
? { incarnationId: result.incarnationId }
: {}),
state: 'attached',
lastAttachedAt: Date.now()
})