fix(ssh): preserve generation-qualified teardown state

This commit is contained in:
Brennan Benson
2026-07-13 19:04:26 -07:00
parent 4257e3a082
commit b44ca1abe5
6 changed files with 96 additions and 26 deletions
+10 -6
View File
@@ -2469,11 +2469,15 @@ describe('registerPtyHandlers', () => {
})
).rejects.toThrow('SSH_SESSION_EXPIRED: remote-pty')
expect(store.markSshRemotePtyLease).toHaveBeenCalledWith('ssh-1', 'remote-pty', 'expired')
expect(store.markSshRemotePtyLease).toHaveBeenCalledWith(
'ssh-1',
'ssh:ssh-1@@remote-pty',
'expired'
)
})
it('marks a scoped SSH session expired using the raw relay lease id', async () => {
const scopedPtyId = 'ssh:ssh-1@@remote-pty'
it('marks a generation-scoped SSH session expired using its exact lease identity', async () => {
const scopedPtyId = 'ssh:ssh-1@@boot-a@@remote-pty'
const sshSpawn = vi.fn(async () => {
throw new Error('SSH_SESSION_EXPIRED: remote-pty')
})
@@ -2527,7 +2531,7 @@ describe('registerPtyHandlers', () => {
deletePtyOwnership(scopedPtyId)
}
expect(store.markSshRemotePtyLease).toHaveBeenCalledWith('ssh-1', 'remote-pty', 'expired')
expect(store.markSshRemotePtyLease).toHaveBeenCalledWith('ssh-1', scopedPtyId, 'expired')
expect(openCodeClearPtyMock).toHaveBeenCalledWith(scopedPtyId)
expect(piClearPtyMock).toHaveBeenCalledWith(scopedPtyId)
})
@@ -6677,7 +6681,7 @@ describe('registerPtyHandlers', () => {
persistHostSessionBinding?: boolean
}): Promise<{ id: string }>
}
const appPtyId = 'ssh:ssh-expired-runtime@@relay-pty'
const appPtyId = 'ssh:ssh-expired-runtime@@boot-a@@relay-pty'
const remoteWrite = vi.fn()
registerSshPtyProvider('ssh-expired-runtime', {
spawn: vi.fn(async () => {
@@ -6751,7 +6755,7 @@ describe('registerPtyHandlers', () => {
expect(store.markSshRemotePtyLease).toHaveBeenCalledWith(
'ssh-expired-runtime',
'relay-pty',
appPtyId,
'expired'
)
expect(store.upsertSshRemotePtyLease).not.toHaveBeenCalled()
+2 -10
View File
@@ -3755,9 +3755,7 @@ export function registerPtyHandlers(
if (effectiveSessionAppId !== undefined && !isIdentityMismatch) {
clearProviderPtyState(effectiveSessionAppId)
deletePtyOwnership(effectiveSessionAppId)
}
if (!isIdentityMismatch) {
store?.markSshRemotePtyLease(args.connectionId, effectiveSessionRelayId, 'expired')
store?.markSshRemotePtyLease(args.connectionId, effectiveSessionAppId, 'expired')
}
}
if (isMintedSessionId && sessionId !== undefined) {
@@ -4689,13 +4687,7 @@ export function registerPtyHandlers(
if (effectiveSessionAppId !== undefined && !isIdentityMismatch) {
clearProviderPtyState(effectiveSessionAppId)
deletePtyOwnership(effectiveSessionAppId)
}
if (!isIdentityMismatch) {
store?.markSshRemotePtyLease(
args.connectionId,
effectiveSessionRelayId,
'expired'
)
store?.markSshRemotePtyLease(args.connectionId, effectiveSessionAppId, 'expired')
}
}
// Why: if buildPtyHostEnv materialized provider state for this minted
+19 -7
View File
@@ -1332,7 +1332,11 @@ describe('SSH IPC handlers', () => {
expect(mockConnectionManager.connect).toHaveBeenCalledWith(target)
expect(mockForceStopRelayForTarget).toHaveBeenCalledWith(conn, 'ssh-1')
expect(mockStore.markSshRemotePtyLease).toHaveBeenCalledWith('ssh-1', 'pty-1', 'expired')
expect(mockStore.markSshRemotePtyLease).toHaveBeenCalledWith(
'ssh-1',
'ssh:ssh-1@@pty-1',
'expired'
)
expect(mockStore.markSshRemotePtyLease).not.toHaveBeenCalledWith(
'ssh-1',
'pty-expired',
@@ -1341,7 +1345,7 @@ describe('SSH IPC handlers', () => {
expect(mockConnectionManager.disconnect).toHaveBeenCalledWith('ssh-1')
})
it('ssh:resetRelay clears scoped live PTYs while expiring raw leases', async () => {
it('ssh:resetRelay clears scoped live PTYs and exact generation-qualified leases', async () => {
const target: SshTarget = {
id: 'ssh-1',
label: 'Server',
@@ -1354,17 +1358,21 @@ describe('SSH IPC handlers', () => {
mockConnectionManager.connect.mockResolvedValue(conn)
mockConnectionManager.getConnection.mockReturnValue(undefined)
mockStore.getSshRemotePtyLeases.mockReturnValue([
{ targetId: 'ssh-1', ptyId: 'pty-lease', state: 'detached' }
{ targetId: 'ssh-1', ptyId: 'pty-lease', relayInstanceId: 'boot-a', state: 'detached' }
])
vi.mocked(getPtyIdsForConnection).mockReturnValue(['ssh:ssh-1@@pty-live'])
await handlers.get('ssh:resetRelay')!(null, { targetId: 'ssh-1' })
expect(mockStore.markSshRemotePtyLease).toHaveBeenCalledWith('ssh-1', 'pty-lease', 'expired')
expect(mockStore.markSshRemotePtyLease).toHaveBeenCalledWith(
'ssh-1',
'ssh:ssh-1@@boot-a@@pty-lease',
'expired'
)
expect(clearProviderPtyState).toHaveBeenCalledWith('ssh:ssh-1@@pty-live')
expect(clearProviderPtyState).toHaveBeenCalledWith('ssh:ssh-1@@pty-lease')
expect(clearProviderPtyState).toHaveBeenCalledWith('ssh:ssh-1@@boot-a@@pty-lease')
expect(deletePtyOwnership).toHaveBeenCalledWith('ssh:ssh-1@@pty-live')
expect(deletePtyOwnership).toHaveBeenCalledWith('ssh:ssh-1@@pty-lease')
expect(deletePtyOwnership).toHaveBeenCalledWith('ssh:ssh-1@@boot-a@@pty-lease')
expect(mockConnectionManager.disconnect).toHaveBeenCalledWith('ssh-1')
})
@@ -1667,7 +1675,11 @@ describe('SSH IPC handlers', () => {
expect(mockStore.markSshRemotePtyLeases).not.toHaveBeenCalledWith('ssh-1', 'terminated')
expect(mockStore.markSshRemotePtyLeases).toHaveBeenCalledWith('ssh-1', 'detached')
expect(mockStore.markSshRemotePtyLease).toHaveBeenCalledWith('ssh-1', 'pty-1', 'expired')
expect(mockStore.markSshRemotePtyLease).toHaveBeenCalledWith(
'ssh-1',
'ssh:ssh-1@@pty-1',
'expired'
)
expect(mockForceStopRelayForTarget).toHaveBeenCalledWith(conn, 'ssh-1')
})
+3 -2
View File
@@ -1106,8 +1106,9 @@ export function registerSshHandlers(
const ptyIds = new Set(getPtyIdsForConnection(targetId))
for (const lease of persistedStore!.getSshRemotePtyLeases(targetId)) {
if (lease.state !== 'terminated' && lease.state !== 'expired') {
ptyIds.add(lease.ptyId)
persistedStore!.markSshRemotePtyLease(targetId, lease.ptyId, 'expired')
const appPtyId = toAppSshPtyId(targetId, lease.ptyId, lease.relayInstanceId)
ptyIds.add(appPtyId)
persistedStore!.markSshRemotePtyLease(targetId, appPtyId, 'expired')
}
}
// Why: reset force-kills the remote relay daemon, so every local PTY
+57
View File
@@ -14,6 +14,7 @@ import {
statSync,
symlinkSync
} from 'node:fs'
import type * as NodeFsPromises from 'node:fs/promises'
import { join } from 'node:path'
import { tmpdir } from 'node:os'
import type {
@@ -8872,6 +8873,62 @@ describe('Store', () => {
)
})
it('keeps an SSH generation migration journaled behind an in-flight stale save', async () => {
vi.useFakeTimers()
const actualFs = await vi.importActual<typeof NodeFsPromises>('node:fs/promises')
let releaseWrite!: () => void
let markWriteStarted!: () => void
const writeCanFinish = new Promise<void>((resolve) => {
releaseWrite = resolve
})
const writeStarted = new Promise<void>((resolve) => {
markWriteStarted = resolve
})
let blockNextDataWrite = true
vi.doMock('node:fs/promises', () => ({
...actualFs,
writeFile: async (...args: unknown[]) => {
const path = String(args[0])
if (blockNextDataWrite && path.includes('orca-data.json.') && path.endsWith('.tmp')) {
blockNextDataWrite = false
markWriteStarted()
await writeCanFinish
}
return Reflect.apply(actualFs.writeFile, actualFs, args)
}
}))
try {
const store = await createStore()
store.upsertSshRemotePtyLease({
targetId: 'ssh-1',
ptyId: 'remote-pty',
state: 'detached'
})
store.updateUI({ sidebarWidth: 401 })
vi.advanceTimersByTime(1_000)
await writeStarted
expect(store.migrateLegacySshRemotePtyLeaseGeneration('ssh-1', 'remote-pty', 'boot-a')).toBe(
true
)
releaseWrite()
await store.waitForPendingWrite()
expect(readTerminalTeardownIntentJournal()).toContainEqual(
expect.objectContaining({ kind: 'ssh-migrate-generation', relayInstanceId: 'boot-a' })
)
const reloaded = await createStore()
expect(reloaded.getSshRemotePtyLeases('ssh-1')).toEqual([
expect.objectContaining({ ptyId: 'remote-pty', relayInstanceId: 'boot-a' })
])
} finally {
vi.clearAllTimers()
vi.doUnmock('node:fs/promises')
vi.resetModules()
vi.useRealTimers()
}
})
it('preserves exact local and runtime close intent across app restart', async () => {
const store = await createStore()
store.upsertPendingLocalPtyShutdown({
+5 -1
View File
@@ -3793,7 +3793,11 @@ export class Store {
// is not what the file holds.
if (this.writeGeneration === gen) {
this.lastWrittenStateHash = stateHash
this.compactTerminalTeardownIntentJournal()
// Why: a teardown mutation can append while this stale payload is in
// flight. Keep its journal record until that newer state reaches disk.
if (this.computeStateHash() === stateHash) {
this.compactTerminalTeardownIntentJournal()
}
}
} finally {
if (!renamed) {