fix(skills): resolve SSH transport collisions

This commit is contained in:
Brennan Benson
2026-08-05 18:50:13 -07:00
parent ae64fed822
commit 414183111d
3 changed files with 164 additions and 31 deletions
@@ -8,6 +8,7 @@ import {
} from '../../../../shared/execution-host'
import { getRepoIdFromWorktreeId } from '../../../../shared/worktree-id'
import type { RuntimeClientTarget } from '@/runtime/runtime-rpc-client'
import { resolveExactWorktreeRoute } from '@/lib/worktree-owner-route'
import {
getExplicitRuntimeEnvironmentIdForWorktree,
getExecutionHostIdForWorktree
@@ -77,17 +78,23 @@ export function selectNativeChatSkillStateInputs(state: AppState): NativeChatSki
}
}
function hasRuntimeSshTransportEvidence(
type SshTransportResolution =
| { kind: 'resolved'; environmentId: string | null }
| { kind: 'ambiguous' }
| { kind: 'missing' }
function resolveCatalogSshTransport(
state: NativeChatSkillStateInputs,
worktreeId: string,
hostId: ExecutionHostId
): boolean {
): SshTransportResolution {
const scope = parseWorkspaceKey(worktreeId)
if (scope?.type === 'folder') {
return false
return { kind: 'missing' }
}
const rawWorktreeId = scope?.type === 'worktree' ? scope.worktreeId : worktreeId
const repoIds = new Set([getRepoIdFromWorktreeId(rawWorktreeId)])
const environmentIds = new Set<string | null>()
const catalogs = [
...Object.values(state.worktreesByRepo ?? {}),
...Object.values(state.detectedWorktreesByRepo ?? {}).map((result) => result.worktrees)
@@ -98,23 +105,36 @@ function hasRuntimeSshTransportEvidence(
continue
}
repoIds.add(worktree.repoId)
if (worktree.runtimeOwnerEnvironmentId?.trim()) {
return true
const resolution = resolveExactWorktreeRoute(state, worktree)
if (resolution.kind === 'ambiguous') {
return resolution
}
if (resolution.kind === 'resolved' && resolution.route.executionHostId === hostId) {
environmentIds.add(resolution.route.runtimeEnvironmentId)
}
}
}
for (const repo of state.repos) {
const connectionId = repo.connectionId?.trim()
if (
repoIds.has(repo.id) &&
parseExecutionHostId(repo.executionHostId)?.kind === 'runtime' &&
connectionId &&
toSshExecutionHostId(connectionId) === hostId
) {
return true
if (environmentIds.size === 0) {
for (const repo of state.repos) {
if (!repoIds.has(repo.id)) {
continue
}
const executionHost = parseExecutionHostId(repo.executionHostId)
const connectionId = repo.connectionId?.trim()
const connectionHostId = connectionId ? toSshExecutionHostId(connectionId) : null
if (connectionHostId !== hostId && executionHost?.id !== hostId) {
continue
}
environmentIds.add(executionHost?.kind === 'runtime' ? executionHost.environmentId : null)
}
}
return false
if (environmentIds.size > 1) {
return { kind: 'ambiguous' }
}
const environmentId = environmentIds.values().next().value
return environmentIds.size === 1
? { kind: 'resolved', environmentId: environmentId ?? null }
: { kind: 'missing' }
}
export function resolveNativeChatSkillDiscoveryCwd(
@@ -151,14 +171,25 @@ export function resolveNativeChatSkillDiscoveryContext(
const hostId = getExecutionHostIdForWorktree(state, worktreeId)
const parsedHost = parseExecutionHostId(hostId)
if (parsedHost?.kind === 'ssh') {
const runtimeEnvironmentId = getExplicitRuntimeEnvironmentIdForWorktree(state, worktreeId)
const explicitRuntimeEnvironmentId = getExplicitRuntimeEnvironmentIdForWorktree(
state,
worktreeId
)
const catalogTransport = resolveCatalogSshTransport(state, worktreeId, parsedHost.id)
if (
catalogTransport.kind === 'ambiguous' ||
(explicitRuntimeEnvironmentId &&
catalogTransport.kind === 'resolved' &&
catalogTransport.environmentId !== explicitRuntimeEnvironmentId)
) {
return null
}
const runtimeEnvironmentId =
explicitRuntimeEnvironmentId ??
(catalogTransport.kind === 'resolved' ? catalogTransport.environmentId : null)
// Why: unresolved paired-runtime transport can collide with a local target
// id; failing closed prevents the identity-only RPC reaching the wrong host.
if (
!runtimeEnvironmentId &&
(isRuntimeOwnedSshTargetId(parsedHost.targetId) ||
hasRuntimeSshTransportEvidence(state, worktreeId, parsedHost.id))
) {
if (!runtimeEnvironmentId && isRuntimeOwnedSshTargetId(parsedHost.targetId)) {
return null
}
const connectionState = runtimeEnvironmentId
@@ -56,20 +56,21 @@ function stateForHost(hostId: string) {
}
}
function sshConnectionState(targetId: string, connectionGeneration = 3) {
return {
targetId,
status: 'connected',
error: null,
reconnectAttempt: 0,
connectionGeneration
}
}
function connectedSshState(connectionGeneration = 3) {
return {
...stateForHost('ssh:target-1'),
sshConnectionStates: new Map([
[
'target-1',
{
targetId: 'target-1',
status: 'connected',
error: null,
reconnectAttempt: 0,
connectionGeneration
}
]
['target-1', sshConnectionState('target-1', connectionGeneration)]
])
}
}
@@ -202,6 +203,72 @@ describe('useNativeChatSkills', () => {
)
})
it('routes an ordinary paired SSH pane through its owning runtime', async () => {
mocks.state = {
...stateForHost('ssh:private-target'),
sshConnectionStates: new Map([['private-target', sshConnectionState('private-target')]]),
sshStateByEnvironment: new Map([
[
'hub-a',
{
connectionStates: new Map([['private-target', sshConnectionState('private-target')]])
}
]
]),
worktreesByRepo: {
'repo-1': [
{
id: 'worktree-1',
repoId: 'repo-1',
path: '/repo/worktree',
hostId: 'ssh:private-target',
runtimeOwnerEnvironmentId: 'hub-a'
}
]
}
}
mocks.callRuntimeRpc.mockResolvedValue({ status: 'ok', result: DISCOVERY_RESULT })
render(<Probe enabled />)
await waitFor(() => expect(mocks.snapshots.at(-1)?.status).toBe('ready'))
expect(mocks.callRuntimeRpc).toHaveBeenCalledWith(
{ kind: 'environment', environmentId: 'hub-a' },
'skills.discoverForPane',
{ worktreeId: 'worktree-1', terminalTabId: 'tab-1' },
{ timeoutMs: 10_000 }
)
})
it('issues no RPC for an ambiguous direct and paired SSH pane', async () => {
mocks.state = {
...stateForHost('ssh:shared-target'),
activeWorkspaceExecutionHostId: 'ssh:shared-target',
sshConnectionStates: new Map([['shared-target', sshConnectionState('shared-target')]]),
worktreesByRepo: {
'repo-1': [
{
id: 'worktree-1',
repoId: 'repo-1',
path: '/repo/worktree',
hostId: 'ssh:shared-target'
},
{
id: 'worktree-1',
repoId: 'repo-1',
path: '/repo/worktree',
hostId: 'ssh:shared-target',
runtimeOwnerEnvironmentId: 'hub-a'
}
]
}
}
render(<Probe enabled />)
await waitFor(() => expect(mocks.snapshots.at(-1)?.status).toBe('idle'))
expect(mocks.callRuntimeRpc).not.toHaveBeenCalled()
})
it('bounds cached SSH reconnect generations per composer', async () => {
mocks.state = connectedSshState(1)
mocks.callRuntimeRpc.mockResolvedValue({ status: 'ok', result: DISCOVERY_RESULT })
@@ -478,6 +478,41 @@ describe('resolveNativeChatSkillDiscoveryContext for SSH panes', () => {
expect(context).toBeNull()
})
it('fails closed when direct and paired panes share an SSH target id', () => {
const worktreeId = 'repo-1::/remote/repo'
const context = resolveNativeChatSkillDiscoveryContext(
selectedSshInputs({
activeWorkspaceExecutionHostId: 'ssh:shared-target',
activeWorktreeId: worktreeId,
repos: [],
sshConnectionStates: new Map([
['shared-target', connectionState({ targetId: 'shared-target' })]
]),
tabsByWorktree: { [worktreeId]: [{ id: 'tab-1' }] },
worktreesByRepo: {
'repo-1': [
{
id: worktreeId,
repoId: 'repo-1',
path: '/remote/repo',
hostId: 'ssh:shared-target'
},
{
id: worktreeId,
repoId: 'repo-1',
path: '/remote/repo',
hostId: 'ssh:shared-target',
runtimeOwnerEnvironmentId: 'hub-a'
}
]
}
}),
'tab-1'
)
expect(context).toBeNull()
})
it('keeps the same remote path on two SSH hosts cache-isolated', () => {
const onTargetTwo = resolveNativeChatSkillDiscoveryContext(
sshInputs({