From d2741dc203e4134d71d004a0e80fc4f1ed410bbd Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Mon, 3 Aug 2026 18:59:18 -0700 Subject: [PATCH] fix(skills): preserve SSH owner hydration --- .../skills/discovery-wire-contract.test.ts | 13 +++- .../native-chat-skill-discovery-context.ts | 4 ++ .../use-native-chat-skills.test.ts | 65 ++++++++++++++++++- src/shared/skills.ts | 23 ++++--- 4 files changed, 92 insertions(+), 13 deletions(-) diff --git a/src/main/skills/discovery-wire-contract.test.ts b/src/main/skills/discovery-wire-contract.test.ts index 4dc646d201c..939f26975e9 100644 --- a/src/main/skills/discovery-wire-contract.test.ts +++ b/src/main/skills/discovery-wire-contract.test.ts @@ -2,10 +2,21 @@ import { mkdir, mkdtemp, writeFile } from 'node:fs/promises' import { tmpdir } from 'node:os' import { join } from 'node:path' import { describe, expect, it } from 'vitest' -import { parseSkillDiscoveryResult, SKILL_DISCOVERY_LIMITS } from '../../shared/skills' +import { + PaneSkillDiscoveryTargetSchema, + parseSkillDiscoveryResult, + SKILL_DISCOVERY_LIMITS +} from '../../shared/skills' import { discoverSkills } from './discovery' describe('skill discovery wire contract', () => { + it('accepts pane identities containing a valid long workspace path', () => { + const worktreeId = `repo-1::/${'nested/'.repeat(100)}repo` + + expect(worktreeId.length).toBeGreaterThan(512) + expect(() => PaneSkillDiscoveryTargetSchema.parse({ worktreeId })).not.toThrow() + }) + it('bounds scanner metadata before returning it to an SSH client', async () => { const root = await mkdtemp(join(tmpdir(), 'orca-skills-')) const home = join(root, 'home') 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 abba268de54..4d2cccb80ed 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 @@ -12,7 +12,9 @@ import { parseWorkspaceKey } from '../../../../shared/workspace-scope' export type NativeChatSkillStateInputs = Pick< AppState, | 'activeRepoId' + | 'activeWorkspaceExecutionHostId' | 'activeWorktreeId' + | 'detectedWorktreesByRepo' | 'folderWorkspaces' | 'projectGroups' | 'projects' @@ -53,7 +55,9 @@ export type NativeChatSkillDiscoveryContext = export function selectNativeChatSkillStateInputs(state: AppState): NativeChatSkillStateInputs { return { activeRepoId: state.activeRepoId, + activeWorkspaceExecutionHostId: state.activeWorkspaceExecutionHostId, activeWorktreeId: state.activeWorktreeId, + detectedWorktreesByRepo: state.detectedWorktreesByRepo, folderWorkspaces: state.folderWorkspaces, projectGroups: state.projectGroups, projects: state.projects, 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 b9d5d328500..9ab4875f316 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 @@ -1,5 +1,6 @@ import { describe, expect, it } from 'vitest' import type { DiscoveredSkill, SkillDiscoveryResult } from '../../../../shared/skills' +import type { AppState } from '../../store/types' import { isNativeChatSkillForAgent, resolveNativeChatSkillDiscoveryContext, @@ -7,6 +8,7 @@ import { } from './use-native-chat-skills' import { getNativeChatSkillDiscoverySubscriptionKey, + selectNativeChatSkillStateInputs, type NativeChatSkillStateInputs } from './native-chat-skill-discovery-context' @@ -178,7 +180,9 @@ function connectionState(overrides: Record = {}): Record = {}): NativeChatSkillStateInputs { return { activeRepoId: 'repo-1', + activeWorkspaceExecutionHostId: null, activeWorktreeId: 'worktree-1', + detectedWorktreesByRepo: {}, folderWorkspaces: [], projectGroups: [], projects: [], @@ -197,6 +201,10 @@ function sshInputs(overrides: Record = {}): NativeChatSkillStat } as unknown as NativeChatSkillStateInputs } +function selectedSshInputs(overrides: Record): NativeChatSkillStateInputs { + return selectNativeChatSkillStateInputs(sshInputs(overrides) as unknown as AppState) +} + describe('resolveNativeChatSkillDiscoveryContext for SSH panes', () => { it('builds a pane-bound SSH context keyed on the connection generation', () => { const context = resolveNativeChatSkillDiscoveryContext(sshInputs(), 'tab-1') @@ -210,7 +218,7 @@ describe('resolveNativeChatSkillDiscoveryContext for SSH panes', () => { expect(context.key).toContain('7') const regenerated = resolveNativeChatSkillDiscoveryContext( - sshInputs({ + selectedSshInputs({ sshConnectionStates: new Map([['target-1', connectionState({ connectionGeneration: 8 })]]) }), 'tab-1' @@ -225,7 +233,7 @@ describe('resolveNativeChatSkillDiscoveryContext for SSH panes', () => { 'tab-1' ) const reconnecting = resolveNativeChatSkillDiscoveryContext( - sshInputs({ + selectedSshInputs({ sshConnectionStates: new Map([['target-1', connectionState({ status: 'reconnecting' })]]) }), 'tab-1' @@ -342,6 +350,59 @@ describe('resolveNativeChatSkillDiscoveryContext for SSH panes', () => { expect(context.paneTarget).toEqual({ worktreeId, terminalTabId: 'tab-1' }) }) + it('uses the active SSH host while duplicate catalogs are still hydrating', () => { + const worktreeId = 'repo-1::/remote/repo' + const context = resolveNativeChatSkillDiscoveryContext( + selectedSshInputs({ + activeWorkspaceExecutionHostId: 'ssh:target-2', + activeWorktreeId: worktreeId, + repos: [ + { id: 'repo-1', path: '/remote/repo', connectionId: 'target-1' }, + { id: 'repo-1', path: '/remote/repo', connectionId: 'target-2' } + ], + sshConnectionStates: new Map([['target-2', connectionState({ targetId: 'target-2' })]]), + tabsByWorktree: { [worktreeId]: [{ id: 'tab-1' }] }, + worktreesByRepo: {} + }), + 'tab-1' + ) + + expect(context?.executionHostKind).toBe('ssh') + expect(context?.key).toContain('ssh:target-2') + }) + + it('uses a detected-only SSH owner before the primary worktree catalog hydrates', () => { + const worktreeId = 'repo-1::/remote/repo' + const context = resolveNativeChatSkillDiscoveryContext( + selectedSshInputs({ + activeWorktreeId: 'other-worktree', + detectedWorktreesByRepo: { + 'repo-1': { + worktrees: [ + { + id: worktreeId, + repoId: 'repo-1', + path: '/remote/repo', + hostId: 'ssh:target-2' + } + ] + } + }, + repos: [ + { id: 'repo-1', path: '/remote/repo', connectionId: 'target-1' }, + { id: 'repo-1', path: '/remote/repo', connectionId: 'target-2' } + ], + sshConnectionStates: new Map([['target-2', connectionState({ targetId: 'target-2' })]]), + tabsByWorktree: { [worktreeId]: [{ id: 'tab-1' }] }, + worktreesByRepo: {} + }), + 'tab-1' + ) + + expect(context?.executionHostKind).toBe('ssh') + expect(context?.key).toContain('ssh:target-2') + }) + it('keeps the same remote path on two SSH hosts cache-isolated', () => { const onTargetTwo = resolveNativeChatSkillDiscoveryContext( sshInputs({ diff --git a/src/shared/skills.ts b/src/shared/skills.ts index c76b4728108..e5b46d7e394 100644 --- a/src/shared/skills.ts +++ b/src/shared/skills.ts @@ -109,11 +109,23 @@ export type PaneSkillDiscoveryTarget = { terminalTabId?: string } +export const SKILL_DISCOVERY_LIMITS = { + descriptionLength: 8192, + nameLength: 512, + pathLength: 4096, + rootPaths: 64, + skills: 5000, + sources: 1000 +} as const + /** Pane discovery carries workspace/pane identity only — never a path or SSH * connection id. The owning runtime derives the scan directory from its own * persisted state. */ export const PaneSkillDiscoveryTargetSchema: z.ZodType = z.object({ - worktreeId: z.string().min(1).max(512), + worktreeId: z + .string() + .min(1) + .max(SKILL_DISCOVERY_LIMITS.pathLength + 512), terminalTabId: z.string().min(1).max(512).optional() }) @@ -127,15 +139,6 @@ export type SkillDiscoveryForPaneResponse = const SKILL_PROVIDER_VALUES = ['codex', 'claude', 'agent-skills'] as const const SKILL_SOURCE_KIND_VALUES = ['home', 'repo', 'bundled', 'plugin'] as const -export const SKILL_DISCOVERY_LIMITS = { - descriptionLength: 8192, - nameLength: 512, - pathLength: 4096, - rootPaths: 64, - skills: 5000, - sources: 1000 -} as const - const boundedString = (max: number): z.ZodString => z.string().max(max) /** Validates untrusted skill metadata at the SSH relay boundary before it can