From 16d1ab81d3fd08e342b2f0ac0cc6f9e4aea4aaee Mon Sep 17 00:00:00 2001 From: PPP-JH <62415640+PPP-JH@users.noreply.github.com> Date: Mon, 14 Sep 2026 11:16:42 +0900 Subject: [PATCH] perf(skills): bound WSL installed skill discovery (#12314) * perf(skills): bound WSL installed skill discovery * fix(skills): preserve bounded discovery correctness * fix(skills): bound WSL metadata prefilter reads * fix(skills): isolate absent discovery cwd cache keys * test(skills): adapt WSL discovery mocks to runner * fix(skills): preserve filtered discovery fallbacks * fix(skills): share filtered scans and preserve WSL inventory * fix(skills): share WSL scans without losing skill aliases * fix: preserve skill metadata and retire filtered peer caches --------- Co-authored-by: m4air --- src/main/ipc/skills.test.ts | 41 ++- src/main/runtime/rpc/methods/skills.test.ts | 19 ++ .../skills/discovery-filter-sharing.test.ts | 143 ++++++++++ .../discovery-source-filter-order.test.ts | 45 ++++ src/main/skills/discovery.test.ts | 40 +++ src/main/skills/discovery.ts | 24 +- src/main/skills/skill-delete/roots.test.ts | 30 +++ src/main/skills/skill-delete/roots.ts | 3 +- .../skill-discovery-source-filter.test.ts | 24 ++ .../skills/skill-discovery-source-filter.ts | 29 ++ .../skills/skill-discovery-target.test.ts | 20 +- src/main/skills/skill-discovery-target.ts | 108 ++++++-- .../skill-discovery-wsl-alias-sharing.test.ts | 145 ++++++++++ .../skill-discovery-wsl-bash-filter.test.ts | 89 +++++++ .../skills/skill-discovery-wsl-observation.ts | 165 ++++++++++++ .../skill-discovery-wsl-plugins.test.ts | 172 ++++++++---- src/main/skills/skill-discovery-wsl.test.ts | 82 ++++++ src/main/skills/skill-discovery-wsl.ts | 250 +++++++++--------- .../installed-agent-skill-discovery-cache.ts | 8 +- .../installed-agent-skill-discovery.test.ts | 41 +++ .../hooks/installed-agent-skill-discovery.ts | 102 +++++-- ...alled-agent-skill-runtime-eviction.test.ts | 74 ++++++ .../useInstalledAgentSkills.react.test.tsx | 39 ++- .../src/hooks/useInstalledAgentSkills.test.ts | 38 +++ .../src/hooks/useInstalledAgentSkills.ts | 15 +- .../src/runtime/runtime-skills-client.test.ts | 22 ++ .../src/runtime/runtime-skills-client.ts | 7 +- src/shared/skill-metadata.test.ts | 13 + src/shared/skill-metadata.ts | 2 +- src/shared/skills.ts | 10 +- 30 files changed, 1550 insertions(+), 250 deletions(-) create mode 100644 src/main/skills/discovery-filter-sharing.test.ts create mode 100644 src/main/skills/discovery-source-filter-order.test.ts create mode 100644 src/main/skills/skill-delete/roots.test.ts create mode 100644 src/main/skills/skill-discovery-source-filter.test.ts create mode 100644 src/main/skills/skill-discovery-source-filter.ts create mode 100644 src/main/skills/skill-discovery-wsl-alias-sharing.test.ts create mode 100644 src/main/skills/skill-discovery-wsl-bash-filter.test.ts create mode 100644 src/main/skills/skill-discovery-wsl-observation.ts create mode 100644 src/renderer/src/hooks/installed-agent-skill-runtime-eviction.test.ts diff --git a/src/main/ipc/skills.test.ts b/src/main/ipc/skills.test.ts index e14c716f9cc..98cd54c384f 100644 --- a/src/main/ipc/skills.test.ts +++ b/src/main/ipc/skills.test.ts @@ -3,7 +3,7 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' const { handleMock, discoverSkillsMock, - discoverSkillsInWslMock, + discoverSkillObservationInWslMock, inventorySkillFreshnessMock, getDefaultWslDistroMock, getWslHomeMock, @@ -11,7 +11,7 @@ const { } = vi.hoisted(() => ({ handleMock: vi.fn(), discoverSkillsMock: vi.fn(), - discoverSkillsInWslMock: vi.fn(), + discoverSkillObservationInWslMock: vi.fn(), inventorySkillFreshnessMock: vi.fn(), getDefaultWslDistroMock: vi.fn(), getWslHomeMock: vi.fn(), @@ -37,7 +37,7 @@ vi.mock('../skills/discovery', () => ({ })) vi.mock('../skills/skill-discovery-wsl', () => ({ - discoverSkillsInWsl: discoverSkillsInWslMock + discoverSkillObservationInWsl: discoverSkillObservationInWslMock })) vi.mock('../skills/skill-freshness-inventory', () => ({ @@ -60,6 +60,7 @@ vi.mock('../wsl', () => ({ })) import { registerSkillsHandlers } from './skills' +import { clearSkillDiscoveryCaches } from '../skills/skill-discovery-target' describe('registerSkillsHandlers', () => { const originalPlatform = Object.getOwnPropertyDescriptor(process, 'platform') @@ -69,15 +70,16 @@ describe('registerSkillsHandlers', () => { } beforeEach(() => { + clearSkillDiscoveryCaches() handleMock.mockReset() discoverSkillsMock.mockReset() - discoverSkillsInWslMock.mockReset() + discoverSkillObservationInWslMock.mockReset() getDefaultWslDistroMock.mockReset() getWslHomeMock.mockReset() parseWslPathMock.mockReset() parseWslPathMock.mockReturnValue(null) discoverSkillsMock.mockResolvedValue({ skills: [], sources: [], scannedAt: 1 }) - discoverSkillsInWslMock.mockResolvedValue({ skills: [], sources: [], scannedAt: 1 }) + discoverSkillObservationInWslMock.mockResolvedValue({ rows: [], sources: [], scannedAt: 1 }) inventorySkillFreshnessMock.mockResolvedValue({ schemaVersion: 1, installations: [], @@ -171,10 +173,30 @@ describe('registerSkillsHandlers', () => { expect(getDefaultWslDistroMock).not.toHaveBeenCalled() expect(getWslHomeMock).toHaveBeenCalledWith('Ubuntu') - expect(discoverSkillsInWslMock).toHaveBeenCalledWith({ + expect(discoverSkillObservationInWslMock).toHaveBeenCalledWith({ distro: 'Ubuntu', homeDir: '/home/alice', - cwd: '/home/alice' + sourceKinds: undefined + }) + }) + + it('shares the home and bundled WSL scan across name-filtered requests', async () => { + const handler = getDiscoverHandler() + + for (const name of ['orchestration', 'linear-tickets']) { + await handler(null, { + runtime: 'wsl', + wslDistro: 'Ubuntu', + names: [name], + sourceKinds: ['home'] + }) + } + + expect(discoverSkillObservationInWslMock).toHaveBeenCalledOnce() + expect(discoverSkillObservationInWslMock).toHaveBeenCalledWith({ + distro: 'Ubuntu', + homeDir: '/home/alice', + sourceKinds: ['bundled', 'home'] }) }) @@ -196,10 +218,11 @@ describe('registerSkillsHandlers', () => { } }) - expect(discoverSkillsInWslMock).toHaveBeenCalledWith({ + expect(discoverSkillObservationInWslMock).toHaveBeenCalledWith({ distro: 'Ubuntu', homeDir: '/home/alice', - cwd: '/mnt/c/repo/worktree' + cwd: '/mnt/c/repo/worktree', + sourceKinds: undefined }) }) diff --git a/src/main/runtime/rpc/methods/skills.test.ts b/src/main/runtime/rpc/methods/skills.test.ts index 9bc5a647c82..097a948febf 100644 --- a/src/main/runtime/rpc/methods/skills.test.ts +++ b/src/main/runtime/rpc/methods/skills.test.ts @@ -106,6 +106,25 @@ describe('skills.discover RPC', () => { it('accepts a params payload from an older client that cannot send refresh', () => { expect(discoverMethod().params?.parse({ cwd: '/repo' })).toEqual({ cwd: '/repo' }) }) + + it('preserves portable filters through the server RPC boundary', async () => { + await discoverMethod().handler( + { names: ['orchestration'], sourceKinds: ['home'] }, + makeContext({}) + ) + + expect(vi.mocked(resolveSkillDiscoveryTarget)).toHaveBeenLastCalledWith( + expect.objectContaining({ names: ['orchestration'], sourceKinds: ['home'] }) + ) + }) + + it('accepts empty portable filters as an unbounded request', async () => { + await discoverMethod().handler({ names: [], sourceKinds: [] }, makeContext({})) + + expect(vi.mocked(resolveSkillDiscoveryTarget)).toHaveBeenLastCalledWith( + expect.objectContaining({ names: [], sourceKinds: [] }) + ) + }) }) describe('skills.install RPC', () => { diff --git a/src/main/skills/discovery-filter-sharing.test.ts b/src/main/skills/discovery-filter-sharing.test.ts new file mode 100644 index 00000000000..14f755d0100 --- /dev/null +++ b/src/main/skills/discovery-filter-sharing.test.ts @@ -0,0 +1,143 @@ +import { mkdir, mkdtemp, rm, symlink, writeFile } from 'node:fs/promises' +import type * as FsPromises from 'node:fs/promises' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { afterEach, expect, it, vi } from 'vitest' + +const observed = vi.hoisted(() => ({ opens: 0 })) +vi.mock('node:fs/promises', async (importOriginal) => { + const actual = await importOriginal() + return { + ...actual, + open: (...args: Parameters) => { + observed.opens += 1 + return actual.open(...args) + } + } +}) +import * as repair from './discovery' +import { SkillScanCoalescer, SkillScanShedError } from './skill-scan-coalescer' + +afterEach(() => { + repair.clearSkillRootScanCache() + vi.restoreAllMocks() + vi.unstubAllEnvs() +}) + +async function fixture(task: (root: string) => Promise): Promise { + const root = await mkdtemp(join(tmpdir(), 'orca-skill-name-repair-')) + vi.stubEnv('HERMES_HOME', '') + vi.stubEnv('LOCALAPPDATA', '') + try { + for (let index = 0; index < 48; index += 1) { + const dir = join(root, '.agents', 'skills', `skill-${index}`) + await mkdir(dir, { recursive: true }) + await writeFile(join(dir, 'SKILL.md'), `---\nname: skill-${index}\n---\n`) + } + await task(root) + } finally { + await rm(root, { recursive: true, force: true }) + } +} + +it('shares one root read across six concurrent name filters', async () => { + await fixture(async (root) => { + repair.clearSkillRootScanCache() + observed.opens = 0 + const checks = await Promise.all( + Array.from({ length: 6 }, (_, index) => + repair.discoverSkills({ + homeDir: root, + repos: [], + includeCwd: false, + names: [`skill-${index}`], + sourceKinds: ['home'] + }) + ) + ) + expect(checks.map((result) => result.skills.map((skill) => skill.name))).toEqual( + Array.from({ length: 6 }, (_, index) => [`skill-${index}`]) + ) + expect(observed.opens).toBe(48) + }) +}) + +it('reuses the raw snapshot for a new name and invalidates it on mutation', async () => { + await fixture(async (root) => { + const args = { homeDir: root, repos: [], includeCwd: false, sourceKinds: ['home' as const] } + observed.opens = 0 + await repair.discoverSkills({ ...args, names: ['skill-0'] }) + const next = await repair.discoverSkills({ ...args, names: ['skill-47'] }) + expect(next.skills.map((skill) => skill.name)).toEqual(['skill-47']) + expect(observed.opens).toBe(48) + await writeFile( + join(root, '.agents', 'skills', 'skill-47', 'SKILL.md'), + '---\nname: renamed\n---\n' + ) + repair.clearSkillRootScanCache() + const updated = await repair.discoverSkills({ ...args, names: ['renamed'] }) + expect(updated.skills.map((skill) => skill.name)).toEqual(['renamed']) + expect(observed.opens).toBe(96) + }) +}) + +it('retains a newly requested name when its previously observed root becomes unavailable', async () => { + await fixture(async (root) => { + const args = { homeDir: root, repos: [], includeCwd: false, sourceKinds: ['home' as const] } + await repair.discoverSkills({ ...args, names: ['skill-0'] }) + const original = SkillScanCoalescer.prototype.run + vi.spyOn(SkillScanCoalescer.prototype, 'run').mockImplementation( + function (this: SkillScanCoalescer, key, options, task) { + if ( + key === `home\0${join(root, '.agents', 'skills')}` || + key.startsWith(`home\0${join(root, '.agents', 'skills')}\0`) + ) { + return Promise.reject(new SkillScanShedError()) + } + return original.call(this, key, options, task) + } + ) + const next = await repair.discoverSkills({ ...args, names: ['skill-47'] }) + expect(next.skills.map((skill) => skill.name)).toEqual(['skill-47']) + expect(next.sources.find((source) => source.id === 'home-agents')?.skippedReason).toBe( + 'unavailable' + ) + }) +}) + +it('keeps simultaneous forced refreshes independent', async () => { + await fixture(async (root) => { + const args = { homeDir: root, repos: [], includeCwd: false, sourceKinds: ['home' as const] } + await repair.discoverSkills({ ...args, names: ['skill-0'] }) + observed.opens = 0 + const results = await Promise.all( + [0, 1].map((index) => + repair.discoverSkills({ ...args, names: [`skill-${index}`], refresh: true }) + ) + ) + expect(results.map((result) => result.skills[0]?.name)).toEqual(['skill-0', 'skill-1']) + expect(observed.opens).toBe(96) + }) +}) + +it('filters aliases before deduplication so excluded bundled roots cannot own home results', async () => { + await fixture(async (root) => { + const bundled = join(root, '.codex', 'skills', '.system', 'bundle') + const alias = join(root, '.agents', 'skills', 'bundle-alias') + await mkdir(bundled, { recursive: true }) + await writeFile(join(bundled, 'SKILL.md'), '---\nname: bundle\n---\n') + await symlink(bundled, alias, 'dir') + const result = await repair.discoverSkills({ + homeDir: root, + repos: [], + includeCwd: false, + names: ['bundle'], + sourceKinds: ['home'] + }) + expect(result.skills).toHaveLength(1) + expect(result.skills[0]).toMatchObject({ + sourceKind: 'home', + rootPath: join(root, '.agents', 'skills') + }) + }) +}) diff --git a/src/main/skills/discovery-source-filter-order.test.ts b/src/main/skills/discovery-source-filter-order.test.ts new file mode 100644 index 00000000000..991fa457853 --- /dev/null +++ b/src/main/skills/discovery-source-filter-order.test.ts @@ -0,0 +1,45 @@ +import { mkdir, mkdtemp, rm, writeFile } from 'node:fs/promises' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { describe, expect, it, vi } from 'vitest' +import type * as SkillMetadata from '../../shared/skill-metadata' + +const summarizeSkillMarkdown = vi.hoisted(() => vi.fn()) + +vi.mock('../../shared/skill-metadata', async (importOriginal) => { + const original = await importOriginal() + return { + ...original, + summarizeSkillMarkdown: (markdown: string) => { + summarizeSkillMarkdown(markdown) + return original.summarizeSkillMarkdown(markdown) + } + } +}) + +import { discoverSkills } from './discovery' + +describe('native skill source filtering', () => { + it('serves home and bundled filters from one raw root observation', async () => { + const root = await mkdtemp(join(tmpdir(), 'orca-skill-source-filter-')) + const homeSkill = join(root, '.codex', 'skills', 'home-skill') + const bundledSkill = join(root, '.codex', 'skills', '.system', 'bundled-skill') + await mkdir(homeSkill, { recursive: true }) + await mkdir(bundledSkill, { recursive: true }) + await writeFile(join(homeSkill, 'SKILL.md'), '# Home Skill\n') + await writeFile(join(bundledSkill, 'SKILL.md'), '# Bundled Skill\n') + + try { + const result = await discoverSkills({ homeDir: root, repos: [], sourceKinds: ['home'] }) + + expect(result.skills.map((skill) => skill.name)).toEqual(['Home Skill']) + expect(summarizeSkillMarkdown).toHaveBeenCalledTimes(2) + expect(summarizeSkillMarkdown).toHaveBeenCalledWith('# Home Skill\n') + const bundled = await discoverSkills({ homeDir: root, repos: [], sourceKinds: ['bundled'] }) + expect(bundled.skills.map((skill) => skill.name)).toEqual(['Bundled Skill']) + expect(summarizeSkillMarkdown).toHaveBeenCalledTimes(2) + } finally { + await rm(root, { recursive: true, force: true }) + } + }) +}) diff --git a/src/main/skills/discovery.test.ts b/src/main/skills/discovery.test.ts index 740c4af7cd0..7c6212a463b 100644 --- a/src/main/skills/discovery.test.ts +++ b/src/main/skills/discovery.test.ts @@ -212,6 +212,46 @@ describe('skill discovery', () => { ]) }) + it('filters discovery by requested directory name and source kind', async () => { + const root = await mkdtemp(join(tmpdir(), 'orca-skills-')) + const home = join(root, 'home') + const repo = join(root, 'repo') + const homeSkill = join(home, '.agents', 'skills', 'orchestration') + const repoSkill = join(repo, '.agents', 'skills', 'orchestration') + const unrelatedSkill = join(home, '.agents', 'skills', 'computer-use') + await mkdir(homeSkill, { recursive: true }) + await mkdir(repoSkill, { recursive: true }) + await mkdir(unrelatedSkill, { recursive: true }) + await writeFile(join(homeSkill, 'SKILL.md'), '---\nname: Agent Orchestration\n---\n') + await writeFile(join(repoSkill, 'SKILL.md'), '# orchestration') + await writeFile(join(unrelatedSkill, 'SKILL.md'), '# computer-use') + + const result = await discoverSkills({ + homeDir: home, + cwd: repo, + repos: [], + names: ['orchestration'], + sourceKinds: ['home'] + }) + + expect(result.skills).toMatchObject([ + { name: 'Agent Orchestration', sourceKind: 'home', directoryPath: homeSkill } + ]) + expect(result.sources.every((source) => source.sourceKind === 'home')).toBe(true) + + const unfiltered = await discoverSkills({ + homeDir: home, + cwd: repo, + repos: [], + sourceKinds: [] + }) + expect(unfiltered.skills.map((skill) => skill.name).sort()).toEqual([ + 'Agent Orchestration', + 'computer-use', + 'orchestration' + ]) + }) + it('discovers the enabled Claude plugin version applicable to the project cwd', async () => { const root = await mkdtemp(join(tmpdir(), 'orca-skills-')) const home = join(root, 'home') diff --git a/src/main/skills/discovery.ts b/src/main/skills/discovery.ts index 385ae99ca33..075261dce5d 100644 --- a/src/main/skills/discovery.ts +++ b/src/main/skills/discovery.ts @@ -6,7 +6,8 @@ import type { Repo } from '../../shared/repo-types' import type { DiscoveredSkill, SkillDiscoveryResult, - SkillDiscoverySource + SkillDiscoverySource, + SkillSourceKind } from '../../shared/skills' import { buildSkillDiscoverySources, @@ -17,6 +18,7 @@ import { stablePathId, type SkillScanRoot } from './skill-discovery-sources' +import { rootMayContainSourceKind } from './skill-discovery-source-filter' import { discoverClaudePluginSkillSources } from './claude-plugin-skill-sources' import { findSkillFiles } from './skill-root-file-walk' import { runSkillCandidateTasks } from './skill-candidate-concurrency' @@ -264,6 +266,8 @@ export async function discoverSkills(args: { includeCwd?: boolean providerRootOverrides?: SkillProviderRootOverrides refresh?: boolean + names?: string[] + sourceKinds?: SkillSourceKind[] }): Promise { const startedAt = Date.now() const homeDir = args.homeDir ?? homedir() @@ -272,10 +276,12 @@ export async function discoverSkills(args: { ...buildSkillDiscoverySources({ ...args, homeDir }), // Why: plugin discovery is native-chat data keyed to an explicit workspace. // Untargeted scans (Settings) keep their pre-picker inventory and cost. - ...(args.cwd && args.includeCwd !== false + ...(args.cwd && + args.includeCwd !== false && + (!args.sourceKinds?.length || args.sourceKinds.includes('plugin')) ? await discoverClaudePluginSkillSources({ homeDir, cwd: args.cwd }) : []) - ] + ].filter((root) => rootMayContainSourceKind(root, args.sourceKinds)) const scans = await Promise.all(roots.map((root) => scanRootShared(root, refresh))) const sources: SkillDiscoverySource[] = roots.map((root, index) => ({ ...root, @@ -287,9 +293,21 @@ export async function discoverSkills(args: { ? undefined : 'missing' })) + const normalizedNames = args.names?.map((name) => name.trim().toLowerCase()).filter(Boolean) + const expectedNames = normalizedNames?.length ? new Set(normalizedNames) : undefined const seen = new Map() for (const { value } of scans) { for (const skill of value.skills) { + if (args.sourceKinds?.length && !args.sourceKinds.includes(skill.sourceKind)) { + continue + } + if ( + expectedNames && + !expectedNames.has(skill.name.trim().toLowerCase()) && + !expectedNames.has(basename(skill.directoryPath).trim().toLowerCase()) + ) { + continue + } mergeScannedSkill(seen, skill) } } diff --git a/src/main/skills/skill-delete/roots.test.ts b/src/main/skills/skill-delete/roots.test.ts new file mode 100644 index 00000000000..572c99343b3 --- /dev/null +++ b/src/main/skills/skill-delete/roots.test.ts @@ -0,0 +1,30 @@ +import { describe, expect, it, vi } from 'vitest' + +vi.mock('../claude-plugin-skill-sources-wsl', () => ({ + discoverClaudePluginSkillSourcesInWsl: vi.fn().mockResolvedValue([]) +})) +import { discoverClaudePluginSkillSourcesInWsl } from '../claude-plugin-skill-sources-wsl' +import { buildSkillDeleteRootSet } from './roots' + +describe('WSL skill deletion root ownership', () => { + it('uses the guest home for an omitted cwd, preserving the prior resolved target', async () => { + const target = { + kind: 'wsl' as const, + distro: 'Ubuntu', + homeDir: '/home/alice', + cwd: undefined + } + const omitted = await buildSkillDeleteRootSet({ target, repos: [] }) + const explicit = await buildSkillDeleteRootSet({ + target: { ...target, cwd: target.homeDir }, + repos: [] + }) + expect(omitted.roots).toEqual(explicit.roots) + expect(omitted.roots.every((root) => root.path.startsWith('/home/alice/'))).toBe(true) + expect(discoverClaudePluginSkillSourcesInWsl).toHaveBeenCalledWith({ + distro: 'Ubuntu', + homeDir: '/home/alice', + cwd: '/home/alice' + }) + }) +}) diff --git a/src/main/skills/skill-delete/roots.ts b/src/main/skills/skill-delete/roots.ts index 1958ed91942..86d2b80bd17 100644 --- a/src/main/skills/skill-delete/roots.ts +++ b/src/main/skills/skill-delete/roots.ts @@ -33,7 +33,8 @@ export async function buildSkillDeleteRootSet(input: { homeDir?: string }): Promise { if (input.target.kind === 'wsl') { - const { distro, homeDir, cwd } = input.target + const { distro, homeDir } = input.target + const cwd = input.target.cwd ?? homeDir return { roots: [ ...buildSkillDiscoverySources({ diff --git a/src/main/skills/skill-discovery-source-filter.test.ts b/src/main/skills/skill-discovery-source-filter.test.ts new file mode 100644 index 00000000000..29d6ac6ca7e --- /dev/null +++ b/src/main/skills/skill-discovery-source-filter.test.ts @@ -0,0 +1,24 @@ +import { describe, expect, it } from 'vitest' +import type { SkillScanRoot } from './skill-discovery-sources' +import { rootMayContainSourceKind } from './skill-discovery-source-filter' + +const homeRoot: SkillScanRoot = { + id: 'home', + owner: 'agents', + path: '/home/alice/.agents/skills', + label: 'Home', + sourceKind: 'home', + providers: ['agent-skills'] +} + +describe('rootMayContainSourceKind', () => { + it('treats an empty list as no filter', () => { + expect(rootMayContainSourceKind(homeRoot, undefined)).toBe(true) + expect(rootMayContainSourceKind(homeRoot, [])).toBe(true) + }) + + it('keeps home roots for bundled classification', () => { + expect(rootMayContainSourceKind(homeRoot, ['bundled'])).toBe(true) + expect(rootMayContainSourceKind(homeRoot, ['plugin'])).toBe(false) + }) +}) diff --git a/src/main/skills/skill-discovery-source-filter.ts b/src/main/skills/skill-discovery-source-filter.ts new file mode 100644 index 00000000000..ce76a518612 --- /dev/null +++ b/src/main/skills/skill-discovery-source-filter.ts @@ -0,0 +1,29 @@ +import type { SkillSourceKind } from '../../shared/skills' +import type { SkillScanRoot } from './skill-discovery-sources' + +export function skillScanSourceKinds( + sourceKinds: readonly SkillSourceKind[] | undefined +): SkillSourceKind[] | undefined { + if (!sourceKinds?.length) { + return undefined + } + const kinds = new Set(sourceKinds) + if (kinds.has('home') || kinds.has('bundled')) { + kinds.add('home') + kinds.add('bundled') + } + return [...kinds].sort() +} + +export function rootMayContainSourceKind( + root: SkillScanRoot, + sourceKinds: readonly SkillSourceKind[] | undefined +): boolean { + if (!sourceKinds?.length) { + return true + } + if (root.sourceKind === 'home') { + return sourceKinds.includes('home') || sourceKinds.includes('bundled') + } + return sourceKinds.includes(root.sourceKind) +} diff --git a/src/main/skills/skill-discovery-target.test.ts b/src/main/skills/skill-discovery-target.test.ts index 899acbb8477..8bea1cbcf50 100644 --- a/src/main/skills/skill-discovery-target.test.ts +++ b/src/main/skills/skill-discovery-target.test.ts @@ -18,9 +18,9 @@ vi.mock('./discovery', () => ({ })) vi.mock('./skill-discovery-wsl', () => ({ - discoverSkillsInWsl: vi.fn(async (args: unknown) => { + discoverSkillObservationInWsl: vi.fn(async (args: unknown) => { wslScans.push(args) - return emptyResult() + return { rows: [], sources: [], scannedAt: 1 } }) })) @@ -135,6 +135,22 @@ describe('discoverSkillsOnTarget', () => { expect(wslScans).toHaveLength(3) }) + it('distinguishes an absent WSL cwd from the literal undefined path', async () => { + await discoverSkillsOnTarget( + { kind: 'wsl', distro: 'Ubuntu', homeDir: '/home/dev', cwd: undefined }, + [] + ) + await discoverSkillsOnTarget( + { kind: 'wsl', distro: 'Ubuntu', homeDir: '/home/dev', cwd: 'undefined' }, + [] + ) + + expect(wslScans).toEqual([ + { distro: 'Ubuntu', homeDir: '/home/dev' }, + { distro: 'Ubuntu', homeDir: '/home/dev', cwd: 'undefined' } + ]) + }) + it('re-reads a WSL target when the caller refreshes', async () => { const target = { kind: 'wsl', diff --git a/src/main/skills/skill-discovery-target.ts b/src/main/skills/skill-discovery-target.ts index ea1ab8c410a..55c83cd94e5 100644 --- a/src/main/skills/skill-discovery-target.ts +++ b/src/main/skills/skill-discovery-target.ts @@ -1,10 +1,15 @@ +import { + projectWslSkillDiscovery, + type WslSkillDiscoveryObservation +} from './skill-discovery-wsl-observation' import type { Repo } from '../../shared/repo-types' import type { SkillDiscoveryResult, SkillDiscoveryTarget } from '../../shared/skills' import { getDefaultWslDistro, getWslHome, parseWslPath, toLinuxPath } from '../wsl' import { clearSkillRootScanCache, discoverSkills } from './discovery' -import { discoverSkillsInWsl } from './skill-discovery-wsl' +import { discoverSkillObservationInWsl } from './skill-discovery-wsl' import type { SkillProviderRootOverrides } from './skill-provider-destinations' import { stablePathId } from './skill-discovery-sources' +import { skillScanSourceKinds } from './skill-discovery-source-filter' import { getRepoExecutionHostId } from '../../shared/execution-host' import { isSkillRootUnavailableError, SkillScanCoalescer } from './skill-scan-coalescer' @@ -14,7 +19,10 @@ import { isSkillRootUnavailableError, SkillScanCoalescer } from './skill-scan-co const WSL_RESULT_TTL_MS = 10_000 const MAX_CACHED_SKILL_TARGETS = 32 -const targetScans = new SkillScanCoalescer(MAX_CACHED_SKILL_TARGETS) +type TargetScanObservation = + | { kind: 'native'; result: SkillDiscoveryResult } + | { kind: 'wsl'; observation: WslSkillDiscoveryObservation } +const targetScans = new SkillScanCoalescer(MAX_CACHED_SKILL_TARGETS) /** Drop every shared scan; used when a skill update run has rewritten disk. */ export function clearSkillDiscoveryCaches(): void { @@ -23,8 +31,20 @@ export function clearSkillDiscoveryCaches(): void { } export type ResolvedSkillDiscoveryTarget = - | { kind: 'native-host'; cwd: string | undefined } - | { kind: 'wsl'; distro: string; homeDir: string; cwd: string } + | { + kind: 'native-host' + cwd: string | undefined + names?: string[] + sourceKinds?: SkillDiscoveryTarget['sourceKinds'] + } + | { + kind: 'wsl' + distro: string + homeDir: string + cwd: string | undefined + names?: string[] + sourceKinds?: SkillDiscoveryTarget['sourceKinds'] + } export function resolveSkillDiscoveryTarget( target: SkillDiscoveryTarget | undefined @@ -49,7 +69,12 @@ export function resolveSkillDiscoveryTarget( throw new Error('No WSL distribution is available for skill discovery.') } if (!wslDistro) { - return { kind: 'native-host', cwd: target?.cwd?.trim() || undefined } + return { + kind: 'native-host', + cwd: target?.cwd?.trim() || undefined, + ...(target?.names ? { names: target.names } : {}), + ...(target?.sourceKinds ? { sourceKinds: target.sourceKinds } : {}) + } } if (process.platform !== 'win32') { throw new Error('WSL skill discovery is only available on Windows.') @@ -67,8 +92,15 @@ export function resolveSkillDiscoveryTarget( ) } const linuxHomeDir = toLinuxPath(homeDir) - const cwd = parsedCwd?.linuxPath ?? (requestedCwd ? toLinuxPath(requestedCwd) : linuxHomeDir) - return { kind: 'wsl', distro: wslDistro, homeDir: linuxHomeDir, cwd } + const cwd = parsedCwd?.linuxPath ?? (requestedCwd ? toLinuxPath(requestedCwd) : undefined) + return { + kind: 'wsl', + distro: wslDistro, + homeDir: linuxHomeDir, + cwd, + ...(target?.names ? { names: target.names } : {}), + ...(target?.sourceKinds ? { sourceKinds: target.sourceKinds } : {}) + } } // Why: repos widen the native root set, so two targets that differ only by the @@ -93,17 +125,28 @@ function scanKey( repos: readonly Repo[], providerRootOverrides: SkillProviderRootOverrides | undefined ): string { - const providerRoots = stablePathId( - Object.entries(providerRootOverrides ?? {}) - .sort(([left], [right]) => left.localeCompare(right)) - .map(([provider, root]) => `${provider}\0${root}`) - .join('\0') + const providerRoots = Object.entries(providerRootOverrides ?? {}).sort(([left], [right]) => + left.localeCompare(right) ) - const targetKey = - target.kind === 'wsl' - ? `wsl\0${target.distro}\0${target.homeDir}\0${target.cwd}` - : `native\0${target.cwd ?? ''}\0${target.cwd ? '' : repoDigest(repos)}` - return `${targetKey}\0${providerRoots}` + const names = target.names?.slice().sort() ?? null + const sourceKinds = target.sourceKinds?.slice().sort() ?? null + return target.kind === 'wsl' + ? JSON.stringify([ + 'wsl', + target.distro, + target.homeDir, + target.cwd ?? null, + providerRoots, + skillScanSourceKinds(target.sourceKinds) ?? null + ]) + : JSON.stringify([ + 'native', + target.cwd ?? null, + target.cwd ? null : repoDigest(repos), + providerRoots, + names, + sourceKinds + ]) } export async function discoverSkillsOnTarget( @@ -116,30 +159,41 @@ export async function discoverSkillsOnTarget( const outcome = await targetScans.run( scanKey(target, repos, options.providerRootOverrides), { ttlMs: target.kind === 'wsl' ? WSL_RESULT_TTL_MS : 0, refresh }, - async () => { + async (): Promise => { if (target.kind === 'wsl') { - return discoverSkillsInWsl({ - distro: target.distro, - homeDir: target.homeDir, - cwd: target.cwd, - providerRootOverrides: options.providerRootOverrides - }) + return { + kind: 'wsl', + observation: await discoverSkillObservationInWsl({ + distro: target.distro, + homeDir: target.homeDir, + ...(target.cwd ? { cwd: target.cwd } : {}), + sourceKinds: skillScanSourceKinds(target.sourceKinds), + providerRootOverrides: options.providerRootOverrides + }) + } } - return target.cwd + const result = await (target.cwd ? discoverSkills({ repos: [], cwd: target.cwd, refresh, + ...(target.names ? { names: target.names } : {}), + ...(target.sourceKinds ? { sourceKinds: target.sourceKinds } : {}), providerRootOverrides: options.providerRootOverrides }) : discoverSkills({ repos: [...repos], refresh, + ...(target.names ? { names: target.names } : {}), + ...(target.sourceKinds ? { sourceKinds: target.sourceKinds } : {}), providerRootOverrides: options.providerRootOverrides - }) + })) + return { kind: 'native', result } } ) - return outcome.value + return outcome.value.kind === 'wsl' + ? projectWslSkillDiscovery(outcome.value.observation, target.sourceKinds, target.names) + : outcome.value.result } catch (error) { if (!isSkillRootUnavailableError(error)) { throw error diff --git a/src/main/skills/skill-discovery-wsl-alias-sharing.test.ts b/src/main/skills/skill-discovery-wsl-alias-sharing.test.ts new file mode 100644 index 00000000000..79a7b4e9b43 --- /dev/null +++ b/src/main/skills/skill-discovery-wsl-alias-sharing.test.ts @@ -0,0 +1,145 @@ +import { beforeEach, expect, it, vi } from 'vitest' +const io = vi.hoisted(() => ({ run: vi.fn(), plugins: vi.fn(async () => []) })) +vi.mock('../wsl/wsl-runner', () => ({ runWslProcess: io.run })) +vi.mock('./claude-plugin-skill-sources-wsl', () => ({ + discoverClaudePluginSkillSourcesInWsl: io.plugins +})) +vi.mock('./discovery', () => ({ clearSkillRootScanCache: vi.fn(), discoverSkills: vi.fn() })) +import { clearSkillDiscoveryCaches, discoverSkillsOnTarget } from './skill-discovery-target' +import { + readWslSkillDiscoveryObservation, + projectWslSkillDiscovery +} from './skill-discovery-wsl-observation' +import type { SkillScanRoot } from './skill-discovery-sources' +const target = { kind: 'wsl' as const, distro: 'Ubuntu', homeDir: '/home/test', cwd: '/repo' } +const record = (...fields: string[]) => `${fields.join('\0')}\0` +const encoded = Buffer.from('---\nname: shared-frontmatter\ndescription: Fixture\n---\n').toString( + 'base64' +) +const common = '/opt/physical/SKILL.md' +const rows = [ + record('S', '0', '/home/test/.codex/skills/.system/bundle/SKILL.md', common, '1', encoded), + record('S', '0', '/home/test/.codex/skills/alias-a/SKILL.md', common, '1', encoded), + record('S', '1', '/home/test/.agents/skills/alias-b/SKILL.md', common, '1', encoded), + ...Array.from({ length: 6 }, (_, i) => + record( + 'S', + '0', + `/home/test/.codex/skills/skill-${i}/SKILL.md`, + `/physical/skill-${i}/SKILL.md`, + '1', + encoded + ) + ) +] +const output = record('R', '0', '1') + record('R', '1', '1') + rows.join('') +beforeEach(() => { + clearSkillDiscoveryCaches() + io.run.mockReset() + io.plugins.mockClear() + io.run.mockResolvedValue({ code: 0, timedOut: false, stdout: output, stderr: '' }) +}) +it('keeps both home aliases when a bundled canonical duplicate appears first', async () => { + const [a, b, bundle] = await Promise.all([ + discoverSkillsOnTarget({ ...target, names: ['alias-a'], sourceKinds: ['home'] }, []), + discoverSkillsOnTarget({ ...target, names: ['alias-b'], sourceKinds: ['home'] }, []), + discoverSkillsOnTarget({ ...target, names: ['bundle'], sourceKinds: ['bundled'] }, []) + ]) + expect(io.run).toHaveBeenCalledTimes(1) + expect(a.skills.map((s) => s.directoryPath)).toEqual(['/home/test/.codex/skills/alias-a']) + expect(b.skills.map((s) => s.directoryPath)).toEqual(['/home/test/.agents/skills/alias-b']) + expect(bundle.skills.map((s) => s.directoryPath)).toEqual([ + '/home/test/.codex/skills/.system/bundle' + ]) + expect(a.skills[0].providers).toEqual(['codex']) + expect(b.skills[0].providers).toEqual(['agent-skills']) + expect(a.skills[0].id).toBe(b.skills[0].id) + expect(a.skills[0].sourceKind).toBe('home') +}) +it('six distinct installed-name checks share one scan and retain all six answers', async () => { + const results = await Promise.all( + Array.from({ length: 6 }, (_, i) => + discoverSkillsOnTarget({ ...target, names: [`skill-${i}`], sourceKinds: ['home'] }, []) + ) + ) + expect(io.run).toHaveBeenCalledTimes(1) + expect(results.map((r) => r.skills[0]?.directoryPath)).toEqual( + Array.from({ length: 6 }, (_, i) => `/home/test/.codex/skills/skill-${i}`) + ) + expect(io.run.mock.calls[0][0].timeoutMs).toBe(10000) + expect(io.run.mock.calls[0][0].script).not.toContain('matches_requested_name') + expect(io.run.mock.calls[0][0].script).not.toContain("'/repo/") + expect(io.plugins).not.toHaveBeenCalled() +}) +it('cache projections do not contaminate later aliases or source metadata', async () => { + const first = await discoverSkillsOnTarget( + { ...target, names: ['alias-a'], sourceKinds: ['home'] }, + [] + ) + first.skills[0].providers.push('claude') + first.skills[0].rootPaths!.push('/poison') + first.sources[0].providers.push('claude') + const later = await discoverSkillsOnTarget( + { ...target, names: ['alias-a'], sourceKinds: ['home'] }, + [] + ) + expect(later.skills[0].providers).toEqual(['codex']) + expect(later.skills[0].rootPaths).toEqual(['/home/test/.codex/skills']) + expect(later.sources[0].providers).not.toContain('claude') + expect(io.run).toHaveBeenCalledTimes(1) +}) +it('refresh, cache clear, distro and broader root requirements are isolated', async () => { + const req = { ...target, names: ['alias-a'], sourceKinds: ['home' as const] } + await discoverSkillsOnTarget(req, []) + await discoverSkillsOnTarget({ ...req, names: ['alias-b'] }, []) + expect(io.run).toHaveBeenCalledTimes(1) + await discoverSkillsOnTarget(req, [], { refresh: true }) + clearSkillDiscoveryCaches() + await discoverSkillsOnTarget(req, []) + await discoverSkillsOnTarget({ ...req, distro: 'Other' }, []) + await discoverSkillsOnTarget({ ...target, names: ['alias-a'] }, []) + expect(io.run).toHaveBeenCalledTimes(5) + expect(io.plugins).toHaveBeenCalledTimes(1) +}) +it('deduplicates and merges only eligible alias roots, independently of row order', () => { + const roots: SkillScanRoot[] = [ + { + id: 'home', + label: 'Home', + path: '/home/test/.codex/skills', + sourceKind: 'home', + providers: ['codex'], + owner: 'codex' + }, + { + id: 'home2', + label: 'Home2', + path: '/home/test/.agents/skills', + sourceKind: 'home', + providers: ['agent-skills'], + owner: null + } + ] + for (const records of [rows, rows.toReversed()]) { + const obs = readWslSkillDiscoveryObservation(records.join(''), roots, 42) + const a = projectWslSkillDiscovery(obs, ['home'], ['alias-a']) + const b = projectWslSkillDiscovery(obs, ['home'], ['alias-b']) + expect(a.skills).toHaveLength(1) + expect(b.skills).toHaveLength(1) + expect(a.skills[0].providers).toEqual(['codex']) + expect(b.skills[0].providers).toEqual(['agent-skills']) + const both = projectWslSkillDiscovery(obs, ['home'], ['alias-a', 'alias-b']) + expect(both.skills).toHaveLength(1) + expect(new Set(both.skills[0].providers)).toEqual(new Set(['codex', 'agent-skills'])) + const all = projectWslSkillDiscovery(obs) + expect(all.skills).toHaveLength(7) + expect(all.scannedAt).toBe(42) + } +}) +it('does not cache failed scans as an empty successful observation', async () => { + io.run.mockResolvedValueOnce({ code: 1, timedOut: false, stdout: '', stderr: 'failure' }) + const req = { ...target, names: ['alias-a'], sourceKinds: ['home' as const] } + await expect(discoverSkillsOnTarget(req, [])).rejects.toThrow('skill-discovery-wsl-scan-failed') + expect((await discoverSkillsOnTarget(req, [])).skills).toHaveLength(1) + expect(io.run).toHaveBeenCalledTimes(2) +}) diff --git a/src/main/skills/skill-discovery-wsl-bash-filter.test.ts b/src/main/skills/skill-discovery-wsl-bash-filter.test.ts new file mode 100644 index 00000000000..95b35acdd50 --- /dev/null +++ b/src/main/skills/skill-discovery-wsl-bash-filter.test.ts @@ -0,0 +1,89 @@ +import { execFileSync } from 'node:child_process' +import { mkdir, mkdtemp, rm, writeFile } from 'node:fs/promises' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { describe, expect, it } from 'vitest' +import { buildWslSkillDiscoveryCommand, parseWslSkillDiscoveryOutput } from './skill-discovery-wsl' +import type { SkillScanRoot } from './skill-discovery-sources' + +async function writeSkill(root: string, directory: string, markdown: string): Promise { + const skillDirectory = join(root, directory) + await mkdir(skillDirectory, { recursive: true }) + await writeFile(join(skillDirectory, 'SKILL.md'), markdown) +} + +describe('generated WSL skill name filter', () => { + it.skipIf(process.platform !== 'linux')( + 'rejects only known scalar mismatches and passes uncertain names to TypeScript', + async () => { + const root = await mkdtemp(join(tmpdir(), 'orca-wsl-name-filter-')) + const scanRoot: SkillScanRoot = { + id: 'home', + owner: 'agents', + path: root, + label: 'Home', + sourceKind: 'home', + providers: ['agent-skills'] + } + for (const directory of [' orchestration', 'orchestration ', ' orchestration ']) { + await writeSkill(root, directory, '---\nname: unrelated\n---\n') + } + await writeSkill(root, 'scalar-match', '---\nname: orchestration\n---\n') + await writeSkill(root, 'scalar-mismatch', '---\nname: unrelated\n---\n') + await writeSkill(root, 'empty-quoted', '---\nname: ""\n---\n# orchestration\n') + await writeSkill(root, 'one-quote', '---\nname: "\n---\n# orchestration\n') + await writeSkill(root, 'block-name', '---\nname: >-\n orchestration\n---\n') + await writeSkill(root, 'bom-crlf', "\uFEFF---\r\nname: 'orchestration'\r\n---\r\n") + await writeSkill(root, 'unicode-space', '---\nname:\u3000orchestration\n---\n') + await writeSkill(root, 'missing-close', '---\nname: unrelated\n# orchestration\n') + await writeSkill(root, 'duplicate-match', '---\nname: unrelated\nname: orchestration\n---\n') + await writeSkill( + root, + 'duplicate-mismatch', + '---\nname: orchestration\nname: unrelated\n---\n' + ) + await writeSkill( + root, + 'beyond-limit', + `---\ndescription: |\n${' x\n'.repeat(70_000)}name: unrelated\n---\n# orchestration\n` + ) + await writeSkill( + root, + 'multibyte-beyond-limit', + `---\ndescription: ${'한'.repeat(90_000)}\nname: unrelated\n---\n# orchestration\n` + ) + + try { + const command = buildWslSkillDiscoveryCommand([scanRoot], ['orchestration']) + const output = execFileSync('/bin/bash', ['-c', command], { + encoding: 'utf8', + maxBuffer: 4 * 1024 * 1024 + }) + expect(output).not.toContain('scalar-mismatch') + expect(output).not.toContain('duplicate-mismatch') + expect(output).toContain('beyond-limit') + expect(output).toContain('multibyte-beyond-limit') + expect( + parseWslSkillDiscoveryOutput(output, [scanRoot], 42, ['home'], ['orchestration']) + .skills.map((skill) => skill.directoryPath.split('/').at(-1)) + .sort() + ).toEqual([ + ' orchestration', + ' orchestration ', + 'block-name', + 'bom-crlf', + 'duplicate-match', + 'empty-quoted', + 'missing-close', + 'one-quote', + 'orchestration ', + 'scalar-match', + 'unicode-space' + ]) + } finally { + await rm(root, { recursive: true, force: true }) + } + }, + 30_000 + ) +}) diff --git a/src/main/skills/skill-discovery-wsl-observation.ts b/src/main/skills/skill-discovery-wsl-observation.ts new file mode 100644 index 00000000000..92341d93678 --- /dev/null +++ b/src/main/skills/skill-discovery-wsl-observation.ts @@ -0,0 +1,165 @@ +import { posix as pathPosix } from 'node:path' +import { summarizeSkillMarkdown } from '../../shared/skill-metadata' +import type { + DiscoveredSkill, + SkillDiscoveryResult, + SkillDiscoverySource, + SkillSourceKind +} from '../../shared/skills' +import { + sortDiscoveredSkills, + sortSkillDiscoverySources, + sourceKindForSkill, + sourceLabelForSkill, + stablePathId, + type SkillScanRoot +} from './skill-discovery-sources' +import { rootMayContainSourceKind } from './skill-discovery-source-filter' + +export type WslSkillDiscoveryObservation = { + rows: { canonicalSkillFilePath: string; skill: DiscoveredSkill }[] + sources: SkillDiscoverySource[] + scannedAt: number +} + +function readProtocolField(fields: string[], index: number): string { + const value = fields[index] + if (value === undefined) { + throw new Error('WSL skill discovery returned an incomplete response.') + } + return value +} + +export function readWslSkillDiscoveryObservation( + output: string, + roots: readonly SkillScanRoot[], + scannedAt = Date.now() +): WslSkillDiscoveryObservation { + const fields = output.split('\0') + const rootExists = new Map() + const rows: WslSkillDiscoveryObservation['rows'] = [] + let index = 0 + while (index < fields.length && fields[index]) { + const recordKind = fields[index++] + const rootIndex = Number.parseInt(readProtocolField(fields, index++), 10) + const root = roots[rootIndex] + if (!root) { + throw new Error('WSL skill discovery returned an unknown source.') + } + if (recordKind === 'R') { + rootExists.set(rootIndex, readProtocolField(fields, index++) === '1') + continue + } + if (recordKind !== 'S') { + throw new Error('WSL skill discovery returned an invalid response.') + } + + const skillFilePath = readProtocolField(fields, index++) + const canonicalSkillFilePath = readProtocolField(fields, index++) + const updatedAtSeconds = Number.parseInt(readProtocolField(fields, index++), 10) + const markdown = Buffer.from(readProtocolField(fields, index++), 'base64').toString('utf8') + const directoryPath = pathPosix.dirname(skillFilePath) + const summary = summarizeSkillMarkdown(markdown) + const sourceKind = sourceKindForSkill(root, skillFilePath, pathPosix) + const directoryName = pathPosix.basename(directoryPath) + rows.push({ + canonicalSkillFilePath, + skill: { + id: stablePathId(canonicalSkillFilePath), + name: summary.name ?? directoryName, + description: summary.description, + // Copy: `root.providers` is shared across every skill/source from this + // root, so a later in-place merge must not mutate the aliased array. + providers: [...root.providers], + sourceKind, + sourceLabel: sourceLabelForSkill(root, sourceKind), + rootPath: root.path, + rootPaths: [root.path], + directoryPath, + skillFilePath, + installed: true, + updatedAt: Number.isFinite(updatedAtSeconds) ? updatedAtSeconds * 1000 : null + } + }) + } + + const sources: SkillDiscoverySource[] = roots.map((root, rootIndex) => { + const exists = rootExists.get(rootIndex) ?? false + return { + ...root, + providers: [...root.providers], + exists, + skippedReason: exists ? undefined : 'missing' + } + }) + return { + rows, + sources: sortSkillDiscoverySources(sources), + scannedAt + } +} + +export function projectWslSkillDiscovery( + observation: WslSkillDiscoveryObservation, + sourceKinds?: readonly SkillSourceKind[], + names?: readonly string[] +): SkillDiscoveryResult { + const normalizedNames = names?.map((name) => name.trim().toLowerCase()).filter(Boolean) + const expectedNames = normalizedNames?.length ? new Set(normalizedNames) : undefined + const skillsByCanonicalPath = new Map() + for (const { canonicalSkillFilePath, skill } of observation.rows) { + if (sourceKinds?.length && !sourceKinds.includes(skill.sourceKind)) { + continue + } + const directoryName = pathPosix.basename(skill.directoryPath) + if ( + expectedNames && + !expectedNames.has(skill.name.trim().toLowerCase()) && + !expectedNames.has(directoryName.trim().toLowerCase()) + ) { + continue + } + // Filter aliases before deduplication; each name/source may select a different row. + const existing = skillsByCanonicalPath.get(canonicalSkillFilePath) + if (existing) { + const existingRoots = (existing.rootPaths ??= [existing.rootPath]) + for (const rootPath of skill.rootPaths ?? [skill.rootPath]) { + if (!existingRoots.includes(rootPath)) { + existingRoots.push(rootPath) + } + } + for (const provider of skill.providers) { + if (!existing.providers.includes(provider)) { + existing.providers.push(provider) + } + } + continue + } + skillsByCanonicalPath.set(canonicalSkillFilePath, { + ...skill, + providers: [...skill.providers], + rootPaths: [...(skill.rootPaths ?? [skill.rootPath])] + }) + } + return { + skills: sortDiscoveredSkills([...skillsByCanonicalPath.values()]), + sources: observation.sources + .filter((source) => rootMayContainSourceKind(source, sourceKinds)) + .map((source) => ({ ...source, providers: [...source.providers] })), + scannedAt: observation.scannedAt + } +} + +export function parseWslSkillDiscoveryOutput( + output: string, + roots: readonly SkillScanRoot[], + scannedAt = Date.now(), + sourceKinds?: readonly SkillSourceKind[], + names?: readonly string[] +): SkillDiscoveryResult { + return projectWslSkillDiscovery( + readWslSkillDiscoveryObservation(output, roots, scannedAt), + sourceKinds, + names + ) +} diff --git a/src/main/skills/skill-discovery-wsl-plugins.test.ts b/src/main/skills/skill-discovery-wsl-plugins.test.ts index f3ee9024ff2..2e8df58f68f 100644 --- a/src/main/skills/skill-discovery-wsl-plugins.test.ts +++ b/src/main/skills/skill-discovery-wsl-plugins.test.ts @@ -7,6 +7,7 @@ const runWslProcessMock = vi.hoisted(() => vi.fn()) vi.mock('../wsl/wsl-runner', () => ({ runWslProcess: runWslProcessMock })) import { buildSkillDiscoverySources } from './skill-discovery-sources' +import { rootMayContainSourceKind } from './skill-discovery-source-filter' import { discoverSkillsInWsl } from './skill-discovery-wsl' function record(...fields: string[]): string { @@ -17,66 +18,131 @@ function wslResult(stdout: string): WslResult { return { environmentResolved: true, code: 0, stdout, stderr: '', timedOut: false } } +function recordedScript(index: number): string { + const script: unknown = runWslProcessMock.mock.calls[index]?.[0].script + if (typeof script !== 'string') { + throw new Error('Expected a generated WSL script') + } + return script +} + describe('WSL Claude plugin skill discovery', () => { beforeEach(() => runWslProcessMock.mockReset()) afterEach(() => vi.unstubAllEnvs()) - it('reads enabled plugin metadata and scans the selected install inside the distro', async () => { - const homeDir = '/home/alice' - const cwd = '/work/orca' - // Why: a Windows host's own Hermes location says nothing about the distro's, - // so neither variable may reach the posix scan script. - vi.stubEnv('HERMES_HOME', 'C:\\Users\\alice\\hermes') - vi.stubEnv('LOCALAPPDATA', 'C:\\Users\\alice\\AppData\\Local') - const pluginId = 'compound-engineering@compound-engineering-plugin' - const installPath = '/home/alice/.claude/plugins/cache/compound/3.14.3' - const installed = JSON.stringify({ - plugins: { - [pluginId]: [{ scope: 'project', projectPath: cwd, installPath }] - } + it('skips workspace roots and plugin metadata for home-only discovery without cwd', async () => { + runWslProcessMock.mockResolvedValueOnce(wslResult('')) + + const result = await discoverSkillsInWsl({ + distro: 'Ubuntu', + homeDir: '/home/alice', + sourceKinds: ['home'] }) - const settings = JSON.stringify({ enabledPlugins: { [pluginId]: true } }) - const metadataOutput = [ - record('F', '0', '1', Buffer.from(installed).toString('base64')), - record('F', '1', '1', Buffer.from(settings).toString('base64')), - record('F', '2', '0', ''), - record('F', '3', '0', '') - ].join('') - const baseRootCount = buildSkillDiscoverySources({ - homeDir, - cwd, + + expect(runWslProcessMock).toHaveBeenCalledTimes(1) + const scanScript = recordedScript(0) + expect(scanScript.match(/'\/home\/alice\/\.agents\/skills'/g)).toHaveLength(1) + expect(scanScript.match(/'\/home\/alice\/\.claude\/skills'/g)).toHaveLength(1) + const expectedRoots = buildSkillDiscoverySources({ + homeDir: '/home/alice', + cwd: undefined, repos: [], + includeCwd: false, pathApi: pathPosix - }).length - const skillPath = `${installPath}/skills/ce-plan/SKILL.md` - const markdown = Buffer.from('---\nname: ce-plan\ndescription: Plan work.\n---\n').toString( - 'base64' - ) - const scanOutput = [ - record('R', String(baseRootCount), '1'), - record('S', String(baseRootCount), skillPath, skillPath, '1700000000', markdown) - ].join('') - runWslProcessMock.mockResolvedValueOnce(wslResult(metadataOutput)) - runWslProcessMock.mockResolvedValueOnce(wslResult(scanOutput)) - - const result = await discoverSkillsInWsl({ distro: 'Ubuntu', homeDir, cwd }) - - expect(runWslProcessMock).toHaveBeenCalledTimes(2) - const scanScript = runWslProcessMock.mock.calls[1]?.[0].script as string - expect(scanScript).toContain('/home/alice/.hermes/skills') - expect(scanScript).not.toContain('AppData') - expect(scanScript).toContain(`${installPath}/skills`) - expect(result.skills).toEqual([ - expect.objectContaining({ - name: 'ce-plan', - sourceKind: 'plugin', - rootPath: `${installPath}/skills` - }) - ]) - expect(result.sources).toEqual( - expect.arrayContaining([ - expect.objectContaining({ path: `${installPath}/skills`, owner: 'claude', exists: true }) - ]) + }) + expect(result.sources).toHaveLength( + expectedRoots.filter((root) => rootMayContainSourceKind(root, ['home'])).length ) }) + + it('skips plugin metadata and unrelated roots for filtered home discovery', async () => { + runWslProcessMock.mockResolvedValueOnce(wslResult('')) + + const result = await discoverSkillsInWsl({ + distro: 'Ubuntu', + homeDir: '/home/alice', + cwd: '/work/orca', + names: ['orchestration'], + sourceKinds: ['home'] + }) + + expect(runWslProcessMock).toHaveBeenCalledTimes(1) + const scanScript = recordedScript(0) + expect(scanScript).not.toContain('/work/orca') + expect(scanScript).not.toContain("'/home/alice/.codex/plugins/cache'") + const expectedRoots = buildSkillDiscoverySources({ + homeDir: '/home/alice', + cwd: '/work/orca', + repos: [], + includeCwd: true, + pathApi: pathPosix + }).filter((root) => rootMayContainSourceKind(root, ['home'])) + expect(result.sources).toHaveLength(expectedRoots.length) + }) + + it.each([true, false])( + 'preserves enabled plugins with explicit workspace=%s', + async (explicitWorkspace) => { + const homeDir = '/home/alice' + const cwd = explicitWorkspace ? '/work/orca' : homeDir + // Why: a Windows host's own Hermes location says nothing about the distro's, + // so neither variable may reach the posix scan script. + vi.stubEnv('HERMES_HOME', 'C:\\Users\\alice\\hermes') + vi.stubEnv('LOCALAPPDATA', 'C:\\Users\\alice\\AppData\\Local') + const pluginId = 'compound-engineering@compound-engineering-plugin' + const installPath = '/home/alice/.claude/plugins/cache/compound/3.14.3' + const installed = JSON.stringify({ + plugins: { + [pluginId]: [{ scope: 'project', projectPath: cwd, installPath }] + } + }) + const settings = JSON.stringify({ enabledPlugins: { [pluginId]: true } }) + const metadataOutput = [ + record('F', '0', '1', Buffer.from(installed).toString('base64')), + record('F', '1', '1', Buffer.from(settings).toString('base64')), + record('F', '2', '0', ''), + record('F', '3', '0', '') + ].join('') + const baseRootCount = buildSkillDiscoverySources({ + homeDir, + cwd, + repos: [], + pathApi: pathPosix + }).length + const skillPath = `${installPath}/skills/ce-plan/SKILL.md` + const markdown = Buffer.from('---\nname: ce-plan\ndescription: Plan work.\n---\n').toString( + 'base64' + ) + const scanOutput = [ + record('R', String(baseRootCount), '1'), + record('S', String(baseRootCount), skillPath, skillPath, '1700000000', markdown) + ].join('') + runWslProcessMock.mockResolvedValueOnce(wslResult(metadataOutput)) + runWslProcessMock.mockResolvedValueOnce(wslResult(scanOutput)) + + const result = await discoverSkillsInWsl({ + distro: 'Ubuntu', + homeDir, + ...(explicitWorkspace ? { cwd } : {}) + }) + + expect(runWslProcessMock).toHaveBeenCalledTimes(2) + const scanScript = recordedScript(1) + expect(scanScript).toContain('/home/alice/.hermes/skills') + expect(scanScript).not.toContain('AppData') + expect(scanScript).toContain(`${installPath}/skills`) + expect(result.skills).toEqual([ + expect.objectContaining({ + name: 'ce-plan', + sourceKind: 'plugin', + rootPath: `${installPath}/skills` + }) + ]) + expect(result.sources).toEqual( + expect.arrayContaining([ + expect.objectContaining({ path: `${installPath}/skills`, owner: 'claude', exists: true }) + ]) + ) + } + ) }) diff --git a/src/main/skills/skill-discovery-wsl.test.ts b/src/main/skills/skill-discovery-wsl.test.ts index 93640698dd0..e0dd6cf8a2b 100644 --- a/src/main/skills/skill-discovery-wsl.test.ts +++ b/src/main/skills/skill-discovery-wsl.test.ts @@ -82,6 +82,88 @@ describe('WSL skill discovery', () => { expect(script).toContain(`'/work/alice'\\''s project/.agents/skills'`) }) + it('filters requested names before reading skill payloads', () => { + const script = buildWslSkillDiscoveryCommand([homeRoot], ['Orchestration', 'computer-use']) + + expect(script).toContain("'orchestration'|'computer-use') return 0") + expect(script).toContain('local normalized_name=${1,,}') + expect(script).toContain('metadata_name_known=0') + expect(script).toContain('IFS= read -r -n "$remaining" line || read_status=$?') + expect(script).toContain('[ "$line_length" -ge "$remaining" ] && return') + expect(script).toContain('[[ "$candidate_name" =~ $non_ascii_pattern ]] && continue') + expect(script).toContain("line=${line#$'\\xEF\\xBB\\xBF'}") + expect(script).toContain('if [ "$metadata_name_known" -eq 1 ]; then') + expect(script).toContain('done < "$1"') + expect(script).not.toContain("awk '") + expect(script).not.toContain("tr '[:upper:]'") + expect(script.indexOf('matches_requested_name "$metadata_name" || continue')).toBeLessThan( + script.indexOf('encoded_markdown=$(head') + ) + }) + + it('filters classified source kinds while parsing', () => { + const markdown = Buffer.from('---\nname: Bundled\n---\n').toString('base64') + const output = [ + record('R', '0', '1'), + record( + 'S', + '0', + '/home/alice/.codex/skills/.system/bundled/SKILL.md', + '/home/alice/.codex/skills/.system/bundled/SKILL.md', + '1700000000', + markdown + ) + ].join('') + + expect(parseWslSkillDiscoveryOutput(output, [homeRoot], 42, ['home']).skills).toEqual([]) + expect(parseWslSkillDiscoveryOutput(output, [homeRoot], 42, []).skills).toHaveLength(1) + expect(parseWslSkillDiscoveryOutput(output, [homeRoot], 42, [], [' ']).skills).toHaveLength(1) + }) + + it('keeps ASCII prefiltering for mixed-locale requested names', () => { + const script = buildWslSkillDiscoveryCommand([homeRoot], ['orchestration', 'hébergement']) + + expect(script).toContain("'orchestration') return 0") + expect(script).not.toContain('hébergement) return 0') + expect(script).toContain('is_ascii_name "$directory_name"') + }) + + it('uses the TypeScript summary parser for uncertain WSL name candidates', () => { + const blockName = Buffer.from('\uFEFF---\nname: >-\n Agent\n Orchestration\n---\n').toString( + 'base64' + ) + const headingName = Buffer.from('# Computer Use\n\nUse the computer.\n').toString('base64') + const output = [ + record('R', '0', '1'), + record( + 'S', + '0', + '/home/alice/.agents/skills/renamed-a/SKILL.md', + '/home/alice/.agents/skills/renamed-a/SKILL.md', + '1700000000', + blockName + ), + record( + 'S', + '0', + '/home/alice/.agents/skills/renamed-b/SKILL.md', + '/home/alice/.agents/skills/renamed-b/SKILL.md', + '1700000000', + headingName + ) + ].join('') + + expect( + parseWslSkillDiscoveryOutput( + output, + [homeRoot], + 42, + ['home'], + ['agent orchestration', 'computer use'] + ).skills.map((skill) => skill.name) + ).toEqual(['Agent Orchestration', 'Computer Use']) + }) + it('rejects malformed host responses instead of reporting an empty scan', () => { expect(() => parseWslSkillDiscoveryOutput(record('S', '9'), [homeRoot])).toThrow( 'unknown source' diff --git a/src/main/skills/skill-discovery-wsl.ts b/src/main/skills/skill-discovery-wsl.ts index eae829a893f..23524bff02f 100644 --- a/src/main/skills/skill-discovery-wsl.ts +++ b/src/main/skills/skill-discovery-wsl.ts @@ -1,21 +1,15 @@ +import { + readWslSkillDiscoveryObservation, + projectWslSkillDiscovery, + type WslSkillDiscoveryObservation +} from './skill-discovery-wsl-observation' +export { parseWslSkillDiscoveryOutput } from './skill-discovery-wsl-observation' import { posix as pathPosix } from 'node:path' -import { summarizeSkillMarkdown } from '../../shared/skill-metadata' -import type { - DiscoveredSkill, - SkillDiscoveryResult, - SkillDiscoverySource -} from '../../shared/skills' +import type { SkillDiscoveryResult, SkillSourceKind } from '../../shared/skills' import { quoteBashString } from '../wsl-bash-command' import { runWslProcess } from '../wsl/wsl-runner' -import { - buildSkillDiscoverySources, - sortDiscoveredSkills, - sortSkillDiscoverySources, - sourceKindForSkill, - sourceLabelForSkill, - stablePathId, - type SkillScanRoot -} from './skill-discovery-sources' +import { buildSkillDiscoverySources, type SkillScanRoot } from './skill-discovery-sources' +import { rootMayContainSourceKind } from './skill-discovery-source-filter' import { discoverClaudePluginSkillSourcesInWsl } from './claude-plugin-skill-sources-wsl' import type { SkillProviderRootOverrides } from './skill-provider-destinations' import { SKILL_STAGING_GLOB } from './skill-delete/staging-names' @@ -25,10 +19,95 @@ const MAX_MARKDOWN_BYTES = 256 * 1024 const WSL_SCAN_TIMEOUT_MS = 10_000 const WSL_SCAN_MAX_OUTPUT_BYTES = 128 * 1024 * 1024 -export function buildWslSkillDiscoveryCommand(roots: readonly SkillScanRoot[]): string { +export function buildWslSkillDiscoveryCommand( + roots: readonly SkillScanRoot[], + names?: readonly string[] +): string { + const normalizedNames = names?.map((name) => name.trim().toLowerCase()).filter(Boolean) + const nameFilterHelpers: string[] = [] + const nameFilterBody: string[] = [] + if (normalizedNames?.length) { + const asciiNames = [...new Set(normalizedNames.filter((name) => /^[\x20-\x7e]+$/.test(name)))] + const matchBody = asciiNames.length + ? [ + ' case "$normalized_name" in', + ` ${asciiNames.map(quoteBashString).join('|')}) return 0 ;;`, + ' *) return 1 ;;', + ' esac' + ] + : [' return 1'] + nameFilterHelpers.push( + 'is_ascii_name() {', + " local LC_ALL=C non_ascii_pattern='[^ -~]'", + ' if [[ "$1" =~ $non_ascii_pattern ]]; then return 1; fi', + ' return 0', + '}', + 'matches_requested_name() {', + ' local LC_ALL=C', + ' local normalized_name=${1,,}', + ' while [[ "$normalized_name" == \' \'* ]]; do normalized_name=${normalized_name#?}; done', + ' while [[ "$normalized_name" == *\' \' ]]; do normalized_name=${normalized_name%?}; done', + ...matchBody, + '}', + 'read_frontmatter_name() {', + ' metadata_name=', + ' metadata_name_known=0', + ` local LC_ALL=C line first_line=1 remaining=${MAX_MARKDOWN_BYTES}`, + " local read_status line_length candidate_name= candidate_name_known=0 non_ascii_pattern='[^ -~]'", + ' while [ "$remaining" -gt 0 ]; do', + ' line=', + ' read_status=0', + ' IFS= read -r -n "$remaining" line || read_status=$?', + ' line_length=${#line}', + ' [ "$line_length" -ge "$remaining" ] && return', + ' [ "$read_status" -eq 0 ] || return', + ' remaining=$((remaining - line_length - 1))', + " line=${line%$'\\r'}", + ' if [ "$first_line" -eq 1 ]; then', + ' first_line=0', + " line=${line#$'\\xEF\\xBB\\xBF'}", + ' [[ "$line" =~ ^---[[:space:]]*$ ]] || return', + ' continue', + ' fi', + ' if [[ "$line" =~ ^---[[:space:]]*$ ]]; then', + ' metadata_name=$candidate_name', + ' metadata_name_known=$candidate_name_known', + ' return', + ' fi', + ' if [[ "$line" =~ ^name:[[:space:]]*(.*)$ ]]; then', + ' candidate_name=${BASH_REMATCH[1]}', + ' candidate_name_known=0', + ' while [[ "$candidate_name" == [[:space:]]* ]]; do candidate_name=${candidate_name#?}; done', + ' while [[ "$candidate_name" == *[[:space:]] ]]; do candidate_name=${candidate_name%?}; done', + ' case "$candidate_name" in ""|"|"|"|-"|">"|">-") continue ;; esac', + ' local quote=${candidate_name:0:1}', + ` if [ "\${#candidate_name}" -eq 1 ] && { [ "$quote" = '"' ] || [ "$quote" = "'" ]; }; then continue; fi`, + ` if [ "\${#candidate_name}" -ge 2 ] && { [ "$quote" = '"' ] || [ "$quote" = "'" ]; } && [ "\${candidate_name: -1}" = "$quote" ]; then`, + ' candidate_name=${candidate_name:1:${#candidate_name}-2}', + ' fi', + ' while [[ "$candidate_name" == [[:space:]]* ]]; do candidate_name=${candidate_name#?}; done', + ' while [[ "$candidate_name" == *[[:space:]] ]]; do candidate_name=${candidate_name%?}; done', + ' [ -n "$candidate_name" ] || continue', + ' [[ "$candidate_name" =~ $non_ascii_pattern ]] && continue', + ' candidate_name_known=1', + ' fi', + ' done < "$1"', + '}' + ) + nameFilterBody.push( + ' directory_name=${directory_path##*/}', + ' if is_ascii_name "$directory_name" && ! matches_requested_name "$directory_name"; then', + ' read_frontmatter_name "$skill_file"', + ' if [ "$metadata_name_known" -eq 1 ]; then', + ' matches_requested_name "$metadata_name" || continue', + ' fi', + ' fi' + ) + } const lines = [ 'set -u', 'set -o pipefail', + ...nameFilterHelpers, 'scan_root() {', ' root_index=$1', ' root_path=$2', @@ -40,6 +119,8 @@ export function buildWslSkillDiscoveryCommand(roots: readonly SkillScanRoot[]): ` printf '%s\\0%s\\0%s\\0' R "$root_index" 1`, ` while IFS= read -r -d '' skill_file; do`, ` canonical_path=$(realpath -- "$skill_file" 2>/dev/null || printf '%s' "$skill_file")`, + ` directory_path=\${skill_file%/*}`, + ...nameFilterBody, ` updated_at=$(stat -c '%Y' -- "$skill_file" 2>/dev/null || true)`, ` encoded_markdown=$(head -c ${MAX_MARKDOWN_BYTES} -- "$skill_file" 2>/dev/null | base64 | tr -d '\\n') || continue`, ` printf '%s\\0%s\\0%s\\0%s\\0%s\\0' S "$root_index" "$skill_file" "$canonical_path" "$updated_at"`, @@ -77,104 +158,28 @@ async function executeWslSkillDiscovery(distro: string, script: string): Promise return result.stdout } -function readProtocolField(fields: string[], index: number): string { - const value = fields[index] - if (value === undefined) { - throw new Error('WSL skill discovery returned an incomplete response.') - } - return value -} - -export function parseWslSkillDiscoveryOutput( - output: string, - roots: readonly SkillScanRoot[], - scannedAt = Date.now() -): SkillDiscoveryResult { - const fields = output.split('\0') - const rootExists = new Map() - const skillsByCanonicalPath = new Map() - let index = 0 - while (index < fields.length && fields[index]) { - const recordKind = fields[index++] - const rootIndex = Number.parseInt(readProtocolField(fields, index++), 10) - const root = roots[rootIndex] - if (!root) { - throw new Error('WSL skill discovery returned an unknown source.') - } - if (recordKind === 'R') { - rootExists.set(rootIndex, readProtocolField(fields, index++) === '1') - continue - } - if (recordKind !== 'S') { - throw new Error('WSL skill discovery returned an invalid response.') - } - - const skillFilePath = readProtocolField(fields, index++) - const canonicalSkillFilePath = readProtocolField(fields, index++) - const updatedAtSeconds = Number.parseInt(readProtocolField(fields, index++), 10) - const markdown = Buffer.from(readProtocolField(fields, index++), 'base64').toString('utf8') - const existing = skillsByCanonicalPath.get(canonicalSkillFilePath) - if (existing) { - // Why: dedup keeps one row, but every contributing root must survive so - // per-agent visibility does not depend on root scan order. providers is - // per-agent visibility too, so union it rather than keeping only the first. - if (existing.rootPaths && !existing.rootPaths.includes(root.path)) { - existing.rootPaths.push(root.path) - } - // Reassign a fresh array — `providers` aliases the scan root's array, so - // pushing in place would mutate the root and sibling skills/sources. - const mergedProviders = [...existing.providers] - for (const provider of root.providers) { - if (!mergedProviders.includes(provider)) { - mergedProviders.push(provider) - } - } - existing.providers = mergedProviders - continue - } - const directoryPath = pathPosix.dirname(skillFilePath) - const summary = summarizeSkillMarkdown(markdown) - const sourceKind = sourceKindForSkill(root, skillFilePath, pathPosix) - skillsByCanonicalPath.set(canonicalSkillFilePath, { - id: stablePathId(canonicalSkillFilePath), - name: summary.name ?? pathPosix.basename(directoryPath), - description: summary.description, - // Copy: `root.providers` is shared across every skill/source from this - // root, so a later in-place merge must not mutate the aliased array. - providers: [...root.providers], - sourceKind, - sourceLabel: sourceLabelForSkill(root, sourceKind), - rootPath: root.path, - rootPaths: [root.path], - directoryPath, - skillFilePath, - installed: true, - updatedAt: Number.isFinite(updatedAtSeconds) ? updatedAtSeconds * 1000 : null - }) - } - - const sources: SkillDiscoverySource[] = roots.map((root, rootIndex) => { - const exists = rootExists.get(rootIndex) ?? false - return { - ...root, - providers: [...root.providers], - exists, - skippedReason: exists ? undefined : 'missing' - } - }) - return { - skills: sortDiscoveredSkills([...skillsByCanonicalPath.values()]), - sources: sortSkillDiscoverySources(sources), - scannedAt - } -} - -export async function discoverSkillsInWsl(args: { +type WslSkillDiscoveryArgs = { distro: string homeDir: string - cwd: string + cwd?: string + names?: string[] + sourceKinds?: SkillSourceKind[] providerRootOverrides?: SkillProviderRootOverrides -}): Promise { +} + +export async function discoverSkillsInWsl( + args: WslSkillDiscoveryArgs +): Promise { + return projectWslSkillDiscovery( + await discoverSkillObservationInWsl(args), + args.sourceKinds, + args.names + ) +} + +export async function discoverSkillObservationInWsl( + args: WslSkillDiscoveryArgs +): Promise { // Plugin roots are resolved (in JS) from metadata this first wsl.exe call // reads, then fed to the scan's own wsl.exe call below — two sequential // process boots. That is a deliberate one-time-per-pane cost (the renderer @@ -184,24 +189,31 @@ export async function discoverSkillsInWsl(args: { // Why: plugin-metadata enrichment is optional. A failed/timed-out read must // degrade to zero plugin roots (matching the native readMetadataFile path), // not abort the mandatory native/home/repo/bundled scan. + const cwd = args.cwd ?? args.homeDir let pluginRoots: SkillScanRoot[] = [] - try { - pluginRoots = await discoverClaudePluginSkillSourcesInWsl(args) - } catch { - pluginRoots = [] + if (!args.sourceKinds?.length || args.sourceKinds.includes('plugin')) { + try { + pluginRoots = await discoverClaudePluginSkillSourcesInWsl({ ...args, cwd }) + } catch { + pluginRoots = [] + } } const roots = [ ...buildSkillDiscoverySources({ homeDir: args.homeDir, - cwd: args.cwd, + cwd, repos: [], + includeCwd: true, pathApi: pathPosix, providerRootOverrides: args.providerRootOverrides }), ...pluginRoots - ] + ].filter((root) => rootMayContainSourceKind(root, args.sourceKinds)) // Why: UNC traversal applies Windows casing and symlink rules. The distro // must own enumeration, metadata reads, and canonical path identity. - const output = await executeWslSkillDiscovery(args.distro, buildWslSkillDiscoveryCommand(roots)) - return parseWslSkillDiscoveryOutput(output, roots) + const output = await executeWslSkillDiscovery( + args.distro, + buildWslSkillDiscoveryCommand(roots, args.names) + ) + return readWslSkillDiscoveryObservation(output, roots) } diff --git a/src/renderer/src/hooks/installed-agent-skill-discovery-cache.ts b/src/renderer/src/hooks/installed-agent-skill-discovery-cache.ts index d678916cdbe..898ef73f815 100644 --- a/src/renderer/src/hooks/installed-agent-skill-discovery-cache.ts +++ b/src/renderer/src/hooks/installed-agent-skill-discovery-cache.ts @@ -67,8 +67,12 @@ export function clearInstalledAgentSkillDiscoveryCache(): void { cachedDiscoveryByTarget.clear() } -export function deleteInstalledAgentSkillDiscoveryCache(key: string): void { - cachedDiscoveryByTarget.delete(key) +export function deleteInstalledAgentSkillDiscoveryCache(matches: (key: string) => boolean): void { + for (const key of cachedDiscoveryByTarget.keys()) { + if (matches(key)) { + cachedDiscoveryByTarget.delete(key) + } + } } export function getInstalledAgentSkillDiscoveryCacheSizeForTests(): number { diff --git a/src/renderer/src/hooks/installed-agent-skill-discovery.test.ts b/src/renderer/src/hooks/installed-agent-skill-discovery.test.ts index fc2067c970b..f55623f87b0 100644 --- a/src/renderer/src/hooks/installed-agent-skill-discovery.test.ts +++ b/src/renderer/src/hooks/installed-agent-skill-discovery.test.ts @@ -231,6 +231,47 @@ describe('installed agent skill discovery lifecycle', () => { 'repo-1:repair:wsl-distro-required:default' ) }) + + it('normalizes effective filters into local, project, and environment keys', () => { + const target = { + projectRuntime: resolvedWslProjectRuntime, + names: [' Computer-Use ', 'orchestration', 'computer-use'], + sourceKinds: ['home' as const] + } + const normalizedFilters = [['computer-use', 'orchestration'], ['home']] + + expect(getSkillDiscoveryTargetKey(target)).toBe( + JSON.stringify(['repo-1:wsl:Ubuntu', ...normalizedFilters]) + ) + expect(getRuntimeScopedSkillDiscoveryKey(remote('env-a'), target)).toBe( + JSON.stringify(['runtime:env-a', ...normalizedFilters]) + ) + expect(getSkillDiscoveryTargetKey(target, [], [])).toBe( + JSON.stringify(['repo-1:wsl:Ubuntu', ...normalizedFilters]) + ) + expect(getSkillDiscoveryTargetKey(target, ['ORCHESTRATION'], [])).toBe( + JSON.stringify(['repo-1:wsl:Ubuntu', ['orchestration'], ['home']]) + ) + }) + + it('isolates project-runtime caches by target-contained filters', async () => { + discoverSkillsForRuntimeTarget.mockResolvedValueOnce(result(1)).mockResolvedValueOnce(result(2)) + + await expect( + discoverInstalledAgentSkills(false, { + projectRuntime: resolvedWslProjectRuntime, + names: ['orchestration'] + }) + ).resolves.toEqual(result(1)) + await expect( + discoverInstalledAgentSkills(false, { + projectRuntime: resolvedWslProjectRuntime, + names: ['computer-use'] + }) + ).resolves.toEqual(result(2)) + + expect(discoverSkillsForRuntimeTarget).toHaveBeenCalledTimes(2) + }) it('keys the bounded cache by runtime scope, not by the client target', async () => { // Why: #6887 scopes remote scans by environment. The cap rewrites this same // module, so pin that getRuntimeScopedSkillDiscoveryKey stays the producer — diff --git a/src/renderer/src/hooks/installed-agent-skill-discovery.ts b/src/renderer/src/hooks/installed-agent-skill-discovery.ts index a5d992e1694..192219a98e0 100644 --- a/src/renderer/src/hooks/installed-agent-skill-discovery.ts +++ b/src/renderer/src/hooks/installed-agent-skill-discovery.ts @@ -1,4 +1,8 @@ -import type { SkillDiscoveryResult, SkillDiscoveryTarget } from '../../../shared/skills' +import type { + SkillDiscoveryResult, + SkillDiscoveryTarget, + SkillSourceKind +} from '../../../shared/skills' import type { RuntimeClientTarget } from '@/runtime/runtime-rpc-client' import { discoverSkillsForRuntimeTarget } from '@/runtime/runtime-skills-client' import { INSTALLED_AGENT_SKILLS_CHANGED_EVENT } from './installed-agent-skills-change-event' @@ -44,9 +48,16 @@ export function evictInstalledAgentSkillDiscoveryForRuntimeEnvironments( ): void { for (const environmentId of environmentIds) { const key = getRuntimeScopedSkillDiscoveryKey({ kind: 'environment', environmentId }, undefined) - deleteInstalledAgentSkillDiscoveryCache(key) - pendingDiscoveryByTarget.delete(key) - pendingDiscoverySatisfiesForcedRefreshByTarget.delete(key) + const filteredPrefix = `[${JSON.stringify(key)},` + const belongsToRuntime = (candidate: string): boolean => + candidate === key || candidate.startsWith(filteredPrefix) + deleteInstalledAgentSkillDiscoveryCache(belongsToRuntime) + for (const pendingKey of pendingDiscoveryByTarget.keys()) { + if (belongsToRuntime(pendingKey)) { + pendingDiscoveryByTarget.delete(pendingKey) + pendingDiscoverySatisfiesForcedRefreshByTarget.delete(pendingKey) + } + } } } @@ -58,40 +69,78 @@ export function resetSkillDiscoveryCacheForTests(): void { } function normalizeSkillDiscoveryTarget( - target: SkillDiscoveryTarget | undefined + target: SkillDiscoveryTarget | undefined, + names?: readonly string[], + sourceKinds?: readonly SkillSourceKind[] ): SkillDiscoveryTarget | undefined { + const requestedNames = names?.map((name) => name.trim().toLowerCase()).filter(Boolean) ?? [] + const targetNames = target?.names?.map((name) => name.trim().toLowerCase()).filter(Boolean) ?? [] + const effectiveNames = [...new Set(requestedNames.length > 0 ? requestedNames : targetNames)] + const effectiveSourceKinds = [ + ...new Set(sourceKinds?.length ? sourceKinds : (target?.sourceKinds ?? [])) + ] + const filters = { + ...(effectiveNames?.length ? { names: [...effectiveNames] } : {}), + ...(effectiveSourceKinds?.length ? { sourceKinds: [...effectiveSourceKinds] } : {}) + } const projectRuntime = target?.projectRuntime if (projectRuntime) { if (projectRuntime.status === 'repair-required') { - return { projectRuntime } + return { projectRuntime, ...filters } } if (projectRuntime.runtime.kind === 'wsl') { return { runtime: 'wsl', wslDistro: projectRuntime.runtime.distro, - projectRuntime + projectRuntime, + ...filters } } return { runtime: 'host', - projectRuntime + projectRuntime, + ...filters } } if (target?.runtime !== 'wsl') { - return undefined + return Object.keys(filters).length > 0 ? filters : undefined } - return { runtime: 'wsl', wslDistro: target.wslDistro?.trim() || null } + return { runtime: 'wsl', wslDistro: target.wslDistro?.trim() || null, ...filters } } -export function getSkillDiscoveryTargetKey(target: SkillDiscoveryTarget | undefined): string { +function appendSkillDiscoveryFiltersToKey( + runtimeKey: string, + target: SkillDiscoveryTarget | undefined +): string { + return target?.names?.length || target?.sourceKinds?.length + ? JSON.stringify([ + runtimeKey, + target.names ? [...target.names].sort() : null, + target.sourceKinds ? [...target.sourceKinds].sort() : null + ]) + : runtimeKey +} + +function getNormalizedSkillDiscoveryRuntimeKey(target: SkillDiscoveryTarget | undefined): string { if (target?.projectRuntime) { return target.projectRuntime.status === 'resolved' ? target.projectRuntime.runtime.cacheKey : target.projectRuntime.repair.cacheKey } - const normalizedTarget = normalizeSkillDiscoveryTarget(target) - return normalizedTarget?.runtime === 'wsl' ? `wsl:${normalizedTarget.wslDistro ?? ''}` : 'host' + return target?.runtime === 'wsl' ? `wsl:${target.wslDistro ?? ''}` : 'host' +} + +export function getSkillDiscoveryTargetKey( + target: SkillDiscoveryTarget | undefined, + names?: readonly string[], + sourceKinds?: readonly SkillSourceKind[] +): string { + const normalizedTarget = normalizeSkillDiscoveryTarget(target, names, sourceKinds) + return appendSkillDiscoveryFiltersToKey( + getNormalizedSkillDiscoveryRuntimeKey(normalizedTarget), + normalizedTarget + ) } // Why: a connected remote runtime scans its own disk. Sharing the local key @@ -101,21 +150,28 @@ export function getSkillDiscoveryTargetKey(target: SkillDiscoveryTarget | undefi // remote gets rescanned once per client-side target shape. export function getRuntimeScopedSkillDiscoveryKey( runtimeTarget: RuntimeClientTarget, - target: SkillDiscoveryTarget | undefined + target: SkillDiscoveryTarget | undefined, + names?: readonly string[], + sourceKinds?: readonly SkillSourceKind[] ): string { - return runtimeTarget.kind === 'environment' - ? `runtime:${runtimeTarget.environmentId}` - : getSkillDiscoveryTargetKey(target) + const normalizedTarget = normalizeSkillDiscoveryTarget(target, names, sourceKinds) + const runtimeKey = + runtimeTarget.kind === 'environment' + ? `runtime:${runtimeTarget.environmentId}` + : getNormalizedSkillDiscoveryRuntimeKey(normalizedTarget) + return appendSkillDiscoveryFiltersToKey(runtimeKey, normalizedTarget) } function startInstalledAgentSkillDiscovery( force: boolean, target: SkillDiscoveryTarget | undefined, runtimeTarget: RuntimeClientTarget, - key: string + key: string, + names?: readonly string[], + sourceKinds?: readonly SkillSourceKind[] ): Promise { const generation = discoveryGeneration - const normalizedTarget = normalizeSkillDiscoveryTarget(target) + const normalizedTarget = normalizeSkillDiscoveryTarget(target, names, sourceKinds) // Why: a forced caller knows disk changed (install finished, explicit recheck), // so it must also bypass the host's shared scans — not just this window's cache. const requestTarget = force ? { ...normalizedTarget, refresh: true } : normalizedTarget @@ -144,9 +200,11 @@ function startInstalledAgentSkillDiscovery( export async function discoverInstalledAgentSkills( force: boolean, target?: SkillDiscoveryTarget, - runtimeTarget: RuntimeClientTarget = LOCAL_RUNTIME_TARGET + runtimeTarget: RuntimeClientTarget = LOCAL_RUNTIME_TARGET, + names?: readonly string[], + sourceKinds?: readonly SkillSourceKind[] ): Promise { - const key = getRuntimeScopedSkillDiscoveryKey(runtimeTarget, target) + const key = getRuntimeScopedSkillDiscoveryKey(runtimeTarget, target, names, sourceKinds) if (!force) { // Why: only a cache-serving read should refresh recency — a forced refresh // discards the entry it would otherwise promote. @@ -173,5 +231,5 @@ export async function discoverInstalledAgentSkills( } } - return startInstalledAgentSkillDiscovery(force, target, runtimeTarget, key) + return startInstalledAgentSkillDiscovery(force, target, runtimeTarget, key, names, sourceKinds) } diff --git a/src/renderer/src/hooks/installed-agent-skill-runtime-eviction.test.ts b/src/renderer/src/hooks/installed-agent-skill-runtime-eviction.test.ts new file mode 100644 index 00000000000..f6c31cbbee0 --- /dev/null +++ b/src/renderer/src/hooks/installed-agent-skill-runtime-eviction.test.ts @@ -0,0 +1,74 @@ +import { afterEach, expect, it, vi } from 'vitest' +import type { SkillDiscoveryResult } from '../../../shared/skills' + +const discover = vi.hoisted(() => vi.fn<() => Promise>()) +vi.mock('@/runtime/runtime-skills-client', () => ({ discoverSkillsForRuntimeTarget: discover })) + +import { + discoverInstalledAgentSkills, + evictInstalledAgentSkillDiscoveryForRuntimeEnvironments, + getCachedSkillDiscovery, + getRuntimeScopedSkillDiscoveryKey, + resetSkillDiscoveryCacheForTests +} from './installed-agent-skill-discovery' + +const result = (scannedAt: number): SkillDiscoveryResult => ({ skills: [], sources: [], scannedAt }) +const runtime = (environmentId: string) => ({ kind: 'environment' as const, environmentId }) + +afterEach(() => { + resetSkillDiscoveryCacheForTests() + discover.mockReset() +}) + +it.each(['env', 'env"[,]'])( + 'evicts every filtered scan for %s without evicting other owners', + async (id) => { + const owner = runtime(id) + const other = runtime(`${id}-other`) + discover.mockResolvedValue(result(1)) + for (const names of [['alpha'], ['beta'], undefined]) { + await discoverInstalledAgentSkills(false, undefined, owner, names, ['home']) + } + await discoverInstalledAgentSkills(false, undefined, other, ['alpha'], ['home']) + await discoverInstalledAgentSkills(false, undefined, { kind: 'local' }, ['alpha'], ['home']) + + evictInstalledAgentSkillDiscoveryForRuntimeEnvironments([id]) + discover.mockResolvedValue(result(2)) + for (const names of [['alpha'], ['beta'], undefined]) { + const key = getRuntimeScopedSkillDiscoveryKey(owner, undefined, names, ['home']) + expect(getCachedSkillDiscovery(key)).toBeNull() + await expect( + discoverInstalledAgentSkills(false, undefined, owner, names, ['home']) + ).resolves.toEqual(result(2)) + } + await expect( + discoverInstalledAgentSkills(false, undefined, other, ['alpha'], ['home']) + ).resolves.toEqual(result(1)) + await expect( + discoverInstalledAgentSkills(false, undefined, { kind: 'local' }, ['alpha'], ['home']) + ).resolves.toEqual(result(1)) + expect(discover).toHaveBeenCalledTimes(8) + } +) + +it('keeps a retired filtered response from replacing the new peer cache', async () => { + let finishOld!: (value: SkillDiscoveryResult) => void + discover.mockReturnValueOnce( + new Promise((resolve) => { + finishOld = resolve + }) + ) + const owner = runtime('env') + const old = discoverInstalledAgentSkills(false, undefined, owner, ['alpha'], ['home']) + evictInstalledAgentSkillDiscoveryForRuntimeEnvironments(['env']) + discover.mockResolvedValue(result(2)) + await expect( + discoverInstalledAgentSkills(false, undefined, owner, ['alpha'], ['home']) + ).resolves.toEqual(result(2)) + finishOld(result(1)) + await old + await expect( + discoverInstalledAgentSkills(false, undefined, owner, ['alpha'], ['home']) + ).resolves.toEqual(result(2)) + expect(discover).toHaveBeenCalledTimes(2) +}) diff --git a/src/renderer/src/hooks/useInstalledAgentSkills.react.test.tsx b/src/renderer/src/hooks/useInstalledAgentSkills.react.test.tsx index f71e3a6242b..f8fd67e6b43 100644 --- a/src/renderer/src/hooks/useInstalledAgentSkills.react.test.tsx +++ b/src/renderer/src/hooks/useInstalledAgentSkills.react.test.tsx @@ -305,8 +305,16 @@ describe('useInstalledAgentSkill', () => { }) expect(latestState?.installed).toBe(false) - expect(discover).toHaveBeenNthCalledWith(1, undefined) - expect(discover).toHaveBeenNthCalledWith(2, { runtime: 'wsl', wslDistro: 'Fedora' }) + expect(discover).toHaveBeenNthCalledWith(1, { + names: ['orca-linear', 'linear-tickets'], + sourceKinds: ['home'] + }) + expect(discover).toHaveBeenNthCalledWith(2, { + runtime: 'wsl', + wslDistro: 'Fedora', + names: ['orca-linear', 'linear-tickets'], + sourceKinds: ['home'] + }) }) it('ignores same-target background discovery results when a forced refresh is waiting', async () => { @@ -340,9 +348,16 @@ describe('useInstalledAgentSkill', () => { }) expect(latestState?.installed).toBe(false) - expect(discover).toHaveBeenNthCalledWith(1, undefined) + expect(discover).toHaveBeenNthCalledWith(1, { + names: ['orca-linear', 'linear-tickets'], + sourceKinds: ['home'] + }) // A forced refresh must also bypass the host's shared scans, not just this cache. - expect(discover).toHaveBeenNthCalledWith(2, { refresh: true }) + expect(discover).toHaveBeenNthCalledWith(2, { + names: ['orca-linear', 'linear-tickets'], + sourceKinds: ['home'], + refresh: true + }) }) it('returns installed from refresh when a legacy Linear skill is discovered', async () => { @@ -390,7 +405,12 @@ describe('useInstalledAgentSkill', () => { }) expect(latestState?.installed).toBe(true) - expect(discover).toHaveBeenCalledWith({ runtime: 'wsl', wslDistro: 'Ubuntu' }) + expect(discover).toHaveBeenCalledWith({ + runtime: 'wsl', + wslDistro: 'Ubuntu', + names: ['orca-linear', 'linear-tickets'], + sourceKinds: ['home'] + }) }) it('detects a legacy Linear install through project-runtime skill discovery', async () => { @@ -411,7 +431,9 @@ describe('useInstalledAgentSkill', () => { expect(discover).toHaveBeenCalledWith({ runtime: 'wsl', wslDistro: 'Ubuntu', - projectRuntime: projectWslRuntime + projectRuntime: projectWslRuntime, + names: ['orca-linear', 'linear-tickets'], + sourceKinds: ['home'] }) }) @@ -479,7 +501,10 @@ describe('useInstalledAgentSkill', () => { // The freshness window is what bounds the storm; past it, focus still reads disk. expect(discover).toHaveBeenCalledTimes(2) - expect(discover).toHaveBeenLastCalledWith(undefined) + expect(discover).toHaveBeenLastCalledWith({ + names: ['orca-linear', 'linear-tickets'], + sourceKinds: ['home'] + }) }) it('reuses cached discovery when another surface finishes re-checking', async () => { diff --git a/src/renderer/src/hooks/useInstalledAgentSkills.test.ts b/src/renderer/src/hooks/useInstalledAgentSkills.test.ts index d3989662d56..a3340a104e2 100644 --- a/src/renderer/src/hooks/useInstalledAgentSkills.test.ts +++ b/src/renderer/src/hooks/useInstalledAgentSkills.test.ts @@ -324,6 +324,44 @@ describe('discoverInstalledAgentSkills', () => { expect(discover).toHaveBeenNthCalledWith(2, { runtime: 'wsl', wslDistro: null }) }) + it('forwards filters and isolates filtered discovery caches', async () => { + const orchestrationResult = discoveryResult([skill({ name: 'orchestration' })]) + const computerUseResult = discoveryResult([skill({ name: 'computer-use' })]) + const discover = vi + .fn() + .mockResolvedValueOnce(orchestrationResult) + .mockResolvedValueOnce(computerUseResult) + vi.stubGlobal('window', { api: { skills: { discover } } }) + + await _installedAgentSkillDiscoveryInternalsForTests.discoverInstalledAgentSkills( + false, + { runtime: 'wsl', wslDistro: 'Ubuntu' }, + undefined, + ['orchestration'], + GLOBAL_AGENT_SKILL_SOURCE_KINDS + ) + await _installedAgentSkillDiscoveryInternalsForTests.discoverInstalledAgentSkills( + false, + { runtime: 'wsl', wslDistro: 'Ubuntu' }, + undefined, + ['computer-use'], + GLOBAL_AGENT_SKILL_SOURCE_KINDS + ) + + expect(discover).toHaveBeenNthCalledWith(1, { + runtime: 'wsl', + wslDistro: 'Ubuntu', + names: ['orchestration'], + sourceKinds: ['home'] + }) + expect(discover).toHaveBeenNthCalledWith(2, { + runtime: 'wsl', + wslDistro: 'Ubuntu', + names: ['computer-use'], + sourceKinds: ['home'] + }) + }) + it('forwards project runtime targets to skill discovery', async () => { const wslResult = discoveryResult([skill({ name: 'wsl-skill' })]) const discover = vi.fn().mockResolvedValueOnce(wslResult) diff --git a/src/renderer/src/hooks/useInstalledAgentSkills.ts b/src/renderer/src/hooks/useInstalledAgentSkills.ts index 900a21c1dc1..d7ac999a873 100644 --- a/src/renderer/src/hooks/useInstalledAgentSkills.ts +++ b/src/renderer/src/hooks/useInstalledAgentSkills.ts @@ -140,7 +140,12 @@ export function useInstalledAgentSkillNames( const candidateSkillNames = useMemo(() => skillNamesKey.split('\n'), [skillNamesKey]) const runtimeTarget = useActiveSkillDiscoveryRuntimeTarget() const discoveryTargetKey = runtimeTarget - ? getRuntimeScopedSkillDiscoveryKey(runtimeTarget, discoveryTarget) + ? getRuntimeScopedSkillDiscoveryKey( + runtimeTarget, + discoveryTarget, + candidateSkillNames, + sourceKinds + ) : UNRESOLVED_RUNTIME_DISCOVERY_KEY // Why: callers derive the target inside a store-backed useMemo, so unrelated // store writes hand us a new object with the same key. Two targets with the @@ -229,7 +234,13 @@ export function useInstalledAgentSkillNames( } let installedAfterRefresh = false try { - const next = await discoverInstalledAgentSkills(force, stableDiscoveryTarget, runtimeTarget) + const next = await discoverInstalledAgentSkills( + force, + stableDiscoveryTarget, + runtimeTarget, + candidateSkillNames, + sourceKinds + ) installedAfterRefresh = hasInstalledAgentSkillNamed(next.skills, candidateSkillNames, { sourceKinds }) diff --git a/src/renderer/src/runtime/runtime-skills-client.test.ts b/src/renderer/src/runtime/runtime-skills-client.test.ts index 2ccb1118843..f3927dead7a 100644 --- a/src/renderer/src/runtime/runtime-skills-client.test.ts +++ b/src/renderer/src/runtime/runtime-skills-client.test.ts @@ -80,6 +80,28 @@ describe('discoverSkillsForRuntimeTarget', () => { ) }) + it('forwards portable inventory filters without client runtime identity', async () => { + const result = discoveryResult('orchestration') + runtimeEnvironmentCall.mockResolvedValueOnce({ id: 'skills', ok: true, result }) + + await discoverSkillsForRuntimeTarget( + { kind: 'environment', environmentId: 'env-1' }, + { + runtime: 'wsl', + wslDistro: 'Ubuntu', + names: ['orchestration'], + sourceKinds: ['home'] + } + ) + + expect(runtimeEnvironmentCall).toHaveBeenCalledWith( + expect.objectContaining({ + method: 'skills.discover', + params: { names: ['orchestration'], sourceKinds: ['home'] } + }) + ) + }) + // Why: no caller can produce these yet, so the remote params must stay empty // rather than shipping a client-host target the server would misread. it('sends no target at all to a remote runtime', async () => { diff --git a/src/renderer/src/runtime/runtime-skills-client.ts b/src/renderer/src/runtime/runtime-skills-client.ts index 7d825154f97..33cade9ef55 100644 --- a/src/renderer/src/runtime/runtime-skills-client.ts +++ b/src/renderer/src/runtime/runtime-skills-client.ts @@ -34,6 +34,7 @@ const SKILL_DISCOVERY_TIMEOUT_MS = 15_000 * `refresh` is the exception and must be forwarded: it describes the *request*, * not the client's host, and it is the only way an explicit re-check reaches * past the remote host's shared scans to its disk. + * Portable inventory filters also apply to the remote disk being scanned. */ export async function discoverSkillsForRuntimeTarget( runtimeTarget: RuntimeClientTarget, @@ -45,7 +46,11 @@ export async function discoverSkillsForRuntimeTarget( return callRuntimeRpc( runtimeTarget, 'skills.discover', - target?.refresh ? { refresh: true } : {}, + { + ...(target?.refresh ? { refresh: true } : {}), + ...(target?.names?.length ? { names: target.names } : {}), + ...(target?.sourceKinds?.length ? { sourceKinds: target.sourceKinds } : {}) + }, { timeoutMs: SKILL_DISCOVERY_TIMEOUT_MS } ) } diff --git a/src/shared/skill-metadata.test.ts b/src/shared/skill-metadata.test.ts index 73855bf6d53..5278fbcd1c8 100644 --- a/src/shared/skill-metadata.test.ts +++ b/src/shared/skill-metadata.test.ts @@ -31,3 +31,16 @@ Use when reviewing UI implementation quality. }) }) }) + +it.each(['', '\uFEFF'])('preserves the last CRLF frontmatter field with BOM %j', (bom) => { + for (const fields of [ + ['name: actual-name', 'description: actual description'], + ['description: actual description', 'name: actual-name'] + ]) { + const markdown = `${bom}${['---', ...fields, '---', '# Fallback heading', '', 'Fallback paragraph'].join('\r\n')}` + expect(summarizeSkillMarkdown(markdown)).toEqual({ + name: 'actual-name', + description: 'actual description' + }) + } +}) diff --git a/src/shared/skill-metadata.ts b/src/shared/skill-metadata.ts index aa4e4f56c6b..7ffc1d8341b 100644 --- a/src/shared/skill-metadata.ts +++ b/src/shared/skill-metadata.ts @@ -14,7 +14,7 @@ function stripQuotePair(value: string): string { } function parseYamlFrontmatter(raw: string): Record { - const lines = raw.replace(/\r\n/g, '\n').split('\n') + const lines = raw.replace(/\r\n/g, '\n').replace(/\r$/, '').split('\n') const data: Record = {} let index = 0 while (index < lines.length) { diff --git a/src/shared/skills.ts b/src/shared/skills.ts index 26b642d6495..5a980219885 100644 --- a/src/shared/skills.ts +++ b/src/shared/skills.ts @@ -54,6 +54,9 @@ export type SkillDiscoveryTarget = { /** Bypass the host's shared scans because the caller knows disk just changed. * Optional so an older host simply ignores it and scans as it always did. */ refresh?: boolean + /** Optional inventory filter for callers that only need known installed skills. */ + names?: string[] + sourceKinds?: SkillSourceKind[] } const ResolvedProjectRuntimeSchema = z.object({ @@ -104,7 +107,12 @@ export const SkillDiscoveryTargetSchema: z.ZodType = z.obj projectRuntime: z .discriminatedUnion('status', [ResolvedProjectRuntimeSchema, RepairProjectRuntimeSchema]) .optional(), - refresh: z.boolean().optional() + refresh: z.boolean().optional(), + names: z.array(z.string().trim().min(1).max(200)).max(100).optional(), + sourceKinds: z + .array(z.enum(['home', 'repo', 'bundled', 'plugin'])) + .max(4) + .optional() }) export type SkillFrontmatterSummary = {