diff --git a/src/main/ipc/skill-cloud-ipc-handlers.ts b/src/main/ipc/skill-cloud-ipc-handlers.ts index 06924e71aa6..fc2b46115ec 100644 --- a/src/main/ipc/skill-cloud-ipc-handlers.ts +++ b/src/main/ipc/skill-cloud-ipc-handlers.ts @@ -37,10 +37,13 @@ import { } from './skill-share-publishing-ipc-schemas' import { getRuntimeEnvironmentRegistryRoot } from './runtime-environments/runtime-environment-registry-root' -function registerSharingHandlers( - runtime: OrcaRuntimeService, - discover: (target?: z.infer) => Promise -): void { +/** Discovery for Share; `skillDirectories` limits which stopped WSL roots it may read. */ +export type SkillShareDiscover = ( + target?: z.infer, + skillDirectories?: readonly string[] +) => Promise + +function registerSharingHandlers(runtime: OrcaRuntimeService, discover: SkillShareDiscover): void { const preparations = new SkillSharePreparationService( join(app.getPath('userData'), 'skill-share-preparations'), { @@ -52,7 +55,7 @@ function registerSharingHandlers( ) handleMainWindowSkillIpc('skills:prepareShare', async (_event, value: unknown) => { const input = skillSharePrepareIpcSchema.parse(value) - const result = await discover(input.target) + const result = await discover(input.target, input.skillDirectories) const requested = new Set(input.skillIds) const skills = result.skills.filter((candidate) => requested.has(candidate.id)) if (skills.length !== requested.size) { @@ -289,7 +292,7 @@ function registerCloudInstallHandlers(runtime: OrcaRuntimeService): void { export function registerSkillCloudIpcHandlers( runtime: OrcaRuntimeService, - discover: (target?: z.infer) => Promise + discover: SkillShareDiscover ): void { registerSharingHandlers(runtime, discover) registerCloudInstallHandlers(runtime) diff --git a/src/main/ipc/skill-share-publishing-ipc-schemas.ts b/src/main/ipc/skill-share-publishing-ipc-schemas.ts index 7caf1f8b4ac..7f2a8b72e23 100644 --- a/src/main/ipc/skill-share-publishing-ipc-schemas.ts +++ b/src/main/ipc/skill-share-publishing-ipc-schemas.ts @@ -4,6 +4,8 @@ import { SkillDiscoveryTargetSchema } from '../../shared/skills' export const skillSharePrepareIpcSchema = z .object({ skillIds: z.array(z.string().min(1).max(4096)).min(1).max(512), + /** The chosen skills' folders: only their roots may start a stopped WSL distro. */ + skillDirectories: z.array(z.string().min(1).max(4096)).max(512).optional(), bundleName: z.string().regex(/^(?!.*(?:--|\.\.))[a-z0-9](?:[a-z0-9.-]{0,62}[a-z0-9])?$/), target: SkillDiscoveryTargetSchema.optional(), packageId: z.string().min(1).max(128).optional() diff --git a/src/main/ipc/skills.test.ts b/src/main/ipc/skills.test.ts index aeed666f93b..a818336f250 100644 --- a/src/main/ipc/skills.test.ts +++ b/src/main/ipc/skills.test.ts @@ -230,14 +230,20 @@ describe('registerSkillsHandlers', () => { expect(discoverSkillObservationInWslMock).toHaveBeenCalled() }) - it('lets the Share dialog, which sends no target, read a stopped distro’s repo skills', async () => { + it('lets the Share dialog, which sends no target, read only its chosen skills’ stopped roots', async () => { registerSkillsHandlers(store as never, bundledRuntime() as never) // oxlint-disable-line typescript/consistent-type-assertions -- SAFETY: registration reads only getRepos and the runtime's provider roots. const shareDiscover = registerSkillCloudIpcHandlersMock.mock.lastCall?.[1] + const chosen = ['\\\\wsl.localhost\\Ubuntu\\home\\me\\repo\\.agents\\skills\\a'] - await shareDiscover(undefined) - + await shareDiscover(undefined, chosen) expect(discoverSkillsMock).toHaveBeenLastCalledWith( - expect.objectContaining({ repos, gateStoppedWslDistros: false }) + expect.objectContaining({ repos, readStoppedWslRootsFor: chosen }) + ) + + // A caller that cannot name folders keeps the earlier behavior. + await shareDiscover(undefined) + expect(discoverSkillsMock).toHaveBeenLastCalledWith( + expect.objectContaining({ repos, readStoppedWslRootsFor: 'all' }) ) }) @@ -245,7 +251,7 @@ describe('registerSkillsHandlers', () => { const handler = getDiscoverHandler() await handler(null, undefined) expect(discoverSkillsMock).toHaveBeenLastCalledWith( - expect.not.objectContaining({ gateStoppedWslDistros: false }) + expect.not.objectContaining({ readStoppedWslRootsFor: expect.anything() }) ) }) diff --git a/src/main/ipc/skills.ts b/src/main/ipc/skills.ts index 06fd894fa49..31bf4d87614 100644 --- a/src/main/ipc/skills.ts +++ b/src/main/ipc/skills.ts @@ -24,12 +24,12 @@ export function registerSkillsHandlers(store: Store, runtime?: OrcaRuntimeServic const discoverResolved = async ( resolvedTarget: Awaited>, refresh: boolean, - userAct = false + readStoppedWslRootsFor?: 'all' | readonly string[] ): Promise => discoverSkillsOnTarget(resolvedTarget, store.getRepos(), { providerRootOverrides: await runtime?.resolveSkillDiscoveryProviderRoots(resolvedTarget), refresh, - ...(userAct ? { gateStoppedWslDistros: false } : {}) + ...(readStoppedWslRootsFor ? { readStoppedWslRootsFor } : {}) }) // Passive UI mounts discovery, so it must not start a stopped WSL distro. const discover = async (target?: SkillDiscoveryTarget): Promise => { @@ -45,7 +45,8 @@ export function registerSkillsHandlers(store: Store, runtime?: OrcaRuntimeServic } // Share is a click on named skills: it may start their distro, as any user act may. const discoverForUserAct = async ( - target?: SkillDiscoveryTarget + target?: SkillDiscoveryTarget, + skillDirectories?: readonly string[] ): Promise => { const parsedTarget = target ? SkillDiscoveryTargetSchema.parse(target) : undefined const resolvedTarget = await resolveSkillDiscoveryTarget(parsedTarget) @@ -56,7 +57,8 @@ export function registerSkillsHandlers(store: Store, runtime?: OrcaRuntimeServic ? wslDistroForPath(resolvedTarget.cwd) : null return runAsWslUserAct(distro, () => - discoverResolved(resolvedTarget, parsedTarget?.refresh === true, true) + // Only the chosen skills' roots may start a stopped distro; without folders, any may. + discoverResolved(resolvedTarget, parsedTarget?.refresh === true, skillDirectories ?? 'all') ) } diff --git a/src/main/runtime/rpc/methods/skills.test.ts b/src/main/runtime/rpc/methods/skills.test.ts index 507ee86a2d2..43546d2bb9a 100644 --- a/src/main/runtime/rpc/methods/skills.test.ts +++ b/src/main/runtime/rpc/methods/skills.test.ts @@ -338,11 +338,51 @@ describe('skills.share RPC', () => { expect(discoverSkillsOnTarget).toHaveBeenCalledWith( expect.objectContaining({ kind: 'native-host', cwd: '/repo' }), [], - { providerRootOverrides: {}, refresh: true, gateStoppedWslDistros: false } + { providerRootOverrides: {}, refresh: true } ) + // Every selector was found without starting any stopped distro. + expect(discoverSkillsOnTarget).toHaveBeenCalledTimes(1) 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' } + vi.mocked(discoverSkillsOnTarget) + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the handler reads only skill ids and source skippedReason. + .mockResolvedValueOnce({ + skills: [{ id: 'alpha' }], + sources: [stoppedSource], + scannedAt: 1 + } as never) + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the handler reads only skill ids and source skippedReason. + .mockResolvedValueOnce({ + skills: [{ id: 'alpha' }, { id: 'beta' }], + sources: [], + scannedAt: 2 + } as never) + const publishDiscoveredSkillsFromAgent = vi.fn(async () => ({ status: 'ok' })) + const runtime = { + assertAgentSkillSharingAllowed: vi.fn(), + resolveProjectRuntimeForWorktree: vi.fn(), + resolveSkillDiscoveryProviderRoots: vi.fn(async () => ({})), + listRepos: vi.fn(() => []), + publishDiscoveredSkillsFromAgent + } + + // 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(vi.mocked(discoverSkillsOnTarget).mock.calls.map((call) => call[2])).toEqual([ + { providerRootOverrides: {}, refresh: true }, + { providerRootOverrides: {}, refresh: true, readStoppedWslRootsFor: 'all' } + ]) + expect(publishDiscoveredSkillsFromAgent).toHaveBeenCalledWith( + request, + [{ id: 'alpha' }, { id: 'beta' }], + undefined + ) + }) + 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 3dbafd5a51a..a5bf89b2c67 100644 --- a/src/main/runtime/rpc/methods/skills.ts +++ b/src/main/runtime/rpc/methods/skills.ts @@ -160,15 +160,27 @@ export const SKILL_METHODS = [ ) } // Sharing named skills is a user act: it may start the workspace's distro. - const discovered = await runAsWslUserAct( - resolvedTarget.cwd ? wslDistroForPath(resolvedTarget.cwd) : null, - async () => - discoverSkillsOnTarget(resolvedTarget, runtime.listRepos(), { - providerRootOverrides: await runtime.resolveSkillDiscoveryProviderRoots(resolvedTarget), - refresh: true, - gateStoppedWslDistros: false - }) - ) + const discover = async (readStoppedWslRootsFor?: 'all') => + runAsWslUserAct( + resolvedTarget.cwd ? wslDistroForPath(resolvedTarget.cwd) : null, + async () => + discoverSkillsOnTarget(resolvedTarget, runtime.listRepos(), { + providerRootOverrides: + await runtime.resolveSkillDiscoveryProviderRoots(resolvedTarget), + refresh: true, + ...(readStoppedWslRootsFor ? { readStoppedWslRootsFor } : {}) + }) + ) + // Selectors name skills, not folders: start other stopped distros only when a selector is missing. + let discovered = await discover() + if ( + !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') + } return runtime.publishDiscoveredSkillsFromAgent(params, discovered.skills, signal) } }), diff --git a/src/main/skills/discovery-wsl-stopped-distro.test.ts b/src/main/skills/discovery-wsl-stopped-distro.test.ts index fbf6e01e46f..d6ed889c12e 100644 --- a/src/main/skills/discovery-wsl-stopped-distro.test.ts +++ b/src/main/skills/discovery-wsl-stopped-distro.test.ts @@ -132,13 +132,39 @@ describe('skill discovery for a stopped WSL distro (E9)', () => { ) }) - it('lets a user act (Share) read a stopped distro’s repo folders and find its skill', async () => { + it('lets Share read only the stopped roots holding the chosen skills, leaving the rest down', async () => { + const chosen = join(mocks.wslRepoRoot, '.agents', 'skills', 'repo-skill') const result = await discoverSkills({ homeDir: join(root, 'home'), repos: [repo('wsl', mocks.wslRepoRoot), repo('host', hostRepo)], includeCwd: false, sourceKinds: ['repo'], - gateStoppedWslDistros: false + readStoppedWslRootsFor: [chosen] + }) + expect(result.skills.map((skill) => skill.name).sort()).toEqual([ + 'Skill in host', + 'Skill in wsl' + ]) + const chosenRoot = join(mocks.wslRepoRoot, '.agents', 'skills') + // The chosen skill's root is read; the distro's other roots stay unread and unavailable. + const otherWslTouches = mocks.touched.filter( + (path) => path.startsWith(mocks.wslRepoRoot) && !path.startsWith(chosenRoot) + ) + expect(otherWslTouches).toEqual([]) + const otherWslSources = result.sources.filter( + (source) => source.path.startsWith(mocks.wslRepoRoot) && source.path !== chosenRoot + ) + expect(otherWslSources.length).toBeGreaterThan(0) + expect(otherWslSources.every((source) => source.skippedReason === 'unavailable')).toBe(true) + }) + + it("lets Share read every stopped root when it can't name the skills' folders", async () => { + const result = await discoverSkills({ + homeDir: join(root, 'home'), + repos: [repo('wsl', mocks.wslRepoRoot), repo('host', hostRepo)], + includeCwd: false, + sourceKinds: ['repo'], + readStoppedWslRootsFor: 'all' }) expect(mocks.blockedWslDistroForPath).not.toHaveBeenCalled() expect(result.skills.map((skill) => skill.name).sort()).toEqual([ diff --git a/src/main/skills/discovery.ts b/src/main/skills/discovery.ts index a97f66c28c8..ad9e05be752 100644 --- a/src/main/skills/discovery.ts +++ b/src/main/skills/discovery.ts @@ -24,6 +24,7 @@ import { isOnStoppedWslDistro, splitStoppedWslDistroRoots, stoppedPluginRoot, + stoppedRootReader, unavailableSkillSource } from './skill-discovery-stopped-wsl-roots' import { wslDistroForPath } from '../wsl/wsl-unc-access-gate' @@ -234,8 +235,11 @@ export async function discoverSkills(args: { refresh?: boolean names?: string[] sourceKinds?: SkillSourceKind[] - /** False for a user act (Share): read stopped WSL distros' folders instead of reporting them unavailable. */ - gateStoppedWslDistros?: boolean + /** + * 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. + */ + readStoppedWslRootsFor?: 'all' | readonly string[] }): Promise { args.signal?.throwIfAborted() const startedAt = Date.now() @@ -248,10 +252,12 @@ export async function discoverSkills(args: { (!args.sourceKinds?.length || args.sourceKinds.includes('plugin')) ? args.cwd : undefined - const gateStopped = args.gateStoppedWslDistros !== false + const gateStopped = args.readStoppedWslRootsFor !== 'all' + const mayReadStopped = stoppedRootReader(args.readStoppedWslRootsFor) const pluginCwdStopped = gateStopped && pluginCwd !== undefined && + !mayReadStopped(pluginCwd) && wslDistroForPath(pluginCwd) !== null && (await isOnStoppedWslDistro(pluginCwd)) const candidateRoots = [ @@ -266,7 +272,7 @@ export async function discoverSkills(args: { // Host-only root sets never await the WSL gate. const { readable, unavailable } = gateStopped && candidateRoots.some((root) => wslDistroForPath(root.path)) - ? await splitStoppedWslDistroRoots(candidateRoots) + ? await splitStoppedWslDistroRoots(candidateRoots, mayReadStopped) : { readable: candidateRoots, unavailable: [] } const roots = readable.filter((root) => rootMayContainSourceKind(root, args.sourceKinds)) args.signal?.throwIfAborted() diff --git a/src/main/skills/skill-discovery-stopped-wsl-roots.ts b/src/main/skills/skill-discovery-stopped-wsl-roots.ts index 0e210fd9a8e..aa38b296a95 100644 --- a/src/main/skills/skill-discovery-stopped-wsl-roots.ts +++ b/src/main/skills/skill-discovery-stopped-wsl-roots.ts @@ -23,13 +23,15 @@ export function unavailableSkillSource(root: SkillScanRoot): SkillDiscoverySourc * it, and passive UI runs this scan. The caller reports them unavailable, so the next scan reads them. */ export async function splitStoppedWslDistroRoots( - roots: readonly SkillScanRoot[] + roots: readonly SkillScanRoot[], + /** Roots a user act may read even on a stopped distro; they never ask the gate. */ + mayReadStopped: (path: string) => boolean = () => false ): Promise<{ readable: SkillScanRoot[]; unavailable: SkillScanRoot[] }> { const blockedByDistro = new Map>() const blocked = await Promise.all( roots.map(async (root) => { const distro = wslDistroForPath(root.path) - if (!distro) { + if (!distro || mayReadStopped(root.path)) { return false } const key = distro.toLowerCase() @@ -47,6 +49,27 @@ export async function splitStoppedWslDistroRoots( } } +function comparablePath(path: string): string { + return path + .replace(/[\\/]+/g, '/') + .replace(/\/$/, '') + .toLowerCase() +} + +/** Whether a stopped-distro root or plugin cwd at `path` holds one of the chosen skill directories. */ +export function stoppedRootReader( + readFor: 'all' | readonly string[] | undefined +): (path: string) => boolean { + if (readFor === 'all') { + return () => true + } + const directories = (readFor ?? []).map(comparablePath) + return (path) => { + const root = comparablePath(path) + return directories.some((directory) => directory === root || directory.startsWith(`${root}/`)) + } +} + /** Whether reading `path` would start a stopped (or unverifiable) WSL distro. */ export async function isOnStoppedWslDistro(path: string): Promise { return wslDistroForPath(path) !== null && (await blockedWslDistroForPath(path)) !== null diff --git a/src/main/skills/skill-discovery-target.ts b/src/main/skills/skill-discovery-target.ts index 4fd28d78c3f..9b1f6b6a630 100644 --- a/src/main/skills/skill-discovery-target.ts +++ b/src/main/skills/skill-discovery-target.ts @@ -156,16 +156,21 @@ export async function discoverSkillsOnTarget( options: { refresh?: boolean providerRootOverrides?: SkillProviderRootOverrides - /** False for a user act (Share), which may read a stopped distro's folders. */ - gateStoppedWslDistros?: boolean + /** A user act (Share) may read stopped distros' folders: 'all', or only those holding these skill directories. */ + readStoppedWslRootsFor?: 'all' | readonly string[] } = {} ): Promise { const refresh = options.refresh === true - const gate = options.gateStoppedWslDistros === false ? { gateStoppedWslDistros: false } : {} + const readFor = options.readStoppedWslRootsFor + const gate = readFor ? { readStoppedWslRootsFor: readFor } : {} + const userActKey = + readFor === undefined + ? '' + : `:user-act:${readFor === 'all' ? 'all' : stablePathId([...readFor].sort().join('\0'))}` try { const outcome = await targetScans.run( // A gated and an ungated scan of one target must not answer for each other. - `${scanKey(target, repos, options.providerRootOverrides)}${options.gateStoppedWslDistros === false ? ':user-act' : ''}`, + `${scanKey(target, repos, options.providerRootOverrides)}${userActKey}`, { ttlMs: target.kind === 'wsl' ? WSL_RESULT_TTL_MS : 0, refresh }, async (): Promise => { if (target.kind === 'wsl') { diff --git a/src/preload/api/agent-skill-api.ts b/src/preload/api/agent-skill-api.ts index 66611000238..36a5ba17099 100644 --- a/src/preload/api/agent-skill-api.ts +++ b/src/preload/api/agent-skill-api.ts @@ -46,6 +46,7 @@ export type SkillsApi = { bundledExecute: (request: BundledSkillExecuteRequest) => Promise prepareShare: (input: { skillIds: string[] + skillDirectories?: string[] bundleName: string target?: SkillDiscoveryTarget packageId?: string diff --git a/src/preload/api/skills-bridge.ts b/src/preload/api/skills-bridge.ts index d9d6d92dd16..d7cc98cd232 100644 --- a/src/preload/api/skills-bridge.ts +++ b/src/preload/api/skills-bridge.ts @@ -49,6 +49,7 @@ export const skillsApi = { ipcRenderer.invoke('skills:bundledExecute', request), prepareShare: (input: { skillIds: string[] + skillDirectories?: string[] bundleName: string target?: SkillDiscoveryTarget packageId?: string diff --git a/src/renderer/src/components/skills/SkillShareDialog.test.tsx b/src/renderer/src/components/skills/SkillShareDialog.test.tsx index 78e4684e91a..a6a0b23ccaf 100644 --- a/src/renderer/src/components/skills/SkillShareDialog.test.tsx +++ b/src/renderer/src/components/skills/SkillShareDialog.test.tsx @@ -245,6 +245,7 @@ describe('SkillShareDialog', () => { expect(screen.getByRole('button', { name: 'Publish new version' })).toBeTruthy() expect(skills.prepareShare).toHaveBeenCalledWith({ skillIds: [skill.id], + skillDirectories: [skill.directoryPath], bundleName: skill.name, packageId: 'pkg_1' }) @@ -260,6 +261,7 @@ describe('SkillShareDialog', () => { await waitFor(() => expect(skills.prepareShare).toHaveBeenCalledWith({ skillIds: [skill.id], + skillDirectories: [skill.directoryPath], bundleName: skill.name }) ) @@ -280,6 +282,7 @@ describe('SkillShareDialog', () => { expect(screen.getByRole('button', { name: 'Publish new version' })).toBeTruthy() expect(skills.prepareShare).toHaveBeenCalledWith({ skillIds: [skill.id, secondSkill.id], + skillDirectories: [skill.directoryPath, secondSkill.directoryPath], bundleName: 'private-skill-and-1-more', packageId: 'pkg_1' }) diff --git a/src/renderer/src/components/skills/SkillShareDialog.tsx b/src/renderer/src/components/skills/SkillShareDialog.tsx index 4e2f739f37b..6686f43f319 100644 --- a/src/renderer/src/components/skills/SkillShareDialog.tsx +++ b/src/renderer/src/components/skills/SkillShareDialog.tsx @@ -102,6 +102,8 @@ export function SkillShareDialog({ } const nextPreview = await window.api.skills.prepareShare({ skillIds: selectedSkills.map((skill) => skill.id), + // Only these folders' roots may start a stopped WSL distro. + skillDirectories: selectedSkills.map((skill) => skill.directoryPath), bundleName, ...(managedInstall ? { packageId: managedInstall.packageId } : {}) })