diff --git a/src/main/skills/discovery.ts b/src/main/skills/discovery.ts index 7977fa4e996..7b84a5d4044 100644 --- a/src/main/skills/discovery.ts +++ b/src/main/skills/discovery.ts @@ -1,8 +1,7 @@ import type { Dirent } from 'node:fs' -import { open, readdir, realpath, stat } from 'node:fs/promises' +import { readdir, realpath, stat } from 'node:fs/promises' import { homedir } from 'node:os' -import { basename, dirname, isAbsolute, join, relative, sep } from 'node:path' -import { summarizeSkillMarkdown } from '../../shared/skill-metadata' +import { isAbsolute, join, relative, sep } from 'node:path' import type { Repo } from '../../shared/types' import { SKILL_DISCOVERY_LIMITS, @@ -10,22 +9,14 @@ import { type SkillDiscoveryResult, type SkillDiscoverySource } from '../../shared/skills' -import { - buildSkillDiscoverySources, - compareSkills, - sourceKindForSkill, - sourceLabelForSkill, - stablePathId, - type SkillScanRoot -} from './skill-discovery-sources' +import { buildSkillDiscoverySources, compareSkills } from './skill-discovery-sources' import { discoverClaudePluginSkillSources } from './claude-plugin-skill-sources' import { runSkillCandidateTasks } from './skill-candidate-concurrency' +import { scanSkillFile, type ScannedSkill } from './skill-file-scanner' export { buildSkillDiscoverySources } from './skill-discovery-sources' const SKILL_FILE_NAME = 'SKILL.md' -const MAX_MARKDOWN_BYTES = 256 * 1024 -const MAX_SKILL_FILES = 200 /** Node-version-safe alternative to signal.throwIfAborted(); an aborted scan * must reject rather than resolve with partial results. */ @@ -61,13 +52,14 @@ function isWithinDepth(rootPath: string, childPath: string, maxDepth: number): b async function findSkillFiles( rootPath: string, maxDepth: number, + budget: { remaining: number }, signal?: AbortSignal ): Promise { const out: string[] = [] const visitedDirectoryPaths = new Set() async function visit(dirPath: string): Promise { throwIfDiscoveryAborted(signal) - if (out.length >= SKILL_DISCOVERY_LIMITS.skills) { + if (budget.remaining <= 0) { return } if (!isWithinDepth(rootPath, dirPath, maxDepth)) { @@ -90,20 +82,26 @@ async function findSkillFiles( } catch { return } + entries.sort((left, right) => (left.name < right.name ? -1 : left.name > right.name ? 1 : 0)) for (const entry of entries) { throwIfDiscoveryAborted(signal) - if (out.length >= SKILL_DISCOVERY_LIMITS.skills) { + if (budget.remaining <= 0) { return } const entryPath = join(dirPath, entry.name) if (entry.name === SKILL_FILE_NAME) { if (entry.isFile()) { + budget.remaining -= 1 out.push(entryPath) continue } if (entry.isSymbolicLink()) { try { if ((await stat(entryPath)).isFile()) { + if (budget.remaining <= 0) { + return + } + budget.remaining -= 1 out.push(entryPath) } } catch { @@ -133,120 +131,6 @@ async function findSkillFiles( return out } -async function countFiles(dirPath: string, signal?: AbortSignal): Promise { - let count = 0 - const visitedDirectoryPaths = new Set() - async function visit(currentPath: string): Promise { - throwIfDiscoveryAborted(signal) - if (count >= MAX_SKILL_FILES) { - return - } - let resolvedPath: string - try { - resolvedPath = await realpath(currentPath) - } catch { - return - } - if (visitedDirectoryPaths.has(resolvedPath)) { - return - } - visitedDirectoryPaths.add(resolvedPath) - - let entries: Dirent[] - try { - entries = await readdir(currentPath, { withFileTypes: true }) - } catch { - return - } - for (const entry of entries) { - if (count >= MAX_SKILL_FILES) { - return - } - const entryPath = join(currentPath, entry.name) - if (entry.isFile()) { - count += 1 - } else if (entry.isDirectory()) { - await visit(entryPath) - } else if (entry.isSymbolicLink()) { - try { - if ((await stat(entryPath)).isFile()) { - count += 1 - } - } catch { - // Broken links do not contribute to the skill package file count. - } - } - } - } - await visit(dirPath) - return count -} - -async function readSkillSummary(skillFilePath: string): Promise<{ - name: string | null - description: string | null - updatedAt: number | null -} | null> { - try { - const fileStat = await stat(skillFilePath) - const file = await open(skillFilePath, 'r') - let content = '' - try { - const buffer = Buffer.alloc(Math.min(fileStat.size, MAX_MARKDOWN_BYTES)) - const { bytesRead } = await file.read(buffer, 0, buffer.length, 0) - content = buffer.toString('utf8', 0, bytesRead) - } finally { - await file.close() - } - return { - ...summarizeSkillMarkdown(content), - updatedAt: fileStat.mtimeMs - } - } catch { - return null - } -} - -type ScannedSkill = DiscoveredSkill & { canonicalSkillFilePath: string } - -async function scanRoot(root: SkillScanRoot, signal?: AbortSignal): Promise { - const maxDepth = root.sourceKind === 'plugin' ? 9 : 4 - const skillFiles = await findSkillFiles(root.path, maxDepth, signal) - const skills = await runSkillCandidateTasks( - skillFiles.map((skillFilePath) => async (): Promise => { - throwIfDiscoveryAborted(signal) - // Why: path identity belongs to the scanning host; canonicalizing before - // returning prevents symlinked roots from becoming duplicate picker rows. - const canonicalSkillFilePath = await realpath(skillFilePath).catch(() => skillFilePath) - const directoryPath = dirname(skillFilePath) - const summary = await readSkillSummary(skillFilePath) - if (!summary) { - return null - } - const sourceKind = sourceKindForSkill(root, skillFilePath, { relative, sep }) - return { - id: stablePathId(canonicalSkillFilePath), - name: (summary.name ?? basename(directoryPath)).slice(0, SKILL_DISCOVERY_LIMITS.nameLength), - description: - summary.description?.slice(0, SKILL_DISCOVERY_LIMITS.descriptionLength) ?? null, - // Copy: `root.providers` is shared across every skill/source from this - // root, so the dedup merge below must not mutate the aliased array. - providers: [...root.providers], - sourceKind, - sourceLabel: sourceLabelForSkill(root, sourceKind), - rootPath: root.path, - directoryPath, - skillFilePath, - installed: true, - fileCount: await countFiles(directoryPath, signal), - updatedAt: summary.updatedAt, - canonicalSkillFilePath - } satisfies ScannedSkill - }) - ) - return skills.filter((skill): skill is ScannedSkill => skill !== null) -} - export async function discoverSkills(args: { repos?: Repo[] homeDir?: string @@ -267,26 +151,33 @@ export async function discoverSkills(args: { ] .filter((root) => root.path.length <= SKILL_DISCOVERY_LIMITS.pathLength) .slice(0, SKILL_DISCOVERY_LIMITS.sources) - const sources: SkillDiscoverySource[] = [] - const skillGroups = await runSkillCandidateTasks( + const budget = { remaining: SKILL_DISCOVERY_LIMITS.skills } + const rootScans = await runSkillCandidateTasks( roots.map((root) => async () => { throwIfDiscoveryAborted(signal) const exists = await pathExists(root.path) - sources.push({ - ...root, - providers: [...root.providers], - exists, - skippedReason: exists ? undefined : 'missing' - }) - if (!exists) { - return [] - } - return scanRoot(root, signal) + const maxDepth = root.sourceKind === 'plugin' ? 9 : 4 + const skillFiles = exists ? await findSkillFiles(root.path, maxDepth, budget, signal) : [] + return { root, exists, skillFiles } }) ) throwIfDiscoveryAborted(signal) + const sources: SkillDiscoverySource[] = rootScans.map(({ root, exists }) => ({ + ...root, + providers: [...root.providers], + exists, + skippedReason: exists ? undefined : 'missing' + })) + const scannedSkills = ( + await runSkillCandidateTasks( + rootScans.flatMap(({ root, skillFiles }) => + skillFiles.map((skillFilePath) => () => scanSkillFile(root, skillFilePath, signal)) + ) + ) + ).filter((skill): skill is ScannedSkill => skill !== null) + throwIfDiscoveryAborted(signal) const seen = new Map() - for (const skill of skillGroups.flat()) { + for (const skill of scannedSkills) { // Why: overlapping repo/cwd roots and symlinked provider homes can reach // the same file. Keep the first source's higher-level scope identity, but // record every contributing root so per-agent visibility survives dedup. diff --git a/src/main/skills/pane-skill-discovery-workspace.test.ts b/src/main/skills/pane-skill-discovery-workspace.test.ts index 5c4f48d9d50..5cf53db474e 100644 --- a/src/main/skills/pane-skill-discovery-workspace.test.ts +++ b/src/main/skills/pane-skill-discovery-workspace.test.ts @@ -147,4 +147,30 @@ describe('resolvePaneSkillDiscoveryWorkspace', () => { }) ).toEqual({ connectionId: 'target-1', cwd: '/remote/folder' }) }) + + it('isolates duplicate folder project groups using persisted workspace ownership', () => { + const workspace = { + id: 'folder-1', + projectGroupId: 'group-1', + folderPath: '/remote/folder', + connectionId: 'target-2' + } as FolderWorkspace + const group = (connectionId: string): ProjectGroup => + ({ + id: 'group-1', + connectionId, + executionHostId: `ssh:${connectionId}`, + parentPath: '/remote/folder' + }) as ProjectGroup + + expect( + resolvePaneSkillDiscoveryWorkspace({ + worktreeId: 'folder:folder-1', + repos: [], + projectGroups: [group('target-1'), group('target-2')], + folderWorkspaces: [workspace], + sessions: [] + }) + ).toEqual({ connectionId: 'target-2', cwd: '/remote/folder' }) + }) }) diff --git a/src/main/skills/pane-skill-discovery-workspace.ts b/src/main/skills/pane-skill-discovery-workspace.ts index 881a4b078c0..8ef32d8a7a4 100644 --- a/src/main/skills/pane-skill-discovery-workspace.ts +++ b/src/main/skills/pane-skill-discovery-workspace.ts @@ -48,14 +48,16 @@ export function resolvePaneSkillDiscoveryWorkspace( if (!workspace) { throw new Error('pane_skill_discovery_workspace_not_found') } - const group = args.projectGroups.find((candidate) => candidate.id === workspace.projectGroupId) - const executionHostId = parseOptionalHostId(workspace.executionHostId ?? group?.executionHostId) - const connectionId = parseOptionalConnectionId(workspace.connectionId ?? group?.connectionId) - const connectionHostId = connectionId ? toSshExecutionHostId(connectionId) : null - assertMatchingHost(executionHostId, connectionHostId) + const workspaceHostId = declaredHostId(workspace) + const group = selectFolderProjectGroup( + args.projectGroups.filter((candidate) => candidate.id === workspace.projectGroupId), + sessionOwner?.hostId ?? workspaceHostId + ) + const groupHostId = group ? declaredHostId(group) : null + assertMatchingHost(workspaceHostId, groupHostId) const hostId = - executionHostId ?? - connectionHostId ?? + workspaceHostId ?? + groupHostId ?? inferLegacyFolderHostId(workspace, args.projectGroups, args.repos) assertMatchingHost(sessionOwner?.hostId, hostId) return sshWorkspace(hostId, sessionOwner?.tab.startupCwd?.trim() || workspace.folderPath) @@ -135,6 +137,37 @@ function selectFolderWorkspace( return exact[0] } +function selectFolderProjectGroup( + candidates: readonly ProjectGroup[], + hostId: ExecutionHostId | null | undefined +): ProjectGroup | null { + if (candidates.length === 0) { + return null + } + if (candidates.length === 1) { + return candidates[0] + } + if (!hostId) { + throw new Error('pane_skill_discovery_owner_ambiguous') + } + const exact = candidates.filter((group) => declaredHostId(group) === hostId) + if (exact.length !== 1) { + throw new Error('pane_skill_discovery_owner_ambiguous') + } + return exact[0] +} + +function declaredHostId(owner: { + executionHostId?: string | null + connectionId?: string | null +}): ExecutionHostId | null { + const executionHostId = parseOptionalHostId(owner.executionHostId) + const connectionId = parseOptionalConnectionId(owner.connectionId) + const connectionHostId = connectionId ? toSshExecutionHostId(connectionId) : null + assertMatchingHost(executionHostId, connectionHostId) + return executionHostId ?? connectionHostId +} + function findPaneSession( sessions: readonly HostSession[], worktreeId: string, diff --git a/src/main/skills/skill-file-scanner.ts b/src/main/skills/skill-file-scanner.ts new file mode 100644 index 00000000000..abed33daa25 --- /dev/null +++ b/src/main/skills/skill-file-scanner.ts @@ -0,0 +1,131 @@ +import type { Dirent } from 'node:fs' +import { open, readdir, realpath, stat } from 'node:fs/promises' +import { basename, dirname, join, relative, sep } from 'node:path' +import { summarizeSkillMarkdown } from '../../shared/skill-metadata' +import { SKILL_DISCOVERY_LIMITS, type DiscoveredSkill } from '../../shared/skills' +import { + sourceKindForSkill, + sourceLabelForSkill, + stablePathId, + type SkillScanRoot +} from './skill-discovery-sources' + +const MAX_MARKDOWN_BYTES = 256 * 1024 +const MAX_SKILL_FILES = 200 + +export type ScannedSkill = DiscoveredSkill & { canonicalSkillFilePath: string } + +function throwIfAborted(signal: AbortSignal | undefined): void { + if (signal?.aborted) { + const error = new Error('Skill discovery aborted') + error.name = 'AbortError' + throw error + } +} + +async function countFiles(dirPath: string, signal?: AbortSignal): Promise { + let count = 0 + const visitedDirectoryPaths = new Set() + async function visit(currentPath: string): Promise { + throwIfAborted(signal) + if (count >= MAX_SKILL_FILES) { + return + } + let resolvedPath: string + try { + resolvedPath = await realpath(currentPath) + } catch { + return + } + if (visitedDirectoryPaths.has(resolvedPath)) { + return + } + visitedDirectoryPaths.add(resolvedPath) + + let entries: Dirent[] + try { + entries = await readdir(currentPath, { withFileTypes: true }) + } catch { + return + } + for (const entry of entries) { + if (count >= MAX_SKILL_FILES) { + return + } + const entryPath = join(currentPath, entry.name) + if (entry.isFile()) { + count += 1 + } else if (entry.isDirectory()) { + await visit(entryPath) + } else if (entry.isSymbolicLink()) { + try { + if ((await stat(entryPath)).isFile()) { + count += 1 + } + } catch { + // Broken links do not contribute to the skill package file count. + } + } + } + } + await visit(dirPath) + return count +} + +async function readSkillSummary(skillFilePath: string): Promise<{ + name: string | null + description: string | null + updatedAt: number | null +} | null> { + try { + const fileStat = await stat(skillFilePath) + const file = await open(skillFilePath, 'r') + let content = '' + try { + const buffer = Buffer.alloc(Math.min(fileStat.size, MAX_MARKDOWN_BYTES)) + const { bytesRead } = await file.read(buffer, 0, buffer.length, 0) + content = buffer.toString('utf8', 0, bytesRead) + } finally { + await file.close() + } + return { ...summarizeSkillMarkdown(content), updatedAt: fileStat.mtimeMs } + } catch { + return null + } +} + +export async function scanSkillFile( + root: SkillScanRoot, + skillFilePath: string, + signal?: AbortSignal +): Promise { + throwIfAborted(signal) + const canonicalSkillFilePath = await realpath(skillFilePath).catch(() => skillFilePath) + const directoryPath = dirname(skillFilePath) + if ( + directoryPath.length > SKILL_DISCOVERY_LIMITS.pathLength || + skillFilePath.length > SKILL_DISCOVERY_LIMITS.pathLength + ) { + return null + } + const summary = await readSkillSummary(skillFilePath) + if (!summary) { + return null + } + const sourceKind = sourceKindForSkill(root, skillFilePath, { relative, sep }) + return { + id: stablePathId(canonicalSkillFilePath), + name: (summary.name ?? basename(directoryPath)).slice(0, SKILL_DISCOVERY_LIMITS.nameLength), + description: summary.description?.slice(0, SKILL_DISCOVERY_LIMITS.descriptionLength) ?? null, + providers: [...root.providers], + sourceKind, + sourceLabel: sourceLabelForSkill(root, sourceKind), + rootPath: root.path, + directoryPath, + skillFilePath, + installed: true, + fileCount: await countFiles(directoryPath, signal), + updatedAt: summary.updatedAt, + canonicalSkillFilePath + } +} 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 4af023e78d7..abba268de54 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 @@ -42,7 +42,6 @@ export type NativeChatSkillDiscoveryContext = } | { key: string - cwd: string executionHostKind: 'ssh' runtimeTarget: RuntimeClientTarget /** Identity only; the owning runtime derives the scanned directory. */ @@ -99,18 +98,6 @@ export function resolveNativeChatSkillDiscoveryContext( if (!worktreeId) { return null } - const workspaceScope = parseWorkspaceKey(worktreeId) - const cwd = - resolveNativeChatSkillDiscoveryCwd(state, terminalTabId) ?? - (workspaceScope?.type === 'folder' - ? state.folderWorkspaces.find( - (workspace) => workspace.id === workspaceScope.folderWorkspaceId - )?.folderPath - : null) - if (!cwd) { - return null - } - const hostId = getExecutionHostIdForWorktree(state, worktreeId) const parsedHost = parseExecutionHostId(hostId) if (parsedHost?.kind === 'ssh') { @@ -134,11 +121,9 @@ export function resolveNativeChatSkillDiscoveryContext( runtimeEnvironmentId ?? null, hostId, connectionState?.connectionGeneration ?? 0, - cwd, worktreeId, terminalTabId ]), - cwd, executionHostKind: 'ssh', runtimeTarget: runtimeEnvironmentId ? { kind: 'environment', environmentId: runtimeEnvironmentId } @@ -148,6 +133,18 @@ export function resolveNativeChatSkillDiscoveryContext( } } + const workspaceScope = parseWorkspaceKey(worktreeId) + const cwd = + resolveNativeChatSkillDiscoveryCwd(state, terminalTabId) ?? + (workspaceScope?.type === 'folder' + ? state.folderWorkspaces.find( + (workspace) => workspace.id === workspaceScope.folderWorkspaceId + )?.folderPath + : null) + if (!cwd) { + return null + } + const runtimeEnvironmentId = getExplicitRuntimeEnvironmentIdForWorktree(state, worktreeId) // Why: a selected global runtime is not proof that it owns this pane. Modern // panes carry an owner stamp; ambiguous legacy panes stay not-ready. 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 a38d8df0a85..7df70e1f106 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 @@ -1,6 +1,6 @@ // @vitest-environment happy-dom -import { cleanup, render, waitFor } from '@testing-library/react' +import { act, cleanup, render, waitFor } from '@testing-library/react' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import type { NativeChatSkillDiscovery } from './use-native-chat-skills' @@ -224,4 +224,28 @@ describe('useNativeChatSkills', () => { expect(mocks.snapshots.at(-1)?.status).toBe('ready') }) }) + + it('does not cache a canceled reconnect generation', async () => { + const pending: ((value: { status: 'ok'; result: typeof DISCOVERY_RESULT }) => void)[] = [] + mocks.state = connectedSshState(1) + mocks.callRuntimeRpc.mockImplementation( + () => + new Promise((resolve) => { + pending.push(resolve) + }) + ) + const view = render() + await waitFor(() => expect(mocks.callRuntimeRpc).toHaveBeenCalledTimes(1)) + + mocks.state = connectedSshState(2) + view.rerender() + await waitFor(() => expect(mocks.callRuntimeRpc).toHaveBeenCalledTimes(2)) + await act(async () => pending[1]({ status: 'ok', result: DISCOVERY_RESULT })) + await waitFor(() => expect(mocks.snapshots.at(-1)?.status).toBe('ready')) + await act(async () => pending[0]({ status: 'ok', result: DISCOVERY_RESULT })) + + mocks.state = connectedSshState(1) + view.rerender() + await waitFor(() => expect(mocks.callRuntimeRpc).toHaveBeenCalledTimes(3)) + }) }) 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 df6fb3d2e5f..b9d5d328500 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 @@ -325,6 +325,23 @@ describe('resolveNativeChatSkillDiscoveryContext for SSH panes', () => { expect(context?.executionHostKind === 'ssh' && context.sshDisconnected).toBe(false) }) + it('uses pane identity when the SSH worktree catalog has not hydrated a cwd', () => { + const worktreeId = 'repo-1::/repo/worktree' + const context = resolveNativeChatSkillDiscoveryContext( + sshInputs({ + tabsByWorktree: { [worktreeId]: [{ id: 'tab-1' }] }, + worktreesByRepo: {} + }), + 'tab-1' + ) + + expect(context?.executionHostKind).toBe('ssh') + if (context?.executionHostKind !== 'ssh') { + throw new Error('expected ssh context') + } + expect(context.paneTarget).toEqual({ worktreeId, terminalTabId: 'tab-1' }) + }) + it('keeps the same remote path on two SSH hosts cache-isolated', () => { const onTargetTwo = resolveNativeChatSkillDiscoveryContext( sshInputs({ @@ -341,7 +358,6 @@ describe('resolveNativeChatSkillDiscoveryContext for SSH panes', () => { 'tab-1' ) const onTargetOne = resolveNativeChatSkillDiscoveryContext(sshInputs(), 'tab-1') - expect(onTargetOne?.cwd).toBe(onTargetTwo?.cwd) expect(onTargetOne?.key).not.toBe(onTargetTwo?.key) }) @@ -358,7 +374,6 @@ describe('resolveNativeChatSkillDiscoveryContext for SSH panes', () => { }), 'tab-2' ) - expect(firstPane?.cwd).toBe(secondPane?.cwd) expect(firstPane?.key).not.toBe(secondPane?.key) }) diff --git a/src/renderer/src/components/native-chat/use-native-chat-skills.ts b/src/renderer/src/components/native-chat/use-native-chat-skills.ts index 44c9f26d271..1bcb5ea0698 100644 --- a/src/renderer/src/components/native-chat/use-native-chat-skills.ts +++ b/src/renderer/src/components/native-chat/use-native-chat-skills.ts @@ -146,10 +146,10 @@ export function useNativeChatSkills( const request = getOrStartDiscovery(context) void request.then( (result) => { - writePaneDiscoveryCache(paneDiscoveryCache.current, paneCacheKey, result) if (cancelled) { return } + writePaneDiscoveryCache(paneDiscoveryCache.current, paneCacheKey, result) emitNativeChatSkillDiscovery({ agent, outcome: 'ready',