fix(wsl): Share only starts the stopped distros that hold the chosen skills (#35659)

#34632 let Share read every stopped WSL distro's skill folders. The Share
dialog now sends the chosen skills' folders, and discovery reads only the
stopped roots (and workspace plugin folder) that hold them; every other
stopped root stays unread and unavailable. The skills.share RPC names skills,
not folders, so it scans gated first and reads stopped roots only when a
selector is missing and a folder went unread.
This commit is contained in:
Neil
2026-10-28 21:57:25 -07:00
committed by GitHub
parent 12bb6aeabe
commit cf15284120
14 changed files with 169 additions and 37 deletions
+9 -6
View File
@@ -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<typeof SkillDiscoveryTargetSchema>) => Promise<SkillDiscoveryResult>
): void {
/** Discovery for Share; `skillDirectories` limits which stopped WSL roots it may read. */
export type SkillShareDiscover = (
target?: z.infer<typeof SkillDiscoveryTargetSchema>,
skillDirectories?: readonly string[]
) => Promise<SkillDiscoveryResult>
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<typeof SkillDiscoveryTargetSchema>) => Promise<SkillDiscoveryResult>
discover: SkillShareDiscover
): void {
registerSharingHandlers(runtime, discover)
registerCloudInstallHandlers(runtime)
@@ -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()
+11 -5
View File
@@ -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() })
)
})
+6 -4
View File
@@ -24,12 +24,12 @@ export function registerSkillsHandlers(store: Store, runtime?: OrcaRuntimeServic
const discoverResolved = async (
resolvedTarget: Awaited<ReturnType<typeof resolveSkillDiscoveryTarget>>,
refresh: boolean,
userAct = false
readStoppedWslRootsFor?: 'all' | readonly string[]
): Promise<SkillDiscoveryResult> =>
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<SkillDiscoveryResult> => {
@@ -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<SkillDiscoveryResult> => {
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')
)
}
+41 -1
View File
@@ -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',
+21 -9
View File
@@ -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)
}
}),
@@ -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([
+10 -4
View File
@@ -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<SkillDiscoveryResult> {
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()
@@ -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<string, Promise<boolean>>()
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<boolean> {
return wslDistroForPath(path) !== null && (await blockedWslDistroForPath(path)) !== null
+9 -4
View File
@@ -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<SkillDiscoveryResult> {
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<TargetScanObservation> => {
if (target.kind === 'wsl') {
+1
View File
@@ -46,6 +46,7 @@ export type SkillsApi = {
bundledExecute: (request: BundledSkillExecuteRequest) => Promise<BundledSkillInstallResult>
prepareShare: (input: {
skillIds: string[]
skillDirectories?: string[]
bundleName: string
target?: SkillDiscoveryTarget
packageId?: string
+1
View File
@@ -49,6 +49,7 @@ export const skillsApi = {
ipcRenderer.invoke('skills:bundledExecute', request),
prepareShare: (input: {
skillIds: string[]
skillDirectories?: string[]
bundleName: string
target?: SkillDiscoveryTarget
packageId?: string
@@ -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'
})
@@ -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 } : {})
})