mirror of
https://github.com/stablyai/orca.git
synced 2026-10-25 08:04:05 +00:00
fix(pty): retry rejected SSH cleanup safely
This commit is contained in:
@@ -6638,6 +6638,7 @@
|
||||
"oracle": "Deterministic tests cover bounded stale-handle replacement, suspended heartbeat clocks, cold and established subscription failure, ten partition/recovery cycles, automatic-recovery cutoff, manual reconnect, and exact direct SSH binding recovery. They assert one atomic store publication clears only exact-target PTY indexes, null-PTY activation remains unchanged, relay identity survives, Git and folder panes retry symmetrically, another target/local/WSL/runtime panes remain byte-identical through target snapshot hydration and reconnect, only an accepted exact failure or timeout starts the second attempt, two 31-second timeouts cannot start a third settlement-triggered attempt, rejected stale/mismatched acknowledgements preserve every store map, and concurrent split-pane spawn and reattach callbacks both commit under the same attempt ID after the first success replaces pending state with live shared authority. A sibling failure revokes that shared authority and starts exactly one second attempt; duplicate failures and late first-attempt PTY callbacks preserve the second attempt and every state map. Attempt-two failure retains continuation authority for later siblings, primary exit promotes a bound survivor or preserves the lease until a late sibling binds, and primary plus non-primary detach retain exact authority and history on both resulting tabs. Both remount callbacks consume split-count activity suppression, intentional dispose emits no failure/timeout, and a same-attempt StrictMode remount still owns one timeout. Hydration clears an untrusted PTY hint without clearing its current pending owner, healthy current-authority bindings suppress correction, hydration finalizes once, and reconnect emits no paired close lifecycle. A provider-level session-ID reattach returns an incarnation, then a legacy exit without an incarnation must resolve to that returned identity rather than minting a fallback identity. The shared-control oracle withholds the first encrypted ready frame, starts one RPC, triggers the successful-probe refresh, then requires exactly two client connections, one host request, a successful response, zero pending calls, and zero retained request bytes. Tests also assert one unsubscribe per remote-runtime epoch, observable recovery phases, stable PTY identity, resumed snapshot/output/input, no healthy state before replay, no retry or input after cutoff, a new manual epoch against the same PTY, quiet recovery UI with an explicit Reconnect action, pane-close state cleanup, one stable create mutation id, old-runtime no-retry behavior, cross-process PTY adoption, and bounded in-flight coordination.",
|
||||
"commands": [
|
||||
"pnpm exec vitest run --config config/vitest.config.ts src/main/ipc/pty.test.ts src/main/ssh/ssh-relay-session.test.ts",
|
||||
"pnpm exec vitest run --config config/vitest.config.ts src/main/ipc/pty.test.ts src/main/persistence.test.ts src/main/ssh/ssh-relay-session.test.ts",
|
||||
"pnpm exec vitest run --config config/vitest.config.ts src/main/providers/ssh-pty-provider-reattach-incarnation.test.ts --reporter=dot",
|
||||
"pnpm exec vitest run --config config/vitest.config.ts src/renderer/src/startup/ssh-startup-reconnect.test.ts src/renderer/src/lib/resolved-worktree-execution-host.test.ts src/renderer/src/components/terminal/background-terminal-worktree-mount.test.ts src/renderer/src/runtime/sync-runtime-graph-scheduling.test.ts src/renderer/src/components/terminal-pane/use-terminal-pane-lifecycle.test.ts src/renderer/src/components/terminal-pane/pty-connection.test.ts src/renderer/src/components/terminal-pane/remote-runtime-pty-transport.test.ts src/renderer/src/runtime/remote-runtime-session-tabs-inflight.test.ts src/renderer/src/runtime/web-session-terminal-handle-events.test.ts src/renderer/src/store/slices/terminal-pty-identity-replacement.test.ts",
|
||||
"pnpm exec vitest run --config config/vitest.config.ts src/renderer/src/components/terminal-pane/pty-transport.test.ts",
|
||||
@@ -6653,6 +6654,7 @@
|
||||
],
|
||||
"testFiles": [
|
||||
"src/main/ipc/pty.test.ts",
|
||||
"src/main/persistence.test.ts",
|
||||
"src/main/ssh/ssh-relay-session.test.ts",
|
||||
"src/main/providers/ssh-pty-provider-reattach-incarnation.test.ts",
|
||||
"src/renderer/src/startup/ssh-startup-reconnect.test.ts",
|
||||
@@ -6717,6 +6719,12 @@
|
||||
"reconnect may reclaim a cleanup-only PTY lease but never registers or persists it as a rejected terminal surface"
|
||||
]
|
||||
},
|
||||
{
|
||||
"file": "src/main/persistence.test.ts",
|
||||
"assertions": [
|
||||
"a cleanup-purpose lease replaces stale surface and cleanup identity when a relay PTY id is reused"
|
||||
]
|
||||
},
|
||||
{
|
||||
"file": "src/renderer/src/startup/ssh-startup-reconnect.test.ts",
|
||||
"assertions": [
|
||||
|
||||
@@ -8706,10 +8706,10 @@ describe('registerPtyHandlers', () => {
|
||||
}
|
||||
liveRemotePtys.delete(id)
|
||||
})
|
||||
registerSshPtyProvider(connectionId, {
|
||||
const provider = {
|
||||
spawn: vi.fn(async () => {
|
||||
liveRemotePtys.add(appPtyId)
|
||||
return { id: appPtyId }
|
||||
return { id: appPtyId, incarnationId: 'rejected-split-incarnation' }
|
||||
}),
|
||||
write: vi.fn(),
|
||||
resize: vi.fn(),
|
||||
@@ -8731,11 +8731,20 @@ describe('registerPtyHandlers', () => {
|
||||
revive: vi.fn(),
|
||||
getDefaultShell: vi.fn(),
|
||||
getProfiles: vi.fn()
|
||||
} as never)
|
||||
}
|
||||
registerSshPtyProvider(connectionId, provider as never)
|
||||
let cleanupLease: Record<string, unknown> | undefined
|
||||
const store = {
|
||||
persistPtyBinding: vi.fn(() => false),
|
||||
upsertSshRemotePtyLease: vi.fn(),
|
||||
markSshRemotePtyLease: vi.fn()
|
||||
upsertSshRemotePtyLease: vi.fn((lease) => {
|
||||
cleanupLease = { ...lease }
|
||||
}),
|
||||
getSshRemotePtyLeases: vi.fn(() => (cleanupLease ? [cleanupLease] : [])),
|
||||
markSshRemotePtyLease: vi.fn((_targetId, _ptyId, state) => {
|
||||
if (cleanupLease) {
|
||||
cleanupLease.state = state
|
||||
}
|
||||
})
|
||||
}
|
||||
let controller: RuntimeSpawnController | null = null
|
||||
const runtime = {
|
||||
@@ -8790,6 +8799,10 @@ describe('registerPtyHandlers', () => {
|
||||
targetId: connectionId,
|
||||
ptyId: 'rejected-split-pty',
|
||||
worktreeId: 'repo-1::/remote/repo',
|
||||
cleanupPending: true,
|
||||
cleanupExpectedPaneKey: `tab-rejected-split:${leafId}`,
|
||||
cleanupExpectedTabId: 'tab-rejected-split',
|
||||
cleanupExpectedIncarnationId: 'rejected-split-incarnation',
|
||||
state: 'attached'
|
||||
})
|
||||
)
|
||||
@@ -8800,6 +8813,15 @@ describe('registerPtyHandlers', () => {
|
||||
expect.objectContaining({ message: 'injected remote shutdown failure' })
|
||||
)
|
||||
|
||||
unregisterSshPtyProvider(connectionId)
|
||||
await expect(handlers.get('pty:kill')!(null, { id: appPtyId })).rejects.toThrow(
|
||||
'requires reconnect'
|
||||
)
|
||||
expect(liveRemotePtys.has(appPtyId)).toBe(true)
|
||||
expect(getPtyIdsForConnection(connectionId)).toEqual([appPtyId])
|
||||
expect(store.markSshRemotePtyLease).not.toHaveBeenCalled()
|
||||
|
||||
registerSshPtyProvider(connectionId, provider as never)
|
||||
shutdownFails = false
|
||||
await handlers.get('pty:kill')!(null, { id: appPtyId })
|
||||
|
||||
|
||||
+62
-8
@@ -1127,6 +1127,21 @@ async function shutdownFreshPtyAfterBindingFailure(
|
||||
finishPtyShutdown(id, connectionId, store)
|
||||
}
|
||||
|
||||
function isPendingSshPtyCleanup(
|
||||
id: string,
|
||||
connectionId: string,
|
||||
store: Store | undefined
|
||||
): boolean {
|
||||
const relayPtyId = getRelayPtyId(connectionId, id)
|
||||
const leases = store?.getSshRemotePtyLeases?.(connectionId)
|
||||
return Boolean(
|
||||
leases?.some(
|
||||
(lease) =>
|
||||
lease.ptyId === relayPtyId && lease.cleanupPending === true && lease.state !== 'terminated'
|
||||
)
|
||||
)
|
||||
}
|
||||
|
||||
// ─── Host PTY env assembly ──────────────────────────────────────────
|
||||
// Why: centralize host-local env injections so both spawn paths (local + daemon) get them; implemented twice they drifted, silently breaking daemon PTYs.
|
||||
|
||||
@@ -5185,7 +5200,7 @@ export function registerPtyHandlers(
|
||||
markNativeWindowsConptyPty(result.id)
|
||||
}
|
||||
const relayResultId = getRelayPtyId(args.connectionId, result.id)
|
||||
const persistSshLease = (includeSurfaceIdentity = true): void => {
|
||||
const persistSshLease = (cleanupPending = false): void => {
|
||||
if (!store || !args.connectionId) {
|
||||
return
|
||||
}
|
||||
@@ -5194,14 +5209,24 @@ export function registerPtyHandlers(
|
||||
targetId: args.connectionId,
|
||||
ptyId: relayResultId,
|
||||
...(typeof args.worktreeId === 'string' ? { worktreeId: args.worktreeId } : {}),
|
||||
...(includeSurfaceIdentity && typeof args.tabId === 'string'
|
||||
? { tabId: args.tabId }
|
||||
: {}),
|
||||
...(includeSurfaceIdentity &&
|
||||
typeof args.leafId === 'string' &&
|
||||
isTerminalLeafId(args.leafId)
|
||||
...(!cleanupPending && typeof args.tabId === 'string' ? { tabId: args.tabId } : {}),
|
||||
...(!cleanupPending && typeof args.leafId === 'string' && isTerminalLeafId(args.leafId)
|
||||
? { leafId: args.leafId }
|
||||
: {}),
|
||||
cleanupPending,
|
||||
...(cleanupPending &&
|
||||
typeof args.tabId === 'string' &&
|
||||
typeof args.leafId === 'string' &&
|
||||
isValidTerminalTabId(args.tabId) &&
|
||||
isTerminalLeafId(args.leafId)
|
||||
? { cleanupExpectedPaneKey: makePaneKey(args.tabId, args.leafId) }
|
||||
: {}),
|
||||
...(cleanupPending && typeof args.tabId === 'string'
|
||||
? { cleanupExpectedTabId: args.tabId }
|
||||
: {}),
|
||||
...(cleanupPending && result.incarnationId
|
||||
? { cleanupExpectedIncarnationId: result.incarnationId }
|
||||
: {}),
|
||||
state: 'attached',
|
||||
lastAttachedAt: Date.now()
|
||||
})
|
||||
@@ -5249,7 +5274,7 @@ export function registerPtyHandlers(
|
||||
console.error('[pty] failed to persist runtime PTY binding after spawn:', err)
|
||||
if (!result.isReattach) {
|
||||
// Why: retain durable cleanup authority without reconnect resurrecting the rejected pane.
|
||||
persistSshLease(false)
|
||||
persistSshLease(true)
|
||||
await shutdownFreshPtyAfterBindingFailure(
|
||||
provider,
|
||||
result.id,
|
||||
@@ -5477,6 +5502,9 @@ export function registerPtyHandlers(
|
||||
provider = connectionId ? getProvider(connectionId) : getProviderForPty(ptyId)
|
||||
} catch {
|
||||
if (connectionId) {
|
||||
if (isPendingSshPtyCleanup(ptyId, connectionId, store)) {
|
||||
return false
|
||||
}
|
||||
// Why: runtime/CLI close can target a detached SSH PTY after its
|
||||
// provider was unregistered. Tombstone the lease so reconnect does
|
||||
// not revive a terminal the user explicitly closed.
|
||||
@@ -5584,6 +5612,9 @@ export function registerPtyHandlers(
|
||||
provider = connectionId ? getProvider(connectionId) : getProviderForPty(ptyId)
|
||||
} catch {
|
||||
if (connectionId) {
|
||||
if (isPendingSshPtyCleanup(ptyId, connectionId, store)) {
|
||||
return false
|
||||
}
|
||||
// Why: an absent SSH provider means there is no live target left to
|
||||
// await, but the relay lease must still be tombstoned.
|
||||
const incarnationId = finishPtyShutdown(ptyId, connectionId, store)
|
||||
@@ -6723,6 +6754,7 @@ export function registerPtyHandlers(
|
||||
...(typeof args.worktreeId === 'string' ? { worktreeId: args.worktreeId } : {}),
|
||||
...(typeof args.tabId === 'string' ? { tabId: args.tabId } : {}),
|
||||
...(validatedLeafId ? { leafId: validatedLeafId } : {}),
|
||||
cleanupPending: false,
|
||||
state: 'attached',
|
||||
lastAttachedAt: Date.now()
|
||||
})
|
||||
@@ -6759,6 +6791,25 @@ export function registerPtyHandlers(
|
||||
} catch (err) {
|
||||
console.error('[pty] failed to persist PTY binding after spawn:', err)
|
||||
if (!result.isReattach) {
|
||||
if (store && args.connectionId) {
|
||||
store.upsertSshRemotePtyLease({
|
||||
targetId: args.connectionId,
|
||||
ptyId: relayResultId,
|
||||
...(typeof args.worktreeId === 'string' ? { worktreeId: args.worktreeId } : {}),
|
||||
cleanupPending: true,
|
||||
...(typeof args.tabId === 'string' &&
|
||||
validatedLeafId &&
|
||||
isValidTerminalTabId(args.tabId)
|
||||
? { cleanupExpectedPaneKey: makePaneKey(args.tabId, validatedLeafId) }
|
||||
: {}),
|
||||
...(typeof args.tabId === 'string' ? { cleanupExpectedTabId: args.tabId } : {}),
|
||||
...(result.incarnationId
|
||||
? { cleanupExpectedIncarnationId: result.incarnationId }
|
||||
: {}),
|
||||
state: 'attached',
|
||||
lastAttachedAt: Date.now()
|
||||
})
|
||||
}
|
||||
await shutdownFreshPtyAfterBindingFailure(
|
||||
provider,
|
||||
result.id,
|
||||
@@ -7509,6 +7560,9 @@ export function registerPtyHandlers(
|
||||
}
|
||||
const provider = connectionId ? sshProviders.get(connectionId) : tryGetProviderForPty(args.id)
|
||||
if (!provider && connectionId) {
|
||||
if (isPendingSshPtyCleanup(args.id, connectionId, store)) {
|
||||
throw new Error('SSH PTY cleanup requires reconnect before shutdown can be confirmed')
|
||||
}
|
||||
// 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.
|
||||
|
||||
@@ -10822,6 +10822,83 @@ describe('Store', () => {
|
||||
])
|
||||
})
|
||||
|
||||
it('replaces stale surface identity with cleanup identity for a reused SSH PTY id', async () => {
|
||||
const store = await createStore()
|
||||
store.upsertSshRemotePtyLease({
|
||||
targetId: 'ssh-1',
|
||||
ptyId: 'remote-pty',
|
||||
worktreeId: 'wt-old',
|
||||
tabId: 'tab-old',
|
||||
leafId: TEST_LEAF_1,
|
||||
state: 'attached'
|
||||
})
|
||||
store.setWorkspaceSession({
|
||||
activeRepoId: 'r1',
|
||||
activeWorktreeId: 'wt-old',
|
||||
activeTabId: 'tab-old',
|
||||
tabsByWorktree: {
|
||||
'wt-old': [
|
||||
{
|
||||
id: 'tab-old',
|
||||
worktreeId: 'wt-old',
|
||||
title: 'Terminal',
|
||||
customTitle: null,
|
||||
color: null,
|
||||
sortOrder: 0,
|
||||
createdAt: 1,
|
||||
ptyId: 'ssh:ssh-1@@remote-pty'
|
||||
}
|
||||
]
|
||||
},
|
||||
terminalLayoutsByTabId: {
|
||||
'tab-old': {
|
||||
root: { type: 'leaf', leafId: TEST_LEAF_1 },
|
||||
activeLeafId: TEST_LEAF_1,
|
||||
expandedLeafId: null,
|
||||
ptyIdsByLeafId: { [TEST_LEAF_1]: 'ssh:ssh-1@@remote-pty' }
|
||||
}
|
||||
}
|
||||
})
|
||||
|
||||
store.upsertSshRemotePtyLease({
|
||||
targetId: 'ssh-1',
|
||||
ptyId: 'remote-pty',
|
||||
cleanupPending: true,
|
||||
cleanupExpectedPaneKey: `tab-new:${TEST_LEAF_2}`,
|
||||
cleanupExpectedTabId: 'tab-new',
|
||||
cleanupExpectedIncarnationId: 'incarnation-new',
|
||||
state: 'attached'
|
||||
})
|
||||
|
||||
expect(store.getSshRemotePtyLeases('ssh-1')).toEqual([
|
||||
expect.objectContaining({
|
||||
ptyId: 'remote-pty',
|
||||
cleanupPending: true,
|
||||
cleanupExpectedPaneKey: `tab-new:${TEST_LEAF_2}`,
|
||||
cleanupExpectedTabId: 'tab-new',
|
||||
cleanupExpectedIncarnationId: 'incarnation-new'
|
||||
})
|
||||
])
|
||||
expect(store.getSshRemotePtyLeases('ssh-1')[0]).not.toHaveProperty('tabId')
|
||||
expect(store.getSshRemotePtyLeases('ssh-1')[0]).not.toHaveProperty('leafId')
|
||||
expect(store.getWorkspaceSession().tabsByWorktree['wt-old'][0].ptyId).toBeNull()
|
||||
expect(store.getWorkspaceSession().terminalLayoutsByTabId['tab-old'].ptyIdsByLeafId).toEqual({})
|
||||
|
||||
store.upsertSshRemotePtyLease({
|
||||
targetId: 'ssh-1',
|
||||
ptyId: 'remote-pty',
|
||||
cleanupPending: true,
|
||||
cleanupExpectedTabId: 'tab-newer',
|
||||
state: 'attached'
|
||||
})
|
||||
expect(store.getSshRemotePtyLeases('ssh-1')[0]).toEqual(
|
||||
expect.objectContaining({ cleanupPending: true, cleanupExpectedTabId: 'tab-newer' })
|
||||
)
|
||||
expect(store.getSshRemotePtyLeases('ssh-1')[0]).not.toHaveProperty(
|
||||
'cleanupExpectedIncarnationId'
|
||||
)
|
||||
})
|
||||
|
||||
it('rejects mismatched scoped SSH remote PTY lease ids on write paths', async () => {
|
||||
const store = await createStore()
|
||||
|
||||
|
||||
@@ -1615,6 +1615,16 @@ function normalizeSshRemotePtyLease(value: unknown): SshRemotePtyLease | null {
|
||||
...(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 } : {}),
|
||||
...(raw.cleanupPending === true ? { cleanupPending: true } : {}),
|
||||
...(typeof raw.cleanupExpectedPaneKey === 'string'
|
||||
? { cleanupExpectedPaneKey: raw.cleanupExpectedPaneKey }
|
||||
: {}),
|
||||
...(typeof raw.cleanupExpectedTabId === 'string'
|
||||
? { cleanupExpectedTabId: raw.cleanupExpectedTabId }
|
||||
: {}),
|
||||
...(typeof raw.cleanupExpectedIncarnationId === 'string'
|
||||
? { cleanupExpectedIncarnationId: raw.cleanupExpectedIncarnationId }
|
||||
: {}),
|
||||
state,
|
||||
createdAt: typeof raw.createdAt === 'number' ? raw.createdAt : now,
|
||||
updatedAt: typeof raw.updatedAt === 'number' ? raw.updatedAt : now,
|
||||
@@ -7264,12 +7274,43 @@ export class Store {
|
||||
entry.targetId === normalizedLease.targetId && entry.ptyId === normalizedLease.ptyId
|
||||
)
|
||||
const existing = existingIndex !== -1 ? this.state.sshRemotePtyLeases[existingIndex] : undefined
|
||||
if (normalizedLease.cleanupPending === true && existing && !existing.cleanupPending) {
|
||||
this.clearSshRemotePtyBindingsForLeases(normalizedLease.targetId, [existing])
|
||||
}
|
||||
const next: SshRemotePtyLease = {
|
||||
...existing,
|
||||
...normalizedLease,
|
||||
createdAt: existing?.createdAt ?? normalizedLease.createdAt ?? now,
|
||||
updatedAt: normalizedLease.updatedAt ?? now
|
||||
}
|
||||
if (normalizedLease.cleanupPending === true) {
|
||||
next.cleanupPending = true
|
||||
delete next.tabId
|
||||
delete next.leafId
|
||||
delete next.cleanupExpectedPaneKey
|
||||
delete next.cleanupExpectedTabId
|
||||
delete next.cleanupExpectedIncarnationId
|
||||
Object.assign(next, {
|
||||
...(normalizedLease.cleanupExpectedPaneKey
|
||||
? { cleanupExpectedPaneKey: normalizedLease.cleanupExpectedPaneKey }
|
||||
: {}),
|
||||
...(normalizedLease.cleanupExpectedTabId
|
||||
? { cleanupExpectedTabId: normalizedLease.cleanupExpectedTabId }
|
||||
: {}),
|
||||
...(normalizedLease.cleanupExpectedIncarnationId
|
||||
? { cleanupExpectedIncarnationId: normalizedLease.cleanupExpectedIncarnationId }
|
||||
: {})
|
||||
})
|
||||
} else if (
|
||||
normalizedLease.cleanupPending === false ||
|
||||
normalizedLease.tabId !== undefined ||
|
||||
normalizedLease.leafId !== undefined
|
||||
) {
|
||||
delete next.cleanupPending
|
||||
delete next.cleanupExpectedPaneKey
|
||||
delete next.cleanupExpectedTabId
|
||||
delete next.cleanupExpectedIncarnationId
|
||||
}
|
||||
if (existingIndex !== -1) {
|
||||
this.state.sshRemotePtyLeases[existingIndex] = next
|
||||
} else {
|
||||
@@ -7381,6 +7422,28 @@ export class Store {
|
||||
this.flush()
|
||||
}
|
||||
|
||||
removeMatchingSshPtyCleanupLease(
|
||||
targetId: string,
|
||||
ptyId: string,
|
||||
cleanupExpectedIncarnationId: string | undefined
|
||||
): boolean {
|
||||
const relayPtyId = this.getRelayPtyIdForSshLeaseStorage(targetId, ptyId)
|
||||
const leases = this.state.sshRemotePtyLeases ?? []
|
||||
const next = leases.filter(
|
||||
(lease) =>
|
||||
lease.targetId !== targetId ||
|
||||
lease.ptyId !== relayPtyId ||
|
||||
lease.cleanupPending !== true ||
|
||||
lease.cleanupExpectedIncarnationId !== cleanupExpectedIncarnationId
|
||||
)
|
||||
if (next.length === leases.length) {
|
||||
return false
|
||||
}
|
||||
this.state.sshRemotePtyLeases = next
|
||||
this.flush()
|
||||
return true
|
||||
}
|
||||
|
||||
removeSshRemotePtyLease(targetId: string, ptyId: string): void {
|
||||
const relayPtyId = this.getRelayPtyIdForSshLeaseStorage(targetId, ptyId)
|
||||
const leases = (this.state.sshRemotePtyLeases ?? []).filter(
|
||||
|
||||
@@ -596,4 +596,173 @@ describe('SshRelaySession reconnect incarnation ordering', () => {
|
||||
)
|
||||
consoleError.mockRestore()
|
||||
})
|
||||
|
||||
it('retries a cleanup lease without restoring a terminal surface', async () => {
|
||||
const { mockConn, mockStore, mockPortForward, getMainWindow } = createMockDeps()
|
||||
const { getSshPtyProvider } = await import('../ipc/pty')
|
||||
const mockAttach = vi.fn().mockResolvedValue({ incarnationId: 'cleanup-incarnation' })
|
||||
const mockShutdown = vi.fn().mockResolvedValue(undefined)
|
||||
vi.mocked(getSshPtyProvider).mockReturnValue({
|
||||
attachForReconnect: mockAttach,
|
||||
listProcesses: vi.fn().mockResolvedValue([
|
||||
{
|
||||
id: 'ssh:target-1@@cleanup-pty',
|
||||
incarnationId: 'cleanup-incarnation',
|
||||
cwd: '/remote/repo',
|
||||
title: 'shell'
|
||||
}
|
||||
]),
|
||||
shutdown: mockShutdown,
|
||||
dispose: vi.fn()
|
||||
} as unknown as ReturnType<typeof getSshPtyProvider>)
|
||||
vi.mocked(mockStore.getSshRemotePtyLeases).mockReturnValue([
|
||||
{
|
||||
targetId: 'target-1',
|
||||
ptyId: 'cleanup-pty',
|
||||
worktreeId: 'repo-1::/remote/repo',
|
||||
cleanupPending: true,
|
||||
cleanupExpectedPaneKey: 'tab-cleanup:33333333-3333-4333-8333-333333333333',
|
||||
cleanupExpectedTabId: 'tab-cleanup',
|
||||
cleanupExpectedIncarnationId: 'cleanup-incarnation',
|
||||
state: 'detached'
|
||||
}
|
||||
] as ReturnType<typeof mockStore.getSshRemotePtyLeases>)
|
||||
const runtime = {
|
||||
onPtySpawned: vi.fn(),
|
||||
registerPty: vi.fn()
|
||||
}
|
||||
const session = new SshRelaySession(
|
||||
'target-1',
|
||||
getMainWindow,
|
||||
mockStore,
|
||||
mockPortForward,
|
||||
runtime as never
|
||||
)
|
||||
|
||||
await session.establish(mockConn)
|
||||
|
||||
expect(mockAttach).not.toHaveBeenCalled()
|
||||
expect(mockShutdown).toHaveBeenCalledWith(
|
||||
'ssh:target-1@@cleanup-pty',
|
||||
expect.objectContaining({ immediate: true, deadlineMs: expect.any(Number) })
|
||||
)
|
||||
expect(mockStore.removeMatchingSshPtyCleanupLease).toHaveBeenCalledWith(
|
||||
'target-1',
|
||||
'cleanup-pty',
|
||||
'cleanup-incarnation'
|
||||
)
|
||||
expect(runtime.onPtySpawned).not.toHaveBeenCalled()
|
||||
expect(runtime.registerPty).not.toHaveBeenCalled()
|
||||
expect(mockStore.persistPtyBinding).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('retains a cleanup lease when reconnect shutdown fails', async () => {
|
||||
const { mockConn, mockStore, mockPortForward, getMainWindow } = createMockDeps()
|
||||
const { getSshPtyProvider } = await import('../ipc/pty')
|
||||
const shutdown = vi.fn().mockRejectedValue(new Error('injected cleanup retry failure'))
|
||||
vi.mocked(getSshPtyProvider).mockReturnValue({
|
||||
attachForReconnect: vi.fn().mockResolvedValue({ incarnationId: 'cleanup-incarnation' }),
|
||||
listProcesses: vi.fn().mockResolvedValue([
|
||||
{
|
||||
id: 'ssh:target-1@@cleanup-pty',
|
||||
incarnationId: 'cleanup-incarnation',
|
||||
cwd: '/remote/repo',
|
||||
title: 'shell'
|
||||
}
|
||||
]),
|
||||
shutdown,
|
||||
dispose: vi.fn()
|
||||
} as unknown as ReturnType<typeof getSshPtyProvider>)
|
||||
vi.mocked(mockStore.getSshRemotePtyLeases).mockReturnValue([
|
||||
{
|
||||
targetId: 'target-1',
|
||||
ptyId: 'cleanup-pty',
|
||||
cleanupPending: true,
|
||||
cleanupExpectedTabId: 'tab-cleanup',
|
||||
cleanupExpectedIncarnationId: 'cleanup-incarnation',
|
||||
state: 'detached'
|
||||
}
|
||||
] as ReturnType<typeof mockStore.getSshRemotePtyLeases>)
|
||||
const warn = vi.spyOn(console, 'warn').mockImplementation(() => {})
|
||||
const runtime = { onPtySpawned: vi.fn(), registerPty: vi.fn() }
|
||||
const session = new SshRelaySession(
|
||||
'target-1',
|
||||
getMainWindow,
|
||||
mockStore,
|
||||
mockPortForward,
|
||||
runtime as never
|
||||
)
|
||||
|
||||
try {
|
||||
await session.establish(mockConn)
|
||||
expect(shutdown).toHaveBeenCalledOnce()
|
||||
expect(mockStore.removeMatchingSshPtyCleanupLease).not.toHaveBeenCalledWith(
|
||||
'target-1',
|
||||
'cleanup-pty',
|
||||
'cleanup-incarnation'
|
||||
)
|
||||
expect(runtime.onPtySpawned).not.toHaveBeenCalled()
|
||||
expect(runtime.registerPty).not.toHaveBeenCalled()
|
||||
} finally {
|
||||
warn.mockRestore()
|
||||
}
|
||||
})
|
||||
|
||||
it.each([
|
||||
['reused', 'different-incarnation', true],
|
||||
['unproven', undefined, false]
|
||||
])('does not auto-clean a %s relay PTY identity', async (_case, incarnationId, terminates) => {
|
||||
const { mockConn, mockStore, mockPortForward, getMainWindow } = createMockDeps()
|
||||
const { getSshPtyProvider } = await import('../ipc/pty')
|
||||
const shutdown = vi.fn()
|
||||
vi.mocked(getSshPtyProvider).mockReturnValue({
|
||||
attachForReconnect: vi.fn().mockResolvedValue({ incarnationId }),
|
||||
listProcesses: vi.fn().mockResolvedValue([
|
||||
{
|
||||
id: 'ssh:target-1@@cleanup-pty',
|
||||
incarnationId,
|
||||
cwd: '/remote/repo',
|
||||
title: 'shell'
|
||||
}
|
||||
]),
|
||||
shutdown,
|
||||
dispose: vi.fn()
|
||||
} as unknown as ReturnType<typeof getSshPtyProvider>)
|
||||
vi.mocked(mockStore.getSshRemotePtyLeases).mockReturnValue([
|
||||
{
|
||||
targetId: 'target-1',
|
||||
ptyId: 'cleanup-pty',
|
||||
cleanupPending: true,
|
||||
cleanupExpectedPaneKey: 'tab-cleanup:33333333-3333-4333-8333-333333333333',
|
||||
cleanupExpectedTabId: 'tab-cleanup',
|
||||
cleanupExpectedIncarnationId: 'expected-incarnation',
|
||||
state: 'detached'
|
||||
}
|
||||
] as ReturnType<typeof mockStore.getSshRemotePtyLeases>)
|
||||
const warn = vi.spyOn(console, 'warn').mockImplementation(() => {})
|
||||
const session = new SshRelaySession('target-1', getMainWindow, mockStore, mockPortForward, {
|
||||
onPtySpawned: vi.fn(),
|
||||
registerPty: vi.fn()
|
||||
} as never)
|
||||
|
||||
try {
|
||||
await session.establish(mockConn)
|
||||
expect(shutdown).not.toHaveBeenCalled()
|
||||
if (terminates) {
|
||||
expect(mockStore.removeMatchingSshPtyCleanupLease).toHaveBeenCalledWith(
|
||||
'target-1',
|
||||
'cleanup-pty',
|
||||
'expected-incarnation'
|
||||
)
|
||||
} else {
|
||||
expect(mockStore.removeMatchingSshPtyCleanupLease).not.toHaveBeenCalledWith(
|
||||
'target-1',
|
||||
'cleanup-pty',
|
||||
'expected-incarnation'
|
||||
)
|
||||
}
|
||||
} finally {
|
||||
warn.mockRestore()
|
||||
}
|
||||
})
|
||||
})
|
||||
|
||||
@@ -26,6 +26,7 @@ export function createMockDeps(): SshRelaySessionTestDeps {
|
||||
markSshRemotePtyLeasesAsync: vi.fn(),
|
||||
markSshRemotePtyLeasesForShutdown: vi.fn(),
|
||||
markSshRemotePtyLeasesAttachedAsync: vi.fn(),
|
||||
removeMatchingSshPtyCleanupLease: vi.fn().mockReturnValue(true),
|
||||
persistPtyBinding: vi.fn()
|
||||
} as unknown as Store
|
||||
const mockPortForward = {
|
||||
|
||||
@@ -531,46 +531,6 @@ describe('SshRelaySession', () => {
|
||||
)
|
||||
})
|
||||
|
||||
it('does not restore a cleanup-only lease as a terminal surface', async () => {
|
||||
const { mockConn, mockStore, mockPortForward, getMainWindow } = createMockDeps()
|
||||
const { getSshPtyProvider } = await import('../ipc/pty')
|
||||
const mockAttach = vi.fn().mockResolvedValue({ incarnationId: 'cleanup-incarnation' })
|
||||
vi.mocked(getSshPtyProvider).mockReturnValue({
|
||||
attachForReconnect: mockAttach,
|
||||
dispose: vi.fn()
|
||||
} as unknown as ReturnType<typeof getSshPtyProvider>)
|
||||
vi.mocked(mockStore.getSshRemotePtyLeases).mockReturnValue([
|
||||
{
|
||||
targetId: 'target-1',
|
||||
ptyId: 'cleanup-pty',
|
||||
worktreeId: 'repo-1::/remote/repo',
|
||||
state: 'detached'
|
||||
}
|
||||
] as ReturnType<typeof mockStore.getSshRemotePtyLeases>)
|
||||
const runtime = {
|
||||
onPtySpawned: vi.fn(),
|
||||
registerPty: vi.fn()
|
||||
}
|
||||
const session = new SshRelaySession(
|
||||
'target-1',
|
||||
getMainWindow,
|
||||
mockStore,
|
||||
mockPortForward,
|
||||
runtime as never
|
||||
)
|
||||
|
||||
await session.establish(mockConn)
|
||||
|
||||
expect(mockAttach).toHaveBeenCalledWith('cleanup-pty')
|
||||
expect(runtime.onPtySpawned).toHaveBeenCalledWith(
|
||||
'ssh:target-1@@cleanup-pty',
|
||||
'cleanup-incarnation',
|
||||
{ awaitsRegistration: false }
|
||||
)
|
||||
expect(runtime.registerPty).not.toHaveBeenCalled()
|
||||
expect(mockStore.persistPtyBinding).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('forwards a lease tab identity to reattach so a reset relay cannot cross-wire it', async () => {
|
||||
const { mockConn, mockStore, mockPortForward, getMainWindow } = createMockDeps()
|
||||
const { getSshPtyProvider } = await import('../ipc/pty')
|
||||
|
||||
@@ -2248,11 +2248,14 @@ export class SshRelaySession {
|
||||
const activeLeases = this.store
|
||||
.getSshRemotePtyLeases(this.targetId)
|
||||
.filter((lease) => lease.state !== 'terminated' && lease.state !== 'expired')
|
||||
const activeLeaseByPtyId = new Map(activeLeases.map((lease) => [lease.ptyId, lease]))
|
||||
const leasedPtyIds = activeLeases.map((lease) => lease.ptyId)
|
||||
const cleanupLeases = activeLeases.filter((lease) => lease.cleanupPending === true)
|
||||
const surfaceLeases = activeLeases.filter((lease) => lease.cleanupPending !== true)
|
||||
const cleanupPtyIds = new Set(cleanupLeases.map((lease) => lease.ptyId))
|
||||
const activeLeaseByPtyId = new Map(surfaceLeases.map((lease) => [lease.ptyId, lease]))
|
||||
const leasedPtyIds = surfaceLeases.map((lease) => lease.ptyId)
|
||||
// Why: pass pane identity so the relay can reject cross-generation id collisions; tabId falls back for pre-leafId leases.
|
||||
const expectedIdentityByPtyId = new Map(
|
||||
activeLeases
|
||||
surfaceLeases
|
||||
.map((lease): [string, ExpectedPtyIdentity] | null => {
|
||||
const expected = expectedIdentityForLease(lease)
|
||||
return expected ? [lease.ptyId, expected] : null
|
||||
@@ -2263,9 +2266,9 @@ export class SshRelaySession {
|
||||
// Why: after app restart ptyOwnership is empty, but durable SSH leases still describe grace-window survivors.
|
||||
const ptyIds = Array.from(
|
||||
new Set([
|
||||
...getPtyIdsForConnection(this.targetId).map((ptyId) =>
|
||||
toRelaySshPtyId(this.targetId, ptyId)
|
||||
),
|
||||
...getPtyIdsForConnection(this.targetId)
|
||||
.map((ptyId) => toRelaySshPtyId(this.targetId, ptyId))
|
||||
.filter((ptyId) => !cleanupPtyIds.has(ptyId)),
|
||||
...leasedPtyIds
|
||||
])
|
||||
)
|
||||
@@ -2274,6 +2277,13 @@ export class SshRelaySession {
|
||||
if (!ptyProvider || providerGeneration === null || this.mux !== mux) {
|
||||
return
|
||||
}
|
||||
void this.retryPendingPtyCleanup(ptyProvider, cleanupLeases, shouldContinue).catch((error) => {
|
||||
console.warn(
|
||||
`[ssh-relay-session] Cleanup retries failed for ${this.targetId}: ${
|
||||
error instanceof Error ? error.message : String(error)
|
||||
}`
|
||||
)
|
||||
})
|
||||
let nextPtyIndex = 0
|
||||
const worker = async (): Promise<void> => {
|
||||
while (shouldContinue()) {
|
||||
@@ -2315,6 +2325,83 @@ export class SshRelaySession {
|
||||
}
|
||||
}
|
||||
|
||||
private async retryPendingPtyCleanup(
|
||||
ptyProvider: SshPtyProvider,
|
||||
leases: SshPtyLease[],
|
||||
shouldContinue: () => boolean
|
||||
): Promise<void> {
|
||||
if (leases.length === 0) {
|
||||
return
|
||||
}
|
||||
const inventory = await ptyProvider.listProcesses({
|
||||
deadlineMs: Date.now() + SSH_PTY_REATTACH_ATTEMPT_TIMEOUT_MS
|
||||
})
|
||||
const processById = new Map(inventory.map((process) => [process.id, process]))
|
||||
const retireCleanupLease = (lease: SshPtyLease, clearRouting: boolean): void => {
|
||||
if (!shouldContinue()) {
|
||||
return
|
||||
}
|
||||
const retired = this.store.removeMatchingSshPtyCleanupLease(
|
||||
this.targetId,
|
||||
lease.ptyId,
|
||||
lease.cleanupExpectedIncarnationId
|
||||
)
|
||||
if (retired && clearRouting) {
|
||||
const appPtyId = toAppSshPtyId(this.targetId, lease.ptyId)
|
||||
clearProviderPtyState(appPtyId)
|
||||
deletePtyOwnership(appPtyId)
|
||||
}
|
||||
}
|
||||
let nextLeaseIndex = 0
|
||||
const worker = async (): Promise<void> => {
|
||||
while (shouldContinue()) {
|
||||
const lease = leases[nextLeaseIndex++]
|
||||
if (!lease) {
|
||||
return
|
||||
}
|
||||
const appPtyId = toAppSshPtyId(this.targetId, lease.ptyId)
|
||||
const process = processById.get(appPtyId)
|
||||
if (!process) {
|
||||
retireCleanupLease(lease, true)
|
||||
continue
|
||||
}
|
||||
if (
|
||||
!lease.cleanupExpectedIncarnationId ||
|
||||
!process.incarnationId ||
|
||||
process.incarnationId !== lease.cleanupExpectedIncarnationId
|
||||
) {
|
||||
if (process.incarnationId && lease.cleanupExpectedIncarnationId) {
|
||||
retireCleanupLease(lease, false)
|
||||
} else {
|
||||
console.warn(
|
||||
`[ssh-relay-session] PTY ${lease.ptyId} cleanup retry lacks incarnation proof for ${this.targetId}`
|
||||
)
|
||||
}
|
||||
continue
|
||||
}
|
||||
try {
|
||||
await ptyProvider.shutdown(appPtyId, {
|
||||
immediate: true,
|
||||
deadlineMs: Date.now() + SSH_PTY_REATTACH_ATTEMPT_TIMEOUT_MS
|
||||
})
|
||||
} catch (error) {
|
||||
if (!isSshPtyNotFoundError(error) && !isSshPtyIdentityMismatchError(error)) {
|
||||
console.warn(
|
||||
`[ssh-relay-session] PTY ${lease.ptyId} cleanup retry failed for ${this.targetId}: ${
|
||||
error instanceof Error ? error.message : String(error)
|
||||
}`
|
||||
)
|
||||
continue
|
||||
}
|
||||
}
|
||||
retireCleanupLease(lease, true)
|
||||
}
|
||||
}
|
||||
await Promise.all(
|
||||
Array.from({ length: Math.min(SSH_PTY_REATTACH_MAX_CONCURRENCY, leases.length) }, worker)
|
||||
)
|
||||
}
|
||||
|
||||
private async reattachKnownPty(args: {
|
||||
ptyProvider: SshPtyProvider
|
||||
ptyId: string
|
||||
|
||||
@@ -198,6 +198,10 @@ export type SshRemotePtyLease = {
|
||||
worktreeId?: string
|
||||
tabId?: string
|
||||
leafId?: string
|
||||
cleanupPending?: boolean
|
||||
cleanupExpectedPaneKey?: string
|
||||
cleanupExpectedTabId?: string
|
||||
cleanupExpectedIncarnationId?: string
|
||||
state: SshRemotePtyLeaseState
|
||||
createdAt: number
|
||||
updatedAt: number
|
||||
|
||||
Reference in New Issue
Block a user