Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions src/main/agent-hooks/remote-hook-service-installers.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -726,6 +726,20 @@ describe('remote hook service installers', () => {
expect(byAgent.get('copilot')).toBe('installed')
})

it('keeps installer result order and isolates per-agent failures under concurrency', async () => {
// Malformed Factory settings force droid into its error path while every
// other agent must still install.
const { sftp } = createFakeSftp({ '/home/dev/.factory/settings.json': '{"hooks": }' })

const results = await installRemoteManagedAgentHooks(sftp, '/home/dev')

expect(results.map((r) => r.agent)).toEqual([...REMOTE_MANAGED_HOOK_INSTALLER_AGENTS])
const byAgent = new Map(results.map((r) => [r.agent, r.state]))
expect(byAgent.get('droid')).toBe('error')
expect(byAgent.get('claude')).toBe('installed')
expect(byAgent.get('copilot')).toBe('installed')
})

it('installs remote Droid hooks into Factory settings.json (issue #7253)', async () => {
const { sftp, fs } = createFakeSftp()

Expand Down
59 changes: 32 additions & 27 deletions src/main/agent-hooks/remote-managed-hook-installers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -76,32 +76,37 @@ export async function installRemoteManagedAgentHooks(
remoteHome: string,
options?: RemoteManagedHookInstallOptions
): Promise<AgentHookInstallStatus[]> {
const results: AgentHookInstallStatus[] = []
for (const [agent, install] of REMOTE_MANAGED_HOOK_INSTALLERS) {
try {
const result = await install(sftp, remoteHome, options)
results.push(result)
if (result.state === 'error') {
console.warn(
`[agent-hooks] Remote ${agent} managed hook install failed for ${result.configPath}: ${
result.detail ?? 'unknown error'
}`
)
// Why: concurrent is safe because installers touch disjoint per-agent paths
// and shared script-dir writes are atomic (tmp+rename). Serially, their
// SFTP round trips dominated SSH connect (~90s measured at 150ms RTT).
return Promise.all(
REMOTE_MANAGED_HOOK_INSTALLERS.map(
async ([agent, install]): Promise<AgentHookInstallStatus> => {
try {
const result = await install(sftp, remoteHome, options)
if (result.state === 'error') {
console.warn(
`[agent-hooks] Remote ${agent} managed hook install failed for ${result.configPath}: ${
result.detail ?? 'unknown error'
}`
)
}
return result
} catch (error) {
// Why: remote hook installation must not block SSH workspace startup.
// A broken agent config or transient SFTP failure should degrade status
// reporting only, while terminals/filesystem/git still come online.
const detail = error instanceof Error ? error.message : String(error)
console.warn(`[agent-hooks] Remote ${agent} managed hook install threw: ${detail}`)
return {
agent,
state: 'error',
configPath: remoteHome,
managedHooksPresent: false,
detail
}
}
}
} catch (error) {
// Why: remote hook installation must not block SSH workspace startup.
// A broken agent config or transient SFTP failure should degrade status
// reporting only, while terminals/filesystem/git still come online.
const detail = error instanceof Error ? error.message : String(error)
console.warn(`[agent-hooks] Remote ${agent} managed hook install threw: ${detail}`)
results.push({
agent,
state: 'error',
configPath: remoteHome,
managedHooksPresent: false,
detail
})
}
}
return results
)
)
}
173 changes: 172 additions & 1 deletion src/main/ipc/ssh.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,12 @@ vi.mock('../ssh/ssh-port-scanner', () => ({
}
}))

import { getSshConnectionManager, registerSshHandlers, resetSshHandlerStateForTests } from './ssh'
import {
eagerReconnectSshTargetsFromShutdown,
getSshConnectionManager,
registerSshHandlers,
resetSshHandlerStateForTests
} from './ssh'
import { SSH_RELAY_CONFIGURE_GRACE_TIME_METHOD, type SshTarget } from '../../shared/ssh-types'
import {
clearProviderPtyState,
Expand All @@ -227,6 +232,7 @@ describe('SSH IPC handlers', () => {
const mockStore = {
getRepos: () => [],
getSshRemotePtyLeases: vi.fn().mockReturnValue([]),
getWorkspaceSession: vi.fn().mockReturnValue({}),
markSshRemotePtyLease: vi.fn(),
markSshRemotePtyLeases: vi.fn(),
removeSshRemotePtyLeases: vi.fn()
Expand Down Expand Up @@ -301,6 +307,7 @@ describe('SSH IPC handlers', () => {
mockSshStore.lastRepoReadoptions = []
mockWindow.webContents.send.mockReset()
mockStore.getSshRemotePtyLeases.mockReset().mockReturnValue([])
mockStore.getWorkspaceSession.mockReset().mockReturnValue({})
mockStore.markSshRemotePtyLease.mockReset()
mockStore.markSshRemotePtyLeases.mockReset()
mockStore.removeSshRemotePtyLeases.mockReset()
Expand Down Expand Up @@ -498,6 +505,170 @@ describe('SSH IPC handlers', () => {
expect(mockConnectionManager.connect).toHaveBeenCalledWith(target)
})

it('eager startup reconnect connects shutdown targets, skipping passphrase and missing ones', async () => {
const eager: SshTarget = {
id: 'ssh-eager',
label: 'Eager',
host: 'eager.example.com',
port: 22,
username: 'deploy'
}
const passphrase: SshTarget = {
id: 'ssh-pass',
label: 'Pass',
host: 'pass.example.com',
port: 22,
username: 'deploy',
lastRequiredPassphrase: true
}
mockStore.getWorkspaceSession.mockReturnValue({
activeConnectionIdsAtShutdown: ['ssh-eager', 'ssh-pass', 'ssh-gone']
})
mockSshStore.getTarget.mockImplementation((id: string) =>
id === 'ssh-eager' ? eager : id === 'ssh-pass' ? passphrase : undefined
)
mockConnectionManager.connect.mockResolvedValue({})
mockConnectionManager.getState.mockReturnValue({
targetId: 'ssh-eager',
status: 'connected',
error: null,
reconnectAttempt: 0
})

await handlers.get('ssh:credentialListenerReady')!(null, {})
eagerReconnectSshTargetsFromShutdown()

await vi.waitFor(() => {
expect(mockConnectionManager.connect).toHaveBeenCalledWith(eager)
})
expect(mockConnectionManager.connect).toHaveBeenCalledTimes(1)
})

it('eager startup reconnect stays off until the renderer credential listener is ready', async () => {
const target: SshTarget = {
id: 'ssh-1',
label: 'Server',
host: 'example.com',
port: 22,
username: 'deploy'
}
mockStore.getWorkspaceSession.mockReturnValue({
activeConnectionIdsAtShutdown: ['ssh-1']
})
mockSshStore.getTarget.mockReturnValue(target)
mockConnectionManager.connect.mockResolvedValue({})
mockConnectionManager.getState.mockReturnValue({
targetId: 'ssh-1',
status: 'connected',
error: null,
reconnectAttempt: 0
})

eagerReconnectSshTargetsFromShutdown({ listenerReadyTimeoutMs: 5_000 })
await new Promise((resolve) => setTimeout(resolve, 25))
expect(mockConnectionManager.connect).not.toHaveBeenCalled()

await handlers.get('ssh:credentialListenerReady')!(null, {})
await vi.waitFor(() => {
expect(mockConnectionManager.connect).toHaveBeenCalledWith(target)
})
})

it('eager startup reconnect is skipped entirely when the ready signal never arrives', async () => {
mockStore.getWorkspaceSession.mockReturnValue({
activeConnectionIdsAtShutdown: ['ssh-1']
})
mockSshStore.getTarget.mockReturnValue({
id: 'ssh-1',
label: 'Server',
host: 'example.com',
port: 22,
username: 'deploy'
} as SshTarget)
mockConnectionManager.connect.mockResolvedValue({})

eagerReconnectSshTargetsFromShutdown({ listenerReadyTimeoutMs: 30 })
await new Promise((resolve) => setTimeout(resolve, 80))
expect(mockConnectionManager.connect).not.toHaveBeenCalled()

// A late signal must not resurrect the abandoned eager pass.
await handlers.get('ssh:credentialListenerReady')!(null, {})
await new Promise((resolve) => setTimeout(resolve, 20))
expect(mockConnectionManager.connect).not.toHaveBeenCalled()
})

it('eager startup reconnect runs only once per app launch', async () => {
const target: SshTarget = {
id: 'ssh-1',
label: 'Server',
host: 'example.com',
port: 22,
username: 'deploy'
}
mockStore.getWorkspaceSession.mockReturnValue({
activeConnectionIdsAtShutdown: ['ssh-1']
})
mockSshStore.getTarget.mockReturnValue(target)
mockConnectionManager.connect.mockResolvedValue({})
mockConnectionManager.getState.mockReturnValue({
targetId: 'ssh-1',
status: 'connected',
error: null,
reconnectAttempt: 0
})

await handlers.get('ssh:credentialListenerReady')!(null, {})
eagerReconnectSshTargetsFromShutdown()
await vi.waitFor(() => {
expect(mockConnectionManager.connect).toHaveBeenCalledTimes(1)
})

// Second pass after the first completed: without the once-guard this
// would start a fresh doConnect (mock sessions never look 'ready').
eagerReconnectSshTargetsFromShutdown()
await new Promise((resolve) => setTimeout(resolve, 0))
expect(mockConnectionManager.connect).toHaveBeenCalledTimes(1)
})

it('renderer ssh:connect joins the in-flight eager reconnect instead of reconnecting', async () => {
const target: SshTarget = {
id: 'ssh-1',
label: 'Server',
host: 'example.com',
port: 22,
username: 'deploy'
}
mockStore.getWorkspaceSession.mockReturnValue({
activeConnectionIdsAtShutdown: ['ssh-1']
})
mockSshStore.getTarget.mockReturnValue(target)
let resolveConnect: ((value: unknown) => void) | undefined
mockConnectionManager.connect.mockImplementation(
() =>
new Promise((resolve) => {
resolveConnect = resolve
})
)
mockConnectionManager.getState.mockReturnValue({
targetId: 'ssh-1',
status: 'connected',
error: null,
reconnectAttempt: 0
})

await handlers.get('ssh:credentialListenerReady')!(null, {})
eagerReconnectSshTargetsFromShutdown()
await vi.waitFor(() => {
expect(resolveConnect).toBeDefined()
})

const rendererConnect = handlers.get('ssh:connect')!(null, { targetId: 'ssh-1' })
resolveConnect!({})
await rendererConnect

expect(mockConnectionManager.connect).toHaveBeenCalledTimes(1)
})

it('ssh:connect exposes the detected remote platform in public state', async () => {
const target: SshTarget = {
id: 'ssh-1',
Expand Down
Loading