diff --git a/src/main/runtime/rpc/methods/skills.test.ts b/src/main/runtime/rpc/methods/skills.test.ts index 43546d2bb9a..b5418c7ce54 100644 --- a/src/main/runtime/rpc/methods/skills.test.ts +++ b/src/main/runtime/rpc/methods/skills.test.ts @@ -9,6 +9,11 @@ vi.mock('../../../skills/skill-discovery-target', () => ({ resolveSkillDiscoveryTarget: vi.fn((target) => ({ kind: 'native-host', cwd: target?.cwd })), discoverSkillsOnTarget: vi.fn(async () => ({ skills: [], sources: [], scannedAt: 1 })) })) +// Off Windows the real gate never sees a distro; stand in for its UNC parse. +vi.mock('../../../wsl/wsl-unc-access-gate', async (importOriginal) => ({ + ...(await importOriginal>()), + wslDistroForPath: (path: string) => /^\\\\wsl\.localhost\\([^\\]+)/.exec(path)?.[1] ?? null +})) import { SKILL_METHODS } from './skills' import { discoverSkillsOnTarget, @@ -345,13 +350,23 @@ describe('skills.share RPC', () => { expect(publishDiscoveredSkillsFromAgent).toHaveBeenCalledWith(request, discoveredSkills, signal) }) - it('reads stopped distros only when a selector is missing and a folder went unread', async () => { - const stoppedSource = { id: 'wsl-root', sourceKind: 'repo', skippedReason: 'unavailable' } + it('reads only the unread distros when a selector is missing', async () => { + const unavailable = (path: string) => ({ + id: path, + path, + sourceKind: 'repo', + skippedReason: 'unavailable' + }) vi.mocked(discoverSkillsOnTarget) - // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the handler reads only skill ids and source skippedReason. + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the handler reads only skill ids and source skippedReason and path. .mockResolvedValueOnce({ skills: [{ id: 'alpha' }], - sources: [stoppedSource], + sources: [ + unavailable('\\\\wsl.localhost\\Ubuntu\\home\\me\\.claude\\skills'), + unavailable('\\\\wsl.localhost\\Ubuntu\\repo\\.claude\\skills'), + // A host folder that failed to read starts no distro. + unavailable('C:\\Users\\me\\.claude\\skills') + ], scannedAt: 1 } as never) // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the handler reads only skill ids and source skippedReason. @@ -374,7 +389,7 @@ describe('skills.share RPC', () => { expect(vi.mocked(discoverSkillsOnTarget).mock.calls.map((call) => call[2])).toEqual([ { providerRootOverrides: {}, refresh: true }, - { providerRootOverrides: {}, refresh: true, readStoppedWslRootsFor: 'all' } + { providerRootOverrides: {}, refresh: true, readStoppedWslRootsFor: { distros: ['Ubuntu'] } } ]) expect(publishDiscoveredSkillsFromAgent).toHaveBeenCalledWith( request, @@ -383,6 +398,30 @@ describe('skills.share RPC', () => { ) }) + it('starts no distro when the only unread folders are on the host', async () => { + vi.mocked(discoverSkillsOnTarget) + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the handler reads only skill ids and source skippedReason and path. + .mockResolvedValueOnce({ + skills: [{ id: 'alpha' }], + sources: [ + { id: 'host', path: 'C:\\skills', sourceKind: 'home', skippedReason: 'unavailable' } + ], + scannedAt: 1 + } as never) + const runtime = { + assertAgentSkillSharingAllowed: vi.fn(), + resolveProjectRuntimeForWorktree: vi.fn(), + resolveSkillDiscoveryProviderRoots: vi.fn(async () => ({})), + listRepos: vi.fn(() => []), + publishDiscoveredSkillsFromAgent: vi.fn(async () => ({ status: 'ok' })) + } + + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: skills.share reads only these runtime members and the signal. + await method('skills.share').handler(request, { runtime } as unknown as RpcContext) + + expect(discoverSkillsOnTarget).toHaveBeenCalledTimes(1) + }) + it('rejects WSL before attempting to read Linux paths with native fs', async () => { vi.mocked(resolveSkillDiscoveryTarget).mockResolvedValueOnce({ kind: 'wsl', diff --git a/src/main/runtime/rpc/methods/skills.ts b/src/main/runtime/rpc/methods/skills.ts index a5bf89b2c67..3a1b90a216c 100644 --- a/src/main/runtime/rpc/methods/skills.ts +++ b/src/main/runtime/rpc/methods/skills.ts @@ -160,7 +160,7 @@ export const SKILL_METHODS = [ ) } // Sharing named skills is a user act: it may start the workspace's distro. - const discover = async (readStoppedWslRootsFor?: 'all') => + const discover = async (readStoppedWslRootsFor?: { distros: readonly string[] }) => runAsWslUserAct( resolvedTarget.cwd ? wslDistroForPath(resolvedTarget.cwd) : null, async () => @@ -171,15 +171,22 @@ export const SKILL_METHODS = [ ...(readStoppedWslRootsFor ? { readStoppedWslRootsFor } : {}) }) ) - // Selectors name skills, not folders: start other stopped distros only when a selector is missing. + // Selectors name skills, not folders: when one is missing, start only the distros left unread. let discovered = await discover() + const unreadDistros = [ + ...new Set( + discovered.sources.flatMap((source) => + source.skippedReason === 'unavailable' ? (wslDistroForPath(source.path) ?? []) : [] + ) + ) + ] if ( + unreadDistros.length > 0 && !params.skillSelectors.every((selector) => discovered.skills.some((skill) => skill.id === selector || skill.name === selector) - ) && - discovered.sources.some((source) => source.skippedReason === 'unavailable') + ) ) { - discovered = await discover('all') + discovered = await discover({ distros: unreadDistros }) } return runtime.publishDiscoveredSkillsFromAgent(params, discovered.skills, signal) } diff --git a/src/main/skills/discovery.ts b/src/main/skills/discovery.ts index ad9e05be752..7f510cb40e6 100644 --- a/src/main/skills/discovery.ts +++ b/src/main/skills/discovery.ts @@ -25,7 +25,8 @@ import { splitStoppedWslDistroRoots, stoppedPluginRoot, stoppedRootReader, - unavailableSkillSource + unavailableSkillSource, + type StoppedWslRootsReadScope } from './skill-discovery-stopped-wsl-roots' import { wslDistroForPath } from '../wsl/wsl-unc-access-gate' import { buildRemoteRepoSkillSources } from './skill-remote-repo-sources' @@ -236,10 +237,11 @@ export async function discoverSkills(args: { names?: string[] sourceKinds?: SkillSourceKind[] /** - * A user act (Share) may read stopped WSL distros' folders: 'all', or only the roots holding these - * skill directories. Every other stopped root is reported unavailable, so its distro stays down. + * A user act (Share) may read stopped WSL distros' folders: 'all', those on named distros, or only + * the roots holding these skill directories. Every other stopped root is reported unavailable, so + * its distro stays down. */ - readStoppedWslRootsFor?: 'all' | readonly string[] + readStoppedWslRootsFor?: StoppedWslRootsReadScope }): Promise { args.signal?.throwIfAborted() const startedAt = Date.now() diff --git a/src/main/skills/skill-discovery-stopped-wsl-roots.test.ts b/src/main/skills/skill-discovery-stopped-wsl-roots.test.ts index 60207399247..6a94be9686f 100644 --- a/src/main/skills/skill-discovery-stopped-wsl-roots.test.ts +++ b/src/main/skills/skill-discovery-stopped-wsl-roots.test.ts @@ -1,6 +1,53 @@ -import { describe, expect, it } from 'vitest' +import { describe, expect, it, vi } from 'vitest' import { WslDistroStoppedError, WslDistroUnverifiableError } from '../wsl/wsl-unc-access-gate' -import { unavailableWhileWslDistroStopped } from './skill-discovery-stopped-wsl-roots' +import { + stoppedRootReader, + stoppedWslRootsReadScopeKey, + unavailableWhileWslDistroStopped +} from './skill-discovery-stopped-wsl-roots' + +// Off Windows the real gate never sees a distro; stand in for its UNC parse. +vi.mock('../wsl/wsl-unc-access-gate', async (importOriginal) => ({ + ...(await importOriginal>()), + wslDistroForPath: (path: string) => /^\\\\wsl\.localhost\\([^\\]+)/.exec(path)?.[1] ?? null +})) + +const ubuntu = (rest: string): string => `\\\\wsl.localhost\\Ubuntu${rest}` + +describe('stoppedRootReader', () => { + it('reads a root that holds a chosen skill directory, and no other', () => { + const read = stoppedRootReader([ubuntu('\\home\\me\\.claude\\skills\\alpha')]) + expect(read(ubuntu('\\home\\me\\.claude\\skills'))).toBe(true) + expect(read(ubuntu('\\HOME\\me\\.claude\\skills\\'))).toBe(true) + expect(read(ubuntu('\\repo\\.claude\\skills'))).toBe(false) + }) + + it('never lets an empty, `/` or bare share root match every directory', () => { + const read = stoppedRootReader([ubuntu('\\home\\me\\.claude\\skills\\alpha')]) + expect(read('')).toBe(false) + expect(read('/')).toBe(false) + expect(read('\\\\')).toBe(false) + }) + + it('reads every root on the named distros, matching names case-insensitively', () => { + const read = stoppedRootReader({ distros: ['ubuntu'] }) + expect(read(ubuntu('\\home\\me\\.claude\\plugins\\x\\skills'))).toBe(true) + expect(read('\\\\wsl.localhost\\Debian\\home\\me\\.claude\\skills')).toBe(false) + expect(read('C:\\Users\\me\\.claude\\skills')).toBe(false) + }) + + it('keys each scope apart for the scan coalescer', () => { + const keys = [ + stoppedWslRootsReadScopeKey('all'), + stoppedWslRootsReadScopeKey({ distros: ['Ubuntu'] }), + stoppedWslRootsReadScopeKey(['Ubuntu']) + ] + expect(new Set(keys).size).toBe(3) + expect(stoppedWslRootsReadScopeKey({ distros: ['Ubuntu', 'Debian'] })).toBe( + stoppedWslRootsReadScopeKey({ distros: ['debian', 'ubuntu'] }) + ) + }) +}) describe('unavailableWhileWslDistroStopped', () => { it.each([new WslDistroStoppedError('Ubuntu'), new WslDistroUnverifiableError('Ubuntu')])( diff --git a/src/main/skills/skill-discovery-stopped-wsl-roots.ts b/src/main/skills/skill-discovery-stopped-wsl-roots.ts index aa38b296a95..fa7a25d13f1 100644 --- a/src/main/skills/skill-discovery-stopped-wsl-roots.ts +++ b/src/main/skills/skill-discovery-stopped-wsl-roots.ts @@ -49,6 +49,7 @@ export async function splitStoppedWslDistroRoots( } } +// Lowercasing can only widen the user-act-readable roots: Windows treats these UNC paths case-insensitively. function comparablePath(path: string): string { return path .replace(/[\\/]+/g, '/') @@ -56,16 +57,39 @@ function comparablePath(path: string): string { .toLowerCase() } -/** Whether a stopped-distro root or plugin cwd at `path` holds one of the chosen skill directories. */ +/** Stopped WSL roots a user act may read: every one, those on named distros, or those holding chosen skill directories. */ +export type StoppedWslRootsReadScope = 'all' | { distros: readonly string[] } | readonly string[] + +/** Distinguishes scans with different read scopes in the scan coalescer. */ +export function stoppedWslRootsReadScopeKey(scope: StoppedWslRootsReadScope): string { + if (scope === 'all') { + return 'all' + } + if ('distros' in scope) { + const distros = scope.distros.map((distro) => distro.toLowerCase()) + return `distros:${stablePathId([...distros].sort().join('\0'))}` + } + return stablePathId([...scope].sort().join('\0')) +} + +/** Whether a stopped-distro root or plugin cwd at `path` falls in the user act's read scope. */ export function stoppedRootReader( - readFor: 'all' | readonly string[] | undefined + readFor: StoppedWslRootsReadScope | undefined ): (path: string) => boolean { if (readFor === 'all') { return () => true } + if (readFor !== undefined && 'distros' in readFor) { + const distros = new Set(readFor.distros.map((distro) => distro.toLowerCase())) + return (path) => distros.has(wslDistroForPath(path)?.toLowerCase() ?? '') + } const directories = (readFor ?? []).map(comparablePath) return (path) => { const root = comparablePath(path) + // `/` or `\\` normalizes to '', which would prefix every directory. + if (root === '') { + return false + } return directories.some((directory) => directory === root || directory.startsWith(`${root}/`)) } } diff --git a/src/main/skills/skill-discovery-target.ts b/src/main/skills/skill-discovery-target.ts index 9b1f6b6a630..5345607e631 100644 --- a/src/main/skills/skill-discovery-target.ts +++ b/src/main/skills/skill-discovery-target.ts @@ -13,6 +13,10 @@ import { skillScanSourceKinds } from './skill-discovery-source-filter' import { getRepoExecutionHostId } from '../../shared/execution-host' import { isSkillRootUnavailableError, SkillScanCoalescer } from './skill-scan-coalescer' import { assertWslUncPathRunning } from '../wsl/wsl-unc-access-gate' +import { + stoppedWslRootsReadScopeKey, + type StoppedWslRootsReadScope +} from './skill-discovery-stopped-wsl-roots' // Cache whole WSL results to amortize process boots; native roots already cache, so only coalesce concurrent targets. const WSL_RESULT_TTL_MS = 10_000 @@ -156,17 +160,15 @@ export async function discoverSkillsOnTarget( options: { refresh?: boolean providerRootOverrides?: SkillProviderRootOverrides - /** A user act (Share) may read stopped distros' folders: 'all', or only those holding these skill directories. */ - readStoppedWslRootsFor?: 'all' | readonly string[] + /** A user act (Share) may read stopped distros' folders; see `StoppedWslRootsReadScope`. */ + readStoppedWslRootsFor?: StoppedWslRootsReadScope } = {} ): Promise { const refresh = options.refresh === true const readFor = options.readStoppedWslRootsFor const gate = readFor ? { readStoppedWslRootsFor: readFor } : {} const userActKey = - readFor === undefined - ? '' - : `:user-act:${readFor === 'all' ? 'all' : stablePathId([...readFor].sort().join('\0'))}` + readFor === undefined ? '' : `:user-act:${stoppedWslRootsReadScopeKey(readFor)}` try { const outcome = await targetScans.run( // A gated and an ungated scan of one target must not answer for each other.