fix(ssh): restore saved port forwards on managed hosts and show their Ports tab (#27361)

* fix(ssh): restore saved port forwards on managed hosts; show Ports for managed workspaces

* test: annotate the mock connection casts

---------

Co-authored-by: m4air <m4air@Mac.localdomain>
This commit is contained in:
OrcaWin
2026-10-10 20:01:11 -07:00
committed by GitHub
co-authored by m4air
parent cf36928a69
commit 5f350a3786
18 changed files with 343 additions and 21 deletions
@@ -11,6 +11,7 @@ import {
testingTargets
} from './ssh-connect-attempt-registry'
import { connectionManager, getCurrentMainWindow } from './ssh-ipc-context'
import { restoreManagedHostPortForwards } from './ssh-managed-port-forward-restore'
import { requestCredential } from './ssh-passphrase'
import { clearRelayLostBackoff } from './ssh-relay-lost-backoff'
import {
@@ -108,6 +109,10 @@ export function handleSshConnectionStateChange(targetId: string, state: SshConne
}
if (!session) {
// An explicit connect restores through its own publish; this covers a transport that recovered.
if (state.status === 'connected' && !connectInFlight.has(targetId)) {
void restoreManagedHostPortForwards(targetId)
}
return
}
// Why: allow reconnect from both 'ready' and 'reconnecting'; without the latter, a failed relay deploy would permanently brick the session.
+2
View File
@@ -18,6 +18,7 @@ import { knownSshHostPlatform } from '../ssh/ssh-host-platform-memo'
import { getSshTargetRegistryStore } from '../ssh/ssh-target-registry'
import { allowsDirectSshRelay } from '../ssh/ssh-connection-store'
import { connectionManager, getCurrentMainWindow } from './ssh-ipc-context'
import { restoreManagedHostPortForwards } from './ssh-managed-port-forward-restore'
import {
broadcastSshState,
clearRelayStateOverride,
@@ -101,6 +102,7 @@ export function publishManagedServerConnect(
...(serving ? { serving } : {})
}
setSshHostServerStatus(targetId, managedServer)
void restoreManagedHostPortForwards(targetId)
const state: SshConnectionState = {
...(connectionManager!.getState(targetId) ?? { targetId, reconnectAttempt: 0 }),
targetId,
@@ -0,0 +1,69 @@
import { beforeEach, describe, expect, it, vi } from 'vitest'
const h = vi.hoisted(() => ({
restore: vi.fn(async () => {}),
removeAllForwards: vi.fn(async () => {}),
disconnect: vi.fn(async () => {}),
getState: vi.fn(() => ({ targetId: 't1', status: 'connected', error: null, reconnectAttempt: 0 }))
}))
vi.mock('./ssh-managed-port-forward-restore', () => ({ restoreManagedHostPortForwards: h.restore }))
vi.mock('./ssh-ipc-context', () => ({
connectionManager: { getState: h.getState, disconnect: h.disconnect, getConnection: vi.fn() },
currentRuntime: null,
getCurrentMainWindow: () => null,
persistedStore: null,
portForwardManager: { removeAllForwards: h.removeAllForwards }
}))
vi.mock('../ssh/ssh-target-registry', async (importOriginal) => ({
...(await importOriginal<Record<string, unknown>>()),
getSshTargetRegistryStore: () => null
}))
const { handleSshConnectionStateChange } = await import('./ssh-connection-state-callbacks')
const { publishManagedServerConnect } = await import('./ssh-host-server-connect')
const { disconnectRegisteredSshTarget } = await import('./ssh-session-teardown')
const { connectInFlight } = await import('./ssh-connect-attempt-registry')
const { getSshProviderAuthority } = await import('../ssh/ssh-provider-authority')
const { clearSshHostServerStatus, setSshHostServerStatus } =
await import('../ssh/ssh-host-server-status')
const connected = { targetId: 't1', status: 'connected', error: null, reconnectAttempt: 0 } as const
describe('saved port forwards on a managed host', () => {
beforeEach(() => {
vi.clearAllMocks()
clearSshHostServerStatus('t1')
})
it('restores them when a managed connect is published', () => {
publishManagedServerConnect('t1', 'env-1')
expect(h.restore).toHaveBeenCalledWith('t1')
})
it('restores them when the managed host transport reconnects on its own', () => {
setSshHostServerStatus('t1', { kind: 'managed', environmentId: 'env-1' })
handleSshConnectionStateChange('t1', { ...connected, status: 'reconnecting' })
expect(h.restore).not.toHaveBeenCalled()
handleSshConnectionStateChange('t1', connected)
expect(h.restore).toHaveBeenCalledWith('t1')
})
it('leaves an explicit connect to restore through its own publish', () => {
connectInFlight.set('t1', {
authority: getSshProviderAuthority('t1'),
promise: new Promise(() => {})
})
try {
handleSshConnectionStateChange('t1', connected)
expect(h.restore).not.toHaveBeenCalled()
} finally {
connectInFlight.delete('t1')
}
})
it('closes the local listeners on disconnect even though no relay session exists', async () => {
await disconnectRegisteredSshTarget('t1')
expect(h.removeAllForwards).toHaveBeenCalledWith('t1')
})
})
@@ -0,0 +1,26 @@
import { getSshHostServerStatus } from '../ssh/ssh-host-server-status'
import { getCurrentMainWindow } from './ssh-ipc-context'
import { restorePortForwards } from './ssh-port-forward-persistence'
const restoring = new Map<string, Promise<void>>()
/** A managed host has no relay session to restore its saved forwards, so its connects do. */
export function restoreManagedHostPortForwards(targetId: string): Promise<void> {
if (getSshHostServerStatus(targetId)?.kind !== 'managed') {
return Promise.resolve()
}
// Why chained: overlapping connects would otherwise both bind the same saved local port.
const previous = restoring.get(targetId) ?? Promise.resolve()
const next = previous
.then(() => restorePortForwards(targetId, getCurrentMainWindow))
.catch((error: unknown) => {
console.warn(`[ssh] Could not restore port forwards for ${targetId}:`, error)
})
.finally(() => {
if (restoring.get(targetId) === next) {
restoring.delete(targetId)
}
})
restoring.set(targetId, next)
return next
}
+12 -4
View File
@@ -4,6 +4,10 @@ import { getSshTargetRegistryStore } from '../ssh/ssh-target-registry'
import { connectionManager, portForwardManager } from './ssh-ipc-context'
import { broadcastPortForwards } from './ssh-renderer-broadcast'
function forwardKey(forward: SavedPortForward): string {
return `${forward.localPort}:${forward.remoteHost}:${forward.remotePort}`
}
// Why: after user add/remove/update the runtime manager is the source of truth — persist exactly its entries (unrestored ones handled by a separate helper).
export function persistPortForwards(targetId: string): void {
const active = portForwardManager!.listForwards(targetId)
@@ -21,12 +25,10 @@ export function persistPortForwards(targetId: string): void {
// Why: keep forwards that failed to restore in the persisted list so they retry on next reconnect instead of being silently dropped.
export function persistPortForwardsWithUnrestored(targetId: string): void {
const active = portForwardManager!.listForwards(targetId)
const activeKeys = new Set(active.map((f) => `${f.localPort}:${f.remoteHost}:${f.remotePort}`))
const activeKeys = new Set(active.map(forwardKey))
const existing = getSshTargetRegistryStore()!.getTarget(targetId)?.portForwards ?? []
const unrestored = existing.filter(
(pf) => !activeKeys.has(`${pf.localPort}:${pf.remoteHost}:${pf.remotePort}`)
)
const unrestored = existing.filter((pf) => !activeKeys.has(forwardKey(pf)))
const saved: SavedPortForward[] = [
...active.map((f) => ({
@@ -54,9 +56,15 @@ export async function restorePortForwards(
if (!conn) {
return
}
// Why: a forward bound to the replaced transport still holds its local port but reaches nothing.
await portForwardManager!.closeStaleForwards(targetId, conn)
const live = new Set(portForwardManager!.listForwards(targetId).map(forwardKey))
// Why: keep failed restores in persisted state — a failure may be transient (port temporarily busy), so retry on next reconnect.
for (const saved of target.portForwards) {
if (live.has(forwardKey(saved))) {
continue
}
// Why: a reconnect mid-loop swaps the connection object; bail on identity change so we don't add forwards to a stale conn (leaking listeners).
if (connectionManager!.getConnection(targetId) !== conn) {
return
@@ -0,0 +1,100 @@
import { beforeEach, describe, expect, it, vi } from 'vitest'
import type { PortForwardEntry, SavedPortForward } from '../../shared/ssh-types'
const h = vi.hoisted(() => {
const conn = { id: 'conn' }
const saved: { portForwards?: SavedPortForward[] } = {}
const active: PortForwardEntry[] = []
return {
conn,
saved,
active,
store: {
getTarget: vi.fn(() => ({ id: 't1', ...saved })),
updateTarget: vi.fn((_id: string, update: { portForwards?: SavedPortForward[] }) => {
saved.portForwards = update.portForwards
})
},
connectionManager: { getConnection: vi.fn(() => conn) },
portForwardManager: {
closeStaleForwards: vi.fn(async () => {}),
listForwards: vi.fn(() => [...active]),
addForward: vi.fn(
async (
connectionId: string,
_conn: unknown,
localPort: number,
remoteHost: string,
remotePort: number,
label?: string
) => {
const entry = {
id: `pf-${active.length + 1}`,
connectionId,
localPort,
remoteHost,
remotePort,
label
}
active.push(entry)
return entry
}
)
},
hostServerStatus: vi.fn<() => { kind: 'managed'; environmentId: string } | undefined>(() => ({
kind: 'managed',
environmentId: 'env-1'
})),
broadcastPortForwards: vi.fn()
}
})
vi.mock('../ssh/ssh-target-registry', () => ({ getSshTargetRegistryStore: () => h.store }))
vi.mock('./ssh-ipc-context', () => ({
connectionManager: h.connectionManager,
portForwardManager: h.portForwardManager,
getCurrentMainWindow: () => null
}))
vi.mock('./ssh-renderer-broadcast', () => ({ broadcastPortForwards: h.broadcastPortForwards }))
vi.mock('../ssh/ssh-host-server-status', () => ({ getSshHostServerStatus: h.hostServerStatus }))
import { restorePortForwards } from './ssh-port-forward-persistence'
import { restoreManagedHostPortForwards } from './ssh-managed-port-forward-restore'
const FORWARD = { localPort: 4100, remoteHost: '127.0.0.1', remotePort: 7860, label: 'app' }
describe('saved port forward restore', () => {
beforeEach(() => {
vi.clearAllMocks()
h.active.length = 0
h.saved.portForwards = [{ ...FORWARD }]
})
it('closes forwards bound to a replaced transport before re-adding the saved ones', async () => {
await restorePortForwards('t1', () => null)
expect(h.portForwardManager.closeStaleForwards).toHaveBeenCalledWith('t1', h.conn)
expect(h.portForwardManager.addForward).toHaveBeenCalledTimes(1)
expect(h.portForwardManager.closeStaleForwards.mock.invocationCallOrder[0]).toBeLessThan(
h.portForwardManager.addForward.mock.invocationCallOrder[0]
)
})
it('leaves a forward that is still live on the current transport alone', async () => {
await restorePortForwards('t1', () => null)
await restorePortForwards('t1', () => null)
expect(h.portForwardManager.addForward).toHaveBeenCalledTimes(1)
expect(h.saved.portForwards).toEqual([FORWARD])
})
it('restores a managed host once even when two connects race', async () => {
await Promise.all([restoreManagedHostPortForwards('t1'), restoreManagedHostPortForwards('t1')])
expect(h.portForwardManager.addForward).toHaveBeenCalledTimes(1)
expect(h.broadcastPortForwards).toHaveBeenCalledWith(expect.any(Function), 't1')
})
it('does nothing for a host that is not managed', async () => {
h.hostServerStatus.mockReturnValueOnce(undefined)
await restoreManagedHostPortForwards('t1')
expect(h.portForwardManager.addForward).not.toHaveBeenCalled()
})
})
+4 -1
View File
@@ -73,7 +73,10 @@ export async function teardownSshTargetTransport(
// Why: start the transport disconnect before session teardown; the async wrapper turns a sync throw into a rejection.
const transportDisconnect = (async () => connectionManager?.disconnect(targetId))()
const [teardownResult, disconnectResult] = await Promise.allSettled([
teardownActiveSshSession(targetId, teardown),
activeSessions.has(targetId)
? teardownActiveSshSession(targetId, teardown)
: // A managed host has no relay session, but its forwards still hold local ports.
(async () => portForwardManager?.removeAllForwards(targetId))(),
transportDisconnect
])
if (teardownResult.status === 'rejected') {
@@ -20,6 +20,8 @@ export type StartedPortForward = {
entry: PortForwardEntry
close: () => Promise<void>
dispose: () => void
/** False once `conn` no longer carries the transport this forward tunnels through. */
isLive?: (conn: SshConnection) => boolean
}
export type SshPortForwardProvider = {
+16
View File
@@ -412,6 +412,22 @@ describe('SshPortForwardManager', () => {
expect(manager.listForwards('conn-2')).toHaveLength(1)
})
it('closes only the ssh2 forwards bound to a client the connection replaced', async () => {
const conn = createMockConn()
// oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the mock implements the getClient/usesSystemSshTransport surface the ssh2 provider reads.
const sshConn = conn as never
await manager.addForward('conn-1', sshConn, 3000, 'localhost', 8080)
await manager.addForward('conn-2', sshConn, 3002, 'localhost', 8082)
await manager.closeStaleForwards('conn-1', sshConn)
expect(manager.listForwards('conn-1')).toHaveLength(1)
conn.getClient.mockReturnValue({ forwardOut: vi.fn() })
await manager.closeStaleForwards('conn-1', sshConn)
expect(manager.listForwards('conn-1')).toHaveLength(0)
expect(manager.listForwards('conn-2')).toHaveLength(1)
})
it('dispose removes all forwards', async () => {
const conn = createMockConn()
await manager.addForward('conn-1', conn as never, 3000, 'localhost', 8080)
+9
View File
@@ -182,6 +182,15 @@ export class SshPortForwardManager {
await Promise.all(toRemove.map((id) => this.removeForwardAsync(id)))
}
/** Closes this connection's forwards that tunnel through a transport `conn` replaced. */
async closeStaleForwards(connectionId: string, conn: SshConnection): Promise<void> {
const stale = [...this.forwards.entries()]
.filter(([, forward]) => forward.entry.connectionId === connectionId)
.filter(([, forward]) => forward.isLive?.(conn) === false)
.map(([id]) => id)
await Promise.all(stale.map((id) => this.removeForwardAsync(id)))
}
dispose(): void {
const ids = [...this.forwards.keys()]
for (const id of ids) {
+3 -1
View File
@@ -86,7 +86,9 @@ export class Ssh2PortForwardProvider implements SshPortForwardProvider {
close,
dispose: () => {
void close()
}
},
// Why: this forward dials through `client`; a reconnect gives the connection a new one.
isLive: (current) => !closed && current.getClient() === client
}
}
}
@@ -23,7 +23,7 @@ export type ActivityBarItem = {
gitOnly?: boolean
/** When true, shown only for folder workspaces. */
folderOnly?: boolean
/** When true, shown only for worktrees that belong to an SSH repo. */
/** When true, shown only for workspaces on an SSH host. */
sshOnly?: boolean
/** Host-owned health indicator; plugin content cannot style this chrome. */
statusIndicator?: CheckStatus
@@ -7,6 +7,7 @@ import type {
import {
getPortsPanelOwnerKey,
portsPanelHostForOwnerKey,
portsPanelReachesSshHost,
type PortsPanelHost
} from './ports-panel-host'
@@ -148,3 +149,41 @@ describe('Ports panel host', () => {
})
})
})
describe('Ports tab visibility', () => {
const managed = {
id: 'env-managed',
orcadDeployment: { sshTargetId: 'box', sshTargetGeneration: 1 }
} as const
const environments = [{ id: 'env-paired' }, managed]
it.each<[string, PortsPanelHost, boolean]>([
['a direct SSH workspace', { kind: 'direct-ssh', connectionId: 'box' }, true],
['an SSH target behind a paired server', serverSsh('env-paired', 't'), true],
[
'a workspace on a managed server reached over SSH',
{ kind: 'endpoint', target: { kind: 'environment', environmentId: 'env-managed' } },
true
],
[
'a workspace on a paired server that is not an SSH host',
{ kind: 'endpoint', target: { kind: 'environment', environmentId: 'env-paired' } },
false
],
['a local workspace', { kind: 'endpoint', target: { kind: 'local' } }, false],
['an unknown owner', { kind: 'unknown' }, false]
])('%s', (_name, host, visible) => {
expect(portsPanelReachesSshHost(host, environments)).toBe(visible)
})
it('follows the workspace owner, not the repo connection', () => {
const state: WorktreeOperationRouteState = {
repos: [{ id: 'repo-1' }],
worktreesByRepo: { 'repo-1': [worktree('runtime:env-managed', 'env-managed')] },
runtimeEnvironments: [{ id: 'env-managed' }],
runtimeEnvironmentCatalogHydrated: true
}
const host = portsPanelHostForOwnerKey(getPortsPanelOwnerKey(state, WORKTREE_ID))
expect(portsPanelReachesSshHost(host, environments)).toBe(true)
})
})
@@ -56,3 +56,27 @@ export function portsPanelHostForOwnerKey(key: HostAuthorityKey | null): PortsPa
}
return { kind: 'host-scoped', route, executionHostId: authority.at }
}
/** The Ports tab shows for a workspace on an SSH host, dialed directly or through its managed server. */
export function portsPanelReachesSshHost(
host: PortsPanelHost,
environments: readonly { id: string; orcadDeployment?: unknown }[]
): boolean {
switch (host.kind) {
case 'direct-ssh':
case 'host-scoped':
return true
case 'endpoint': {
const { target } = host
return (
target.kind === 'environment' &&
environments.some(
(environment) =>
environment.id === target.environmentId && environment.orcadDeployment !== undefined
)
)
}
case 'unknown':
return false
}
}
@@ -37,12 +37,12 @@ const items: ActivityBarItem[] = [
]
describe('getVisibleRightSidebarActivityItems', () => {
it('shows ports only for SSH repos', () => {
it('shows ports only for workspaces on an SSH host', () => {
expect(
getVisibleRightSidebarActivityItems(items, {
isFolder: false,
isFolderWorkspace: false,
isSshRepo: false
hasSshHost: false
}).map((item) => item.id)
).toEqual(['explorer', 'source-control', 'plugin:orca-samples.my-plugin/dashboard'])
@@ -50,7 +50,7 @@ describe('getVisibleRightSidebarActivityItems', () => {
getVisibleRightSidebarActivityItems(items, {
isFolder: false,
isFolderWorkspace: false,
isSshRepo: true
hasSshHost: true
}).map((item) => item.id)
).toEqual(['explorer', 'source-control', 'ports', 'plugin:orca-samples.my-plugin/dashboard'])
})
@@ -60,7 +60,7 @@ describe('getVisibleRightSidebarActivityItems', () => {
getVisibleRightSidebarActivityItems(items, {
isFolder: true,
isFolderWorkspace: true,
isSshRepo: true
hasSshHost: true
}).map((item) => item.id)
).toEqual([
'explorer',
@@ -74,7 +74,7 @@ describe('getVisibleRightSidebarActivityItems', () => {
getVisibleRightSidebarActivityItems(items, {
isFolder: true,
isFolderWorkspace: false,
isSshRepo: true
hasSshHost: true
}).map((item) => item.id)
).toEqual(['explorer', 'ports', 'plugin:orca-samples.my-plugin/dashboard'])
})
@@ -3,17 +3,18 @@ import type { ActivityBarItem } from './activity-bar-buttons'
type RightSidebarActivityVisibilityState = {
isFolder: boolean
isFolderWorkspace: boolean
isSshRepo: boolean
/** The workspace runs on an SSH host, reached directly or through its managed server. */
hasSshHost: boolean
}
export function getVisibleRightSidebarActivityItems(
items: ActivityBarItem[],
{ isFolder, isFolderWorkspace, isSshRepo }: RightSidebarActivityVisibilityState
{ isFolder, isFolderWorkspace, hasSshHost }: RightSidebarActivityVisibilityState
): ActivityBarItem[] {
return items.filter(
(item) =>
(!item.gitOnly || !isFolder) &&
(!item.folderOnly || isFolderWorkspace) &&
(!item.sshOnly || isSshRepo)
(!item.sshOnly || hasSshHost)
)
}
@@ -6,6 +6,11 @@ import { isFolderRepo } from '../../../../shared/repo-kind'
import { parseWorkspaceKey } from '../../../../shared/workspace-scope'
import { getVisibleRightSidebarActivityItems } from './right-sidebar-activity-visibility'
import { getPluginPanelActivityItems } from './plugin-panel-activity-items'
import {
getPortsPanelOwnerKey,
portsPanelHostForOwnerKey,
portsPanelReachesSshHost
} from './ports-panel-host'
import {
collectInstalledPluginTabKeys,
usePluginPanels,
@@ -44,7 +49,13 @@ export function useRightSidebarActivityItems({
const activeWorkspaceScope = parseWorkspaceKey(activeWorktreeId ?? '')
const isFolderWorkspace = activeWorkspaceScope?.type === 'folder'
const isFolder = isFolderWorkspace || (activeRepo ? isFolderRepo(activeRepo) : false)
const isSshRepo = Boolean(activeRepo?.connectionId)
// Why the owner: a managed server's workspace names its server, not an SSH connection.
const hasSshHost = useAppStore((s) =>
portsPanelReachesSshHost(
portsPanelHostForOwnerKey(getPortsPanelOwnerKey(s, activeWorktreeId)),
s.runtimeEnvironments
)
)
const pluginSystemEnabled = useAppStore((s) => s.settings?.pluginSystemEnabled === true)
const pluginPanels = usePluginPanels()
const visiblePluginPanels = useMemo(
@@ -130,9 +141,9 @@ export function useRightSidebarActivityItems({
getVisibleRightSidebarActivityItems(activityItems, {
isFolder,
isFolderWorkspace,
isSshRepo
hasSshHost
}),
[activityItems, isFolder, isFolderWorkspace, isSshRepo]
[activityItems, isFolder, isFolderWorkspace, hasSshHost]
)
const activeFolderWorkspaceKey = isFolderWorkspace ? (activeWorktreeId ?? null) : null
+8 -3
View File
@@ -61,8 +61,6 @@ test('Quick Open finds a file in a monorepo-sized managed workspace', async (//
for (const acrossReconnect of [false, true]) {
test(`a managed host reports its listener, and an SSH forward reaches it${acrossReconnect ? ' across a reconnect' : ''}`, async (// oxlint-disable-next-line no-empty-pattern -- Owns its app launch through a restart session.
{}, testInfo) => {
// Why fixme: saved forwards are re-established only by the relay session's connect callbacks.
test.fixme(acrossReconnect, 'A managed host reconnect never calls restorePortForwards')
test.setTimeout(10 * 60_000)
const remotePort = 7860
const marker = `ORCA_FORWARD_${Date.now()}`
@@ -97,11 +95,18 @@ for (const acrossReconnect of [false, true]) {
}
}
await expect.poll(fetchMarker, { timeout: 30_000 }).toBe(marker)
let forwardId = forward.id
if (acrossReconnect) {
await reconnect(page, seeded.targetId)
await expect.poll(fetchMarker, { timeout: 60_000 }).toBe(marker)
// The restored forward is a new entry for the same saved port.
const restored = await page.evaluate(
(targetId) => window.api.ssh.listPortForwards({ targetId }),
seeded.targetId
)
forwardId = restored.find((entry) => entry.localPort === localPort)?.id ?? forwardId
}
await page.evaluate((id) => window.api.ssh.removePortForward({ id }), forward.id)
await page.evaluate((id) => window.api.ssh.removePortForward({ id }), forwardId)
await expect.poll(fetchMarker, { timeout: 30_000 }).not.toBe(marker)
}).finally(() => reservation.release())
})