From 851ea077d583c097bb5241c35d2bb8f26d490a68 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Mon, 3 Aug 2026 19:33:47 -0700 Subject: [PATCH] fix(skills): fail closed during SSH owner hydration --- .../native-chat-folder-ssh-route.ts | 51 +++++++--- .../native-chat-skill-discovery-context.ts | 7 ++ .../native-chat-worktree-ssh-transport.ts | 15 ++- .../use-native-chat-skills.react.test.tsx | 19 ++++ .../use-native-chat-skills.test.ts | 93 +++++++++++++++++++ 5 files changed, 172 insertions(+), 13 deletions(-) diff --git a/src/renderer/src/components/native-chat/native-chat-folder-ssh-route.ts b/src/renderer/src/components/native-chat/native-chat-folder-ssh-route.ts index 4f2bafde299..102ba5cd80a 100644 --- a/src/renderer/src/components/native-chat/native-chat-folder-ssh-route.ts +++ b/src/renderer/src/components/native-chat/native-chat-folder-ssh-route.ts @@ -19,6 +19,7 @@ export type NativeChatFolderSshRouteResolution = environmentId: string | null } | { kind: 'ambiguous' } + | { kind: 'non-ssh'; ownerHostId: ExecutionHostId | null } | { kind: 'missing'; ownerHostId?: ExecutionHostId | null } type FolderSshRoute = Extract @@ -29,6 +30,7 @@ export function resolveNativeChatFolderSshRoute( ): NativeChatFolderSshRouteResolution { const routes = new Map() const nonSshOwnerHostIds: (ExecutionHostId | null)[] = [] + const unresolvedOwnerHostIds: (ExecutionHostId | null)[] = [] const activeHost = state.activeWorktreeId === folderWorkspaceKey(folderWorkspaceId) ? parseExecutionHostId(state.activeWorkspaceExecutionHostId) @@ -45,24 +47,22 @@ export function resolveNativeChatFolderSshRoute( const groupHost = parseExecutionHostId(group.executionHostId) return !workspaceHost || !groupHost || workspaceHost.id === groupHost.id }) - const owners = groups.length > 0 ? groups : [null] - for (const group of owners) { + for (const group of groups.length > 0 ? groups : [null]) { const route = routeForFolderOwners(workspace, group) if (route.kind === 'ambiguous') { return route } if (route.kind === 'resolved') { routes.set(JSON.stringify([route.hostId, route.environmentId]), route) + } else if (!group) { + unresolvedOwnerHostIds.push(route.ownerHostId ?? null) + } else if (route.kind === 'non-ssh') { + nonSshOwnerHostIds.push(route.ownerHostId) } else { - nonSshOwnerHostIds.push(route.ownerHostId ?? null) + unresolvedOwnerHostIds.push(route.ownerHostId ?? null) } } } - if (routes.size === 0) { - return activeHost?.kind === 'ssh' && nonSshOwnerHostIds.length > 0 - ? { kind: 'ambiguous' } - : { kind: 'missing' } - } const candidates = [...routes.values()].filter((route) => { if (!activeHost) { return true @@ -73,10 +73,37 @@ export function resolveNativeChatFolderSshRoute( ? activeHost.environmentId === route.environmentId : false }) - const hasRelevantNonSshOwner = nonSshOwnerHostIds.some( - (ownerHostId) => !activeHost || !ownerHostId || ownerHostId === activeHost.id + const ownerMatchesActiveHost = (ownerHostId: ExecutionHostId | null): boolean => + !activeHost || !ownerHostId || ownerHostId === activeHost.id + const relevantNonSshOwnerHostIds = nonSshOwnerHostIds.filter(ownerMatchesActiveHost) + const relevantUnresolvedOwnerHostIds = unresolvedOwnerHostIds.filter(ownerMatchesActiveHost) + const evidenceCount = routes.size + nonSshOwnerHostIds.length + unresolvedOwnerHostIds.length + if ( + activeHost && + candidates.length === 0 && + relevantNonSshOwnerHostIds.length === 0 && + relevantUnresolvedOwnerHostIds.length === 0 && + evidenceCount > 0 + ) { + return { kind: 'ambiguous' } + } + if (candidates.length > 0) { + return candidates.length === 1 && + relevantNonSshOwnerHostIds.length === 0 && + relevantUnresolvedOwnerHostIds.length === 0 + ? candidates[0] + : { kind: 'ambiguous' } + } + if (relevantUnresolvedOwnerHostIds.length > 0) { + return { kind: 'missing' } + } + const nonSshHostIds = new Set( + relevantNonSshOwnerHostIds.map((ownerHostId) => ownerHostId ?? 'unresolved') ) - return candidates.length === 1 && !hasRelevantNonSshOwner ? candidates[0] : { kind: 'ambiguous' } + if (nonSshHostIds.size === 1) { + return { kind: 'non-ssh', ownerHostId: relevantNonSshOwnerHostIds[0] ?? null } + } + return nonSshHostIds.size > 1 ? { kind: 'ambiguous' } : { kind: 'missing' } } function routeForFolderOwners( @@ -120,7 +147,7 @@ function routeForFolderOwners( hostId = logicalHost.id } if (!hostId) { - return { kind: 'missing', ownerHostId: logicalHost?.id ?? null } + return { kind: 'non-ssh', ownerHostId: logicalHost?.id ?? null } } const host = parseExecutionHostId(hostId) if (host?.kind !== 'ssh') { diff --git a/src/renderer/src/components/native-chat/native-chat-skill-discovery-context.ts b/src/renderer/src/components/native-chat/native-chat-skill-discovery-context.ts index 23c924c4759..a4739f17c02 100644 --- a/src/renderer/src/components/native-chat/native-chat-skill-discovery-context.ts +++ b/src/renderer/src/components/native-chat/native-chat-skill-discovery-context.ts @@ -114,6 +114,13 @@ export function resolveNativeChatSkillDiscoveryContext( if (folderSshRoute.kind === 'ambiguous') { return null } + if ( + workspaceScope?.type === 'folder' && + parsedHost?.kind === 'runtime' && + folderSshRoute.kind === 'missing' + ) { + return null + } const sshHost = folderSshRoute.kind === 'resolved' ? parseExecutionHostId(folderSshRoute.hostId) diff --git a/src/renderer/src/components/native-chat/native-chat-worktree-ssh-transport.ts b/src/renderer/src/components/native-chat/native-chat-worktree-ssh-transport.ts index edb8fdf20a9..a23272d8126 100644 --- a/src/renderer/src/components/native-chat/native-chat-worktree-ssh-transport.ts +++ b/src/renderer/src/components/native-chat/native-chat-worktree-ssh-transport.ts @@ -14,7 +14,10 @@ import { resolveExactWorktreeRoute } from '@/lib/worktree-owner-route' type WorktreeSshTransportState = Pick< AppState, - 'detectedWorktreesByRepo' | 'repos' | 'worktreesByRepo' + | 'detectedWorktreesByRepo' + | 'repos' + | 'restoredRuntimeHostIdByWorkspaceSessionKey' + | 'worktreesByRepo' > export type NativeChatWorktreeSshTransportResolution = @@ -77,6 +80,16 @@ export function resolveNativeChatWorktreeSshTransport( environmentIds.add(executionHost?.kind === 'runtime' ? executionHost.environmentId : null) } } + const restoredHostId = + state.restoredRuntimeHostIdByWorkspaceSessionKey[rawWorktreeId] ?? + state.restoredRuntimeHostIdByWorkspaceSessionKey[worktreeId] + if (restoredHostId) { + const restoredHost = parseExecutionHostId(restoredHostId) + if (restoredHost?.kind !== 'runtime') { + return { kind: 'ambiguous' } + } + environmentIds.add(restoredHost.environmentId) + } if (environmentIds.size > 1) { return { kind: 'ambiguous' } } diff --git a/src/renderer/src/components/native-chat/use-native-chat-skills.react.test.tsx b/src/renderer/src/components/native-chat/use-native-chat-skills.react.test.tsx index 6df3aef06d5..b982f648acb 100644 --- a/src/renderer/src/components/native-chat/use-native-chat-skills.react.test.tsx +++ b/src/renderer/src/components/native-chat/use-native-chat-skills.react.test.tsx @@ -282,6 +282,25 @@ describe('useNativeChatSkills', () => { ) }) + it('issues no RPC while a restored runtime folder catalog is missing', async () => { + const worktreeId = 'folder:folder-1' + mocks.state = { + ...stateForHost('runtime:hub-a'), + activeWorktreeId: worktreeId, + folderWorkspaces: [], + projectGroups: [], + restoredRuntimeHostIdByWorkspaceSessionKey: { [worktreeId]: 'runtime:hub-a' }, + tabsByWorktree: { + [worktreeId]: [{ id: 'tab-1', startupCwd: '/possibly-remote/folder' }] + }, + worktreesByRepo: {} + } + + render() + await waitFor(() => expect(mocks.snapshots.at(-1)?.status).toBe('idle')) + expect(mocks.callRuntimeRpc).not.toHaveBeenCalled() + }) + it('issues no RPC for an ambiguous direct and paired SSH pane', async () => { mocks.state = { ...stateForHost('ssh:shared-target'), diff --git a/src/renderer/src/components/native-chat/use-native-chat-skills.test.ts b/src/renderer/src/components/native-chat/use-native-chat-skills.test.ts index 450b2f3713e..443be9b8521 100644 --- a/src/renderer/src/components/native-chat/use-native-chat-skills.test.ts +++ b/src/renderer/src/components/native-chat/use-native-chat-skills.test.ts @@ -389,6 +389,27 @@ describe('resolveNativeChatSkillDiscoveryContext for SSH panes', () => { expect(context.paneTarget).toEqual({ worktreeId, terminalTabId: 'tab-1' }) }) + it('uses a restored paired runtime owner before the worktree catalog hydrates', () => { + const worktreeId = 'repo-1::/remote/repo' + const context = resolveNativeChatSkillDiscoveryContext( + selectedSshInputs({ + activeWorkspaceExecutionHostId: 'ssh:private-target', + activeWorktreeId: worktreeId, + repos: [], + restoredRuntimeHostIdByWorkspaceSessionKey: { [worktreeId]: 'runtime:hub-a' }, + sshConnectionStates: new Map([ + ['private-target', connectionState({ targetId: 'private-target' })] + ]), + tabsByWorktree: { [worktreeId]: [{ id: 'tab-1' }] }, + worktreesByRepo: {} + }), + 'tab-1' + ) + + expect(context?.executionHostKind).toBe('ssh') + expect(context?.runtimeTarget).toEqual({ kind: 'environment', environmentId: 'hub-a' }) + }) + it('uses the active SSH host while duplicate catalogs are still hydrating', () => { const worktreeId = 'repo-1::/remote/repo' const context = resolveNativeChatSkillDiscoveryContext( @@ -670,6 +691,78 @@ describe('resolveNativeChatSkillDiscoveryContext for SSH panes', () => { expect(context?.runtimeTarget).toEqual({ kind: 'environment', environmentId: 'hub-a' }) }) + it('fails closed while a restored runtime folder catalog is missing', () => { + const worktreeId = 'folder:folder-1' + const context = resolveNativeChatSkillDiscoveryContext( + selectedSshInputs({ + activeWorkspaceExecutionHostId: 'runtime:hub-a', + activeWorktreeId: worktreeId, + folderWorkspaces: [], + projectGroups: [], + repos: [], + restoredRuntimeHostIdByWorkspaceSessionKey: { [worktreeId]: 'runtime:hub-a' }, + tabsByWorktree: { + [worktreeId]: [{ id: 'tab-1', startupCwd: '/possibly-remote/folder' }] + }, + worktreesByRepo: {} + }), + 'tab-1' + ) + + expect(context).toBeNull() + }) + + it('fails closed before a runtime folder project group hydrates', () => { + const worktreeId = 'folder:folder-1' + const context = resolveNativeChatSkillDiscoveryContext( + selectedSshInputs({ + activeWorkspaceExecutionHostId: 'runtime:hub-a', + activeWorktreeId: worktreeId, + folderWorkspaces: [ + { + id: 'folder-1', + projectGroupId: 'group-1', + folderPath: '/possibly-remote/folder', + executionHostId: 'runtime:hub-a' + } + ], + projectGroups: [], + repos: [], + tabsByWorktree: { [worktreeId]: [{ id: 'tab-1' }] }, + worktreesByRepo: {} + }), + 'tab-1' + ) + + expect(context).toBeNull() + }) + + it('keeps a fully hydrated runtime-local folder on normal discovery', () => { + const worktreeId = 'folder:folder-1' + const context = resolveNativeChatSkillDiscoveryContext( + selectedSshInputs({ + activeWorkspaceExecutionHostId: 'runtime:hub-a', + activeWorktreeId: worktreeId, + folderWorkspaces: [ + { + id: 'folder-1', + projectGroupId: 'group-1', + folderPath: '/runtime/folder', + executionHostId: 'runtime:hub-a' + } + ], + projectGroups: [{ id: 'group-1', executionHostId: 'runtime:hub-a' }], + repos: [], + tabsByWorktree: { [worktreeId]: [{ id: 'tab-1' }] }, + worktreesByRepo: {} + }), + 'tab-1' + ) + + expect(context?.executionHostKind).toBe('runtime') + expect(context?.runtimeTarget).toEqual({ kind: 'environment', environmentId: 'hub-a' }) + }) + it('keeps the same remote path on two SSH hosts cache-isolated', () => { const onTargetTwo = resolveNativeChatSkillDiscoveryContext( sshInputs({