Improve SSH config target handling (#3899)

docs/ssh-config-target-compatibility.md

@@ -0,0 +1,61 @@
+# SSH Config Target Compatibility
+
+## Problem
+
+- `src/main/ssh/ssh-connection-utils.ts:108` resolves OpenSSH config with `ssh -G`, but then prefers the persisted `target.host` over `resolved.hostname`.
+- `src/main/ssh/ssh-connection-utils.ts:109` treats persisted port `22` as an explicit override, so a config-host target can ignore a resolved non-default `Port`.
+- `src/main/ssh/ssh-config-parser.ts:210` imports config aliases with `host` set to the alias when the concrete `Host` block lacks an inline `HostName`; later `ssh -G` may know the real host, but the connection path ignores it.
+- `src/renderer/src/components/settings/SshPane.tsx:63` requires both host and username, even though OpenSSH config aliases can resolve the user and users commonly paste `user@host:port` targets.
+- `src/main/ssh/ssh-config-parser.ts:330` parses `ForwardAgent`, but `src/main/ssh/ssh-connection-utils.ts:112` did not pass it into ssh2, so remote git commands that rely on the local agent could fail.
+
+## Goal
+
+Make Orca behave like a mature SSH client for common config-host flows: aliases imported from `~/.ssh/config`, aliases with inherited `HostName`/`Port`, and pasted SSH targets should connect without users re-entering information that OpenSSH can resolve.
+
+## Non-goals
+
+- Do not add persistent secret storage for SSH passwords or key passphrases.
+- Do not redesign the whole SSH settings page.
+- Do not change relay deployment or remote PTY lease semantics.
+- Do not add a known-host trust UI in this patch.
+
+## Design
+
+1. Preserve explicit target overrides while letting config aliases use resolved values.
+   - In `buildConnectConfig`, prefer `resolved.hostname` only when the persisted host is blank, the same as `configHost`, or the same as the label.
+   - Prefer `resolved.port` when the target is a config-host target still on the default `22`; keep non-default target ports as explicit overrides.
+   - Continue using target username first, then resolved user.
+
+2. Honor resolved agent forwarding where ssh2 can support it.
+   - Set `agentForward` only when resolved config requested forwarding and an agent is actually configured.
+   - Leave system-SSH transport unchanged because it already delegates to OpenSSH config.
+
+3. Normalize settings form drafts before save.
+   - Accept `ssh://user@host:port`, `user@host:port`, and plain aliases in the Host field.
+   - Auto-fill username and port from pasted inputs only when the dedicated fields are still empty/default.
+   - Allow username to be omitted; `ssh -G` can provide it during connect.
+
+4. Keep UI changes small.
+   - Rename copy only where needed to avoid implying username is mandatory.
+   - Render username-less targets without a leading `@`.
+   - Do not introduce new colors, typography, or layout patterns.
+
+5. Cover behavior with focused tests.
+   - Add connection-config tests for config-host resolved hostname/port precedence.
+   - Add connection-config tests for `ForwardAgent yes`.
+   - Add renderer utility tests for pasted SSH target normalization.
+
+## Edge Cases
+
+- Explicit non-default port in Orca still wins over `ssh -G`.
+- Empty or unparsable host input remains invalid.
+- IPv6 bracket syntax is accepted for `ssh://` URLs and preserved conservatively for scp-like inputs.
+- Plain config aliases remain valid even without a username.
+
+## Rollout
+
+1. Add the renderer draft-normalization helper and tests.
+2. Wire the SSH settings form save path and labels to the helper.
+3. Update `buildConnectConfig` precedence/agent-forwarding and tests.
+4. Clean up username-less target display.
+5. Run focused tests and typecheck/lint where feasible.

src/main/ssh/ssh-connection-utils.test.ts

@@ -343,6 +343,33 @@ describe('buildConnectConfig', () => {
     expect(config.username).toBe('admin')
   })
 
+  it('uses ssh -G HostName when a config-host target still points at its alias', () => {
+    const config = buildConnectConfig(
+      makeTarget({ label: 'workbox', configHost: 'workbox', host: 'workbox' }),
+      makeResolved({ hostname: 'workbox.internal' })
+    )
+
+    expect(config.host).toBe('workbox.internal')
+  })
+
+  it('uses ssh -G Port when a config-host target still has the default port', () => {
+    const config = buildConnectConfig(
+      makeTarget({ configHost: 'workbox', host: 'workbox', port: 22 }),
+      makeResolved({ port: 2202 })
+    )
+
+    expect(config.port).toBe(2202)
+  })
+
+  it('keeps explicit non-default target ports ahead of ssh -G Port', () => {
+    const config = buildConnectConfig(
+      makeTarget({ configHost: 'workbox', host: 'workbox', port: 2022 }),
+      makeResolved({ port: 2202 })
+    )
+
+    expect(config.port).toBe(2022)
+  })
+
   it('sets readyTimeout to CONNECT_TIMEOUT_MS', () => {
     const config = buildConnectConfig(makeTarget(), null)
     expect(config.readyTimeout).toBe(30_000)
@@ -358,6 +385,26 @@ describe('buildConnectConfig', () => {
     expect(config.agent).toBe('/tmp/agent.sock')
   })
 
+  it('enables agent forwarding when OpenSSH config requests it and an agent is available', () => {
+    const config = buildConnectConfig(makeTarget(), makeResolved({ forwardAgent: true }))
+
+    expect(config.agent).toBe('/tmp/agent.sock')
+    expect(config.agentForward).toBe(true)
+  })
+
+  it('does not enable agent forwarding without a usable agent', () => {
+    const platformSpy = vi.spyOn(process, 'platform', 'get').mockReturnValue('linux')
+    delete process.env.SSH_AUTH_SOCK
+
+    try {
+      const config = buildConnectConfig(makeTarget(), makeResolved({ forwardAgent: true }))
+      expect(config.agent).toBeUndefined()
+      expect(config.agentForward).toBeUndefined()
+    } finally {
+      platformSpy.mockRestore()
+    }
+  })
+
   it('uses configured IdentityAgent before SSH_AUTH_SOCK', () => {
     const config = buildConnectConfig(
       makeTarget(),

src/main/ssh/ssh-connection-utils.ts

@@ -105,8 +105,8 @@ export function buildConnectConfig(
   resolved: SshResolvedConfig | null,
   options: BuildConnectConfigOptions = {}
 ): ConnectConfig {
-  const effectiveHost = target.host || resolved?.hostname || target.label
-  const effectivePort = target.port || resolved?.port || 22
+  const effectiveHost = resolveEffectiveHost(target, resolved)
+  const effectivePort = resolveEffectivePort(target, resolved)
   const effectiveUser = target.username || resolved?.user || ''
 
   const config: Record<string, unknown> = {
@@ -125,6 +125,10 @@ export function buildConnectConfig(
     config.agent = agent
   }
 
+  if (agent && resolved?.forwardAgent) {
+    config.agentForward = true
+  }
+
   const key =
     (options.includePrivateKey ?? !agent)
       ? resolvePrivateKey(target, resolved)
@@ -136,6 +140,30 @@ export function buildConnectConfig(
   return config as ConnectConfig
 }
 
+function resolveEffectiveHost(target: SshTarget, resolved: SshResolvedConfig | null): string {
+  if (shouldUseResolvedEndpoint(target, resolved)) {
+    return resolved!.hostname
+  }
+  return target.host || resolved?.hostname || target.label
+}
+
+function resolveEffectivePort(target: SshTarget, resolved: SshResolvedConfig | null): number {
+  // Why: imported config aliases store 22 as the schema default even when an
+  // included/wildcard OpenSSH rule later resolves a different effective Port.
+  if (target.configHost && target.port === 22 && resolved?.port) {
+    return resolved.port
+  }
+  return target.port || resolved?.port || 22
+}
+
+function shouldUseResolvedEndpoint(target: SshTarget, resolved: SshResolvedConfig | null): boolean {
+  if (!target.configHost || !resolved?.hostname) {
+    return false
+  }
+  const host = target.host.trim()
+  return host === '' || host === target.configHost || host === target.label
+}
+
 // Why: ProxyJump and jumpHost are syntactic sugar for ProxyCommand.
 // OpenSSH internally converts `ProxyJump bastion` to
 // `ProxyCommand ssh -W %h:%p bastion`. We do the same so that ssh2

src/renderer/src/components/settings/SshPane.tsx

@@ -4,7 +4,6 @@ import { Plus, Upload } from 'lucide-react'
 import {
   DEFAULT_SSH_RELAY_GRACE_PERIOD_SECONDS,
   MAX_SSH_RELAY_GRACE_PERIOD_SECONDS,
-  MIN_SSH_RELAY_GRACE_PERIOD_SECONDS,
   type SshTarget
 } from '../../../../shared/ssh-types'
 import { SSH_TERMINATE_RECONNECT_REQUIRED } from '../../../../shared/constants'
@@ -15,6 +14,11 @@ import { removeSshTargetWithBestEffortCleanup } from './ssh-target-remove'
 import { SshTargetCard } from './SshTargetCard'
 import { SshTargetDestructiveActions } from './SshTargetDestructiveActions'
 import { SshTargetForm, EMPTY_FORM, type EditingTarget } from './SshTargetForm'
+import {
+  getSshTargetDraftConnectionFields,
+  isRelayGracePeriodValid,
+  parseRelayGracePeriodSeconds
+} from './ssh-target-draft'
 export { SSH_PANE_SEARCH_ENTRIES } from './ssh-search'
 
 type SshPaneProps = Record<string, never>
@@ -60,26 +64,19 @@ export function SshPane(_props: SshPaneProps): React.JSX.Element {
   }, [loadTargets])
 
   const handleSave = async (): Promise<void> => {
-    if (!form.host.trim() || !form.username.trim()) {
-      toast.error('Host and username are required')
+    const { host, configHost, username, port } = getSshTargetDraftConnectionFields(form)
+    if (!host) {
+      toast.error('Host or SSH config alias is required')
       return
     }
 
-    const port = parseInt(form.port, 10)
     if (isNaN(port) || port < 1 || port > 65535) {
       toast.error('Port must be between 1 and 65535')
       return
     }
 
-    const graceSeconds = form.relayKeepAliveUntilReset
-      ? 0
-      : parseInt(form.relayGracePeriodSeconds, 10)
-    if (
-      !form.relayKeepAliveUntilReset &&
-      (isNaN(graceSeconds) ||
-        graceSeconds < MIN_SSH_RELAY_GRACE_PERIOD_SECONDS ||
-        graceSeconds > MAX_SSH_RELAY_GRACE_PERIOD_SECONDS)
-    ) {
+    const graceSeconds = parseRelayGracePeriodSeconds(form)
+    if (!isRelayGracePeriodValid(form, graceSeconds)) {
       toast.error(
         `Relay grace period must be between 60 and ${MAX_SSH_RELAY_GRACE_PERIOD_SECONDS} seconds, or choose keep alive until reset`
       )
@@ -87,11 +84,11 @@ export function SshPane(_props: SshPaneProps): React.JSX.Element {
     }
 
     const target = {
-      label: form.label.trim() || `${form.username}@${form.host}`,
-      configHost: form.configHost.trim() || form.host.trim(),
-      host: form.host.trim(),
+      label: form.label.trim() || (username ? `${username}@${host}` : configHost),
+      configHost,
+      host,
       port,
-      username: form.username.trim(),
+      username,
       relayGracePeriodSeconds: graceSeconds,
       ...(form.identityFile.trim() ? { identityFile: form.identityFile.trim() } : {}),
       ...(form.proxyCommand.trim() ? { proxyCommand: form.proxyCommand.trim() } : {}),

src/renderer/src/components/settings/SshTargetCard.tsx

@@ -86,6 +86,9 @@ export function SshTargetCard({
   const resetInFlight = actionInFlight === 'reset' || busyAction === 'reset'
   const removeInFlight = busyAction === 'remove'
   const mountedRef = useRef(true)
+  const endpoint = target.username
+    ? `${target.username}@${target.host}:${target.port}`
+    : `${target.host}:${target.port}`
 
   const handleCardRef = useCallback((node: HTMLDivElement | null): void => {
     // Why: SSH target actions can resolve after the card is removed; the root
@@ -238,7 +241,7 @@ export function SshTargetCard({
           <span className="text-[11px] text-muted-foreground">{STATUS_LABELS[status]}</span>
         </div>
         <p className="truncate text-xs text-muted-foreground">
-          {target.username}@{target.host}:{target.port}
+          {endpoint}
           {target.identityFile ? ` \u2022 ${target.identityFile}` : ''}
         </p>
         {state?.error ? (

src/renderer/src/components/settings/SshTargetForm.tsx

@@ -7,32 +7,8 @@ import {
 import { Button } from '../ui/button'
 import { Input } from '../ui/input'
 import { Label } from '../ui/label'
-
-export type EditingTarget = {
-  label: string
-  configHost: string
-  host: string
-  port: string
-  username: string
-  identityFile: string
-  proxyCommand: string
-  jumpHost: string
-  relayGracePeriodSeconds: string
-  relayKeepAliveUntilReset: boolean
-}
-
-export const EMPTY_FORM: EditingTarget = {
-  label: '',
-  configHost: '',
-  host: '',
-  port: '22',
-  username: '',
-  identityFile: '',
-  proxyCommand: '',
-  jumpHost: '',
-  relayGracePeriodSeconds: String(DEFAULT_SSH_RELAY_GRACE_PERIOD_SECONDS),
-  relayKeepAliveUntilReset: false
-}
+import { applyParsedSshHostInput, type EditingTarget } from './ssh-target-draft'
+export { EMPTY_FORM, type EditingTarget } from './ssh-target-draft'
 
 type SshTargetFormProps = {
   editingId: string | null
@@ -69,15 +45,16 @@ export function SshTargetForm({
           />
         </div>
         <div className="space-y-1.5">
-          <Label>Host *</Label>
+          <Label>Host or alias *</Label>
           <Input
             value={form.host}
             onChange={(e) => onFormChange((f) => ({ ...f, host: e.target.value }))}
-            placeholder="192.168.1.100 or server.example.com"
+            onBlur={() => onFormChange(applyParsedSshHostInput)}
+            placeholder="server, deploy@server:2222, ssh://server"
           />
         </div>
         <div className="space-y-1.5">
-          <Label>Username *</Label>
+          <Label>Username</Label>
           <Input
             value={form.username}
             onChange={(e) => onFormChange((f) => ({ ...f, username: e.target.value }))}

src/renderer/src/components/settings/ssh-target-draft.test.ts

@@ -0,0 +1,86 @@
+import { describe, expect, it } from 'vitest'
+import {
+  EMPTY_FORM,
+  applyParsedSshHostInput,
+  getSshTargetDraftConnectionFields,
+  parseSshHostInput
+} from './ssh-target-draft'
+
+describe('parseSshHostInput', () => {
+  it('parses scp-style user, host, and port input', () => {
+    expect(parseSshHostInput('deploy@example.com:2202')).toEqual({
+      host: 'example.com',
+      username: 'deploy',
+      port: 2202,
+      configHost: 'example.com'
+    })
+  })
+
+  it('parses ssh URLs', () => {
+    expect(parseSshHostInput('ssh://deploy@example.com:2202/srv/app')).toEqual({
+      host: 'example.com',
+      username: 'deploy',
+      port: 2202,
+      configHost: 'example.com'
+    })
+  })
+
+  it('keeps plain OpenSSH config aliases valid without a username', () => {
+    expect(parseSshHostInput('prod-box')).toEqual({
+      host: 'prod-box',
+      username: undefined,
+      port: undefined,
+      configHost: 'prod-box'
+    })
+  })
+})
+
+describe('applyParsedSshHostInput', () => {
+  it('fills empty username and default port from pasted input', () => {
+    expect(
+      applyParsedSshHostInput({ ...EMPTY_FORM, host: 'deploy@example.com:2202' })
+    ).toMatchObject({
+      host: 'example.com',
+      configHost: 'example.com',
+      username: 'deploy',
+      port: '2202'
+    })
+  })
+
+  it('does not overwrite explicit username or non-default port', () => {
+    expect(
+      applyParsedSshHostInput({
+        ...EMPTY_FORM,
+        host: 'deploy@example.com:2202',
+        username: 'root',
+        port: '2022'
+      })
+    ).toMatchObject({
+      host: 'example.com',
+      username: 'root',
+      port: '2022'
+    })
+  })
+})
+
+describe('getSshTargetDraftConnectionFields', () => {
+  it('uses pasted user and port when the dedicated fields are still default', () => {
+    expect(
+      getSshTargetDraftConnectionFields({ ...EMPTY_FORM, host: 'deploy@example.com:2202' })
+    ).toEqual({
+      host: 'example.com',
+      configHost: 'example.com',
+      username: 'deploy',
+      port: 2202
+    })
+  })
+
+  it('allows config aliases without a username', () => {
+    expect(getSshTargetDraftConnectionFields({ ...EMPTY_FORM, host: 'prod-box' })).toEqual({
+      host: 'prod-box',
+      configHost: 'prod-box',
+      username: '',
+      port: 22
+    })
+  })
+})

src/renderer/src/components/settings/ssh-target-draft.ts

@@ -0,0 +1,173 @@
+import {
+  DEFAULT_SSH_RELAY_GRACE_PERIOD_SECONDS,
+  MAX_SSH_RELAY_GRACE_PERIOD_SECONDS,
+  MIN_SSH_RELAY_GRACE_PERIOD_SECONDS
+} from '../../../../shared/ssh-types'
+
+export type EditingTarget = {
+  label: string
+  configHost: string
+  host: string
+  port: string
+  username: string
+  identityFile: string
+  proxyCommand: string
+  jumpHost: string
+  relayGracePeriodSeconds: string
+  relayKeepAliveUntilReset: boolean
+}
+
+export const EMPTY_FORM: EditingTarget = {
+  label: '',
+  configHost: '',
+  host: '',
+  port: '22',
+  username: '',
+  identityFile: '',
+  proxyCommand: '',
+  jumpHost: '',
+  relayGracePeriodSeconds: String(DEFAULT_SSH_RELAY_GRACE_PERIOD_SECONDS),
+  relayKeepAliveUntilReset: false
+}
+
+export type ParsedSshHostInput = {
+  host: string
+  username?: string
+  port?: number
+  configHost: string
+}
+
+export function parseSshHostInput(rawInput: string): ParsedSshHostInput | null {
+  const input = rawInput.trim()
+  if (!input) {
+    return null
+  }
+
+  if (/^ssh:\/\//i.test(input)) {
+    return parseSshUrl(input)
+  }
+
+  const atIndex = input.lastIndexOf('@')
+  const username = atIndex > 0 ? input.slice(0, atIndex).trim() : undefined
+  const hostPort = atIndex > 0 ? input.slice(atIndex + 1).trim() : input
+  const parsed = parseHostAndOptionalPort(hostPort)
+  if (!parsed.host) {
+    return null
+  }
+
+  return {
+    host: parsed.host,
+    username,
+    port: parsed.port,
+    configHost: parsed.host
+  }
+}
+
+export function applyParsedSshHostInput(draft: EditingTarget): EditingTarget {
+  const parsed = parseSshHostInput(draft.host)
+  if (!parsed) {
+    return draft
+  }
+
+  return {
+    ...draft,
+    host: parsed.host,
+    configHost: draft.configHost.trim() || parsed.configHost,
+    username: draft.username.trim() || parsed.username || '',
+    port:
+      parsed.port !== undefined && isDefaultPortDraft(draft.port) ? String(parsed.port) : draft.port
+  }
+}
+
+export function getSshTargetDraftConnectionFields(draft: EditingTarget): {
+  host: string
+  configHost: string
+  username: string
+  port: number
+} {
+  const parsed = parseSshHostInput(draft.host)
+  const host = parsed?.host ?? draft.host.trim()
+  const configHost = draft.configHost.trim() || parsed?.configHost || host
+  const username = draft.username.trim() || parsed?.username || ''
+  const parsedPort = parseInt(draft.port, 10)
+  const port =
+    parsed?.port !== undefined && isDefaultPortDraft(draft.port) ? parsed.port : parsedPort
+
+  return {
+    host,
+    configHost,
+    username,
+    port
+  }
+}
+
+export function parseRelayGracePeriodSeconds(draft: EditingTarget): number {
+  return draft.relayKeepAliveUntilReset ? 0 : parseInt(draft.relayGracePeriodSeconds, 10)
+}
+
+export function isRelayGracePeriodValid(draft: EditingTarget, graceSeconds: number): boolean {
+  return (
+    draft.relayKeepAliveUntilReset ||
+    (!isNaN(graceSeconds) &&
+      graceSeconds >= MIN_SSH_RELAY_GRACE_PERIOD_SECONDS &&
+      graceSeconds <= MAX_SSH_RELAY_GRACE_PERIOD_SECONDS)
+  )
+}
+
+function parseSshUrl(input: string): ParsedSshHostInput | null {
+  try {
+    const url = new URL(input)
+    if (url.protocol !== 'ssh:' || !url.hostname) {
+      return null
+    }
+    const port = url.port ? parseInt(url.port, 10) : undefined
+    if (port !== undefined && !isValidPort(port)) {
+      return null
+    }
+    return {
+      host: url.hostname,
+      username: url.username ? decodeURIComponent(url.username) : undefined,
+      port,
+      configHost: url.hostname
+    }
+  } catch {
+    return null
+  }
+}
+
+function parseHostAndOptionalPort(input: string): { host: string; port?: number } {
+  if (input.startsWith('[')) {
+    const closeIndex = input.indexOf(']')
+    if (closeIndex > 1) {
+      const host = input.slice(1, closeIndex)
+      const suffix = input.slice(closeIndex + 1)
+      if (suffix.startsWith(':')) {
+        const port = parsePort(suffix.slice(1))
+        return port === undefined ? { host } : { host, port }
+      }
+      return { host }
+    }
+  }
+
+  const portMatch = input.match(/^([^:]+):(\d{1,5})$/)
+  if (portMatch) {
+    const port = parsePort(portMatch[2])
+    return port === undefined ? { host: input } : { host: portMatch[1], port }
+  }
+
+  return { host: input }
+}
+
+function parsePort(value: string): number | undefined {
+  const port = parseInt(value, 10)
+  return isValidPort(port) ? port : undefined
+}
+
+function isValidPort(port: number): boolean {
+  return Number.isInteger(port) && port >= 1 && port <= 65535
+}
+
+function isDefaultPortDraft(value: string): boolean {
+  const trimmed = value.trim()
+  return trimmed === '' || trimmed === '22'
+}