fix: flag a config host whose machine is already registered under another name

searchSshConfigHosts decided "already in Orca" by comparing normalized alias
strings — a target's configHost and label — against the config entry's Host
pattern. Two names for one machine therefore both pass:

  manual   { label: 'AI Box', configHost: '172.16.203.131', host: '172.16.203.131' }
  picker   Host aimp / HostName 172.16.203.131

Nothing in either record is string-equal, so the picker lists `aimp` as new and
the user gets a second target for a box they already added. Both connect to the
same endpoint and attach to the same relay daemon, so the two entries list the
same remote sessions and operating either one drives the same underlying ones.

Add sshEndpointIdentity — host, port and user, normalized and NUL-separated —
and treat a candidate as already registered when either its alias or its
endpoint matches. Multiple pathspecs of identity, same as before, just no
longer name-only.

Only entries carrying HostName participate: without it the candidate host
degrades to the alias itself, and matching on that would re-run the alias check
while letting unrelated aliases collide. That also bounds what this fixes —
a Host block that resolves through DNS rather than HostName still yields two
targets, because neither the parser nor `ssh -G` resolves names.

alreadyInOrca is a display flag, not a gate, so a deliberate second target for
the same endpoint (different identity file, different jump host) still gets
added — it just says so first.

Fixes #15478

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
vam
2026-08-21 15:08:33 +08:00
co-authored by Claude Opus 5
parent 59892a2f13
commit 3c3a3c689c
4 changed files with 139 additions and 4 deletions
@@ -57,6 +57,58 @@ describe('SSH config host picker search', () => {
expect(last.hasMore).toBe(false)
})
it('flags a config host whose machine is already registered under another label', () => {
// Why: the same box gets added twice — once manually by IP, once from the config
// picker — and alias comparison cannot see that because the names differ.
const result = searchSshConfigHosts(
[{ host: 'aimp', hostname: '172.16.203.131', user: 'chanmuzi' }],
[
{
configHost: '172.16.203.131',
label: 'AI Box',
host: '172.16.203.131',
port: 22,
username: 'chanmuzi'
}
]
)
expect(result.hosts[0]).toMatchObject({ alias: 'aimp', alreadyInOrca: true })
expect(result.newHostCount).toBe(0)
})
it('leaves a different machine unflagged', () => {
const result = searchSshConfigHosts(
[{ host: 'aimp', hostname: '172.16.203.131', user: 'chanmuzi' }],
[
{
configHost: '10.0.0.9',
label: 'Other Box',
host: '10.0.0.9',
port: 22,
username: 'chanmuzi'
}
]
)
expect(result.hosts[0]).toMatchObject({ alias: 'aimp', alreadyInOrca: false })
expect(result.newHostCount).toBe(1)
})
it('does not collide config hosts that carry no HostName', () => {
// Why: without HostName the candidate host degrades to the alias itself, so the
// endpoint check must not start matching unrelated aliases against each other.
const result = searchSshConfigHosts(
[{ host: 'alpha' }, { host: 'beta' }],
[{ configHost: 'gamma', label: 'Gamma', host: 'gamma', port: 22, username: '' }]
)
expect(result.hosts.map((host) => [host.alias, host.alreadyInOrca])).toEqual([
['alpha', false],
['beta', false]
])
})
it('counts de-duplicated and existing aliases while matching effective display fields', () => {
const result = searchSshConfigHosts(
[
+19 -4
View File
@@ -5,7 +5,7 @@ import type {
SshTarget
} from '../../shared/ssh-types'
import { SSH_CONFIG_HOST_RESULT_LIMIT } from '../../shared/ssh-types'
import { normalizeSshConfigAlias } from '../../shared/ssh-config-alias'
import { normalizeSshConfigAlias, sshEndpointIdentity } from '../../shared/ssh-config-alias'
import { loadUserSshConfig, type SshConfigHost } from './ssh-config-parser'
import { resolveWithSshG, type SshResolvedConfig } from './ssh-g-config-resolution'
@@ -27,7 +27,10 @@ function getUserSshConfigHosts(refresh: boolean): SshConfigHost[] {
export function searchSshConfigHosts(
hosts: SshConfigHost[],
existingTargets: readonly Pick<SshTarget, 'configHost' | 'label'>[],
// Why partial: callers that only know the names still type-check, and an entry with
// no host yields an empty identity rather than a false match.
existingTargets: readonly (Pick<SshTarget, 'configHost' | 'label'> &
Partial<Pick<SshTarget, 'host' | 'port' | 'username'>>)[],
query = '',
suppressedAliases: readonly string[] = []
): SshConfigHostListResult {
@@ -36,6 +39,9 @@ export function searchSshConfigHosts(
[target.configHost, target.label].map(normalizeSshConfigAlias).filter(Boolean)
)
)
// Why: the same machine can be registered twice under different names — once by IP,
// once from this picker — and comparing names cannot see that.
const existingEndpoints = new Set(existingTargets.map(sshEndpointIdentity).filter(Boolean))
const normalizedQuery = query.trim().toLowerCase()
const suppressedAliasSet = new Set(suppressedAliases.map(normalizeSshConfigAlias))
const summaries: SshConfigHostSummary[] = []
@@ -50,7 +56,13 @@ export function searchSshConfigHosts(
continue
}
seenAliases.add(normalizedAlias)
const alreadyInOrca = existingAliases.has(normalizedAlias)
// Why `entry.hostname` only: without HostName the candidate host degrades to the
// alias itself, and matching on that would just re-run the alias check.
const endpoint = entry.hostname
? sshEndpointIdentity({ host: entry.hostname, port: entry.port, username: entry.user })
: ''
const alreadyInOrca =
existingAliases.has(normalizedAlias) || (endpoint !== '' && existingEndpoints.has(endpoint))
// Why: tombstones only block passive bulk import — the picker still lists the
// Host so deleting one Orca target never looks like "~/.ssh/config is empty".
const previouslyRemoved = !alreadyInOrca && suppressedAliasSet.has(normalizedAlias)
@@ -76,7 +88,10 @@ export function searchSshConfigHosts(
}
export function listUserSshConfigHostSummaries(
existingTargets: readonly Pick<SshTarget, 'configHost' | 'label'>[],
// Why partial: callers that only know the names still type-check, and an entry with
// no host yields an empty identity rather than a false match.
existingTargets: readonly (Pick<SshTarget, 'configHost' | 'label'> &
Partial<Pick<SshTarget, 'host' | 'port' | 'username'>>)[],
query?: string,
suppressedAliases?: readonly string[],
options?: { refresh?: boolean }
+53
View File
@@ -0,0 +1,53 @@
import { describe, expect, it } from 'vitest'
import { sshEndpointIdentity } from './ssh-config-alias'
describe('sshEndpointIdentity', () => {
it('treats the same machine reached under different names as one endpoint', () => {
const manual = sshEndpointIdentity({
host: '172.16.203.131',
port: 22,
username: 'chanmuzi'
})
const fromConfig = sshEndpointIdentity({
host: '172.16.203.131',
port: 22,
username: 'chanmuzi'
})
expect(manual).toBe(fromConfig)
})
it('defaults a missing port to 22 so an explicit 22 matches an omitted one', () => {
expect(sshEndpointIdentity({ host: 'box', username: 'me' })).toBe(
sshEndpointIdentity({ host: 'box', port: 22, username: 'me' })
)
})
it('separates host, port and user so no concatenation can alias them', () => {
// Why: 'a' + '1:2' and 'a:1' + '2' must not collapse into the same key.
expect(sshEndpointIdentity({ host: 'a', port: 12, username: '' })).not.toBe(
sshEndpointIdentity({ host: 'a', port: 1, username: '2' })
)
})
it('folds host and user case but keeps a different user distinct', () => {
expect(sshEndpointIdentity({ host: 'BOX', port: 22, username: 'Me' })).toBe(
sshEndpointIdentity({ host: 'box', port: 22, username: 'me' })
)
expect(sshEndpointIdentity({ host: 'box', port: 22, username: 'me' })).not.toBe(
sshEndpointIdentity({ host: 'box', port: 22, username: 'other' })
)
})
it('is empty for an unknown host so unresolved entries never collide', () => {
expect(sshEndpointIdentity({ host: '', port: 22, username: 'me' })).toBe('')
expect(sshEndpointIdentity({ host: ' ', port: 22, username: 'me' })).toBe('')
expect(sshEndpointIdentity({ port: 22, username: 'me' })).toBe('')
})
it('ignores a non-finite port rather than keying on NaN', () => {
expect(sshEndpointIdentity({ host: 'box', port: Number.NaN, username: 'me' })).toBe(
sshEndpointIdentity({ host: 'box', port: 22, username: 'me' })
)
})
})
+15
View File
@@ -6,3 +6,18 @@
export function normalizeSshConfigAlias(alias: string | null | undefined): string {
return alias ? alias.trim().toLowerCase() : ''
}
/** The machine a target actually reaches: hostname, port and user, normalized.
* Empty when the host is unknown, so an unresolved alias never collides. */
export function sshEndpointIdentity(target: {
host?: string | null
port?: number | null
username?: string | null
}): string {
const host = target.host?.trim().toLowerCase() ?? ''
if (!host) {
return ''
}
const port = typeof target.port === 'number' && Number.isFinite(target.port) ? target.port : 22
return `${host}\u0000${port}\u0000${target.username?.trim().toLowerCase() ?? ''}`
}