From 0c7efcb3ff50af5fd12a91c13995fa0c25108b0b Mon Sep 17 00:00:00 2001 From: Jinwoo Hong <73622457+Jinwoo-H@users.noreply.github.com> Date: Thu, 15 Oct 2026 14:04:36 -0400 Subject: [PATCH] Derive skill discovery folders and home env overrides from the agent provider table (#29022) * Derive per-agent skill discovery folders and home env overrides from the provider table Skill discovery and the env-var home overrides now read the skill install provider table instead of repeating each agent's folders by hand. Output is unchanged: same sources, labels, ids, paths and scan order. A drift test fails when a table entry lacks discovery coverage or is not an Orca agent id. * Simplify provider-table discovery: table order, no pinned slots, Muse hand-written Provider home folders now follow the table order right after the shared .agents root, with the one-off folders after them. Drops the pinned scan slots and the discovery-only field; the provider id list is checked against Orca agent ids at compile time. * Derive provider ids from the table; scan the Codex plugin cache before agent homes The provider id union now comes from the table itself, checked against Orca agent ids, so adding an agent is one edit. The plugin cache is scanned before agent homes so a skill reachable from both keeps its plugin source kind. * Type the provider table by inference and drop the id cast The provider definition and id types are read straight off the frozen table, and the id guard compares ids without a type assertion. * Label skill sources with the agent display name Claude and Gemini skill sources now read "Claude Code home" and "Gemini CLI home", matching the install picker; the separate short-name field is gone. --- src/main/skills/skill-discovery-sources.ts | 183 ++++++------------ .../skill-provider-discovery-coverage.test.ts | 67 +++++++ .../skills/skill-provider-runtime-roots.ts | 13 +- .../settings/OrchestrationPane.test.tsx | 4 +- .../OrchestrationSkillAgentCoverage.test.tsx | 4 +- .../skills/skill-delete-copy.test.ts | 4 +- src/shared/skill-install-providers.ts | 40 ++-- 7 files changed, 156 insertions(+), 159 deletions(-) create mode 100644 src/main/skills/skill-provider-discovery-coverage.test.ts diff --git a/src/main/skills/skill-discovery-sources.ts b/src/main/skills/skill-discovery-sources.ts index f013246ac65..1e490413a20 100644 --- a/src/main/skills/skill-discovery-sources.ts +++ b/src/main/skills/skill-discovery-sources.ts @@ -4,6 +4,10 @@ import type { SkillDiscoverySource, SkillProvider, SkillSourceKind } from '../.. import type { AgentType } from '../../shared/agent-status-types' import type { Repo } from '../../shared/repo-types' import { getRepoExecutionHostId, LOCAL_EXECUTION_HOST_ID } from '../../shared/execution-host' +import { + SKILL_INSTALL_PROVIDERS, + type SkillInstallProviderId +} from '../../shared/skill-install-providers' import type { SkillProviderRootOverrides } from './skill-provider-destinations' import { stablePathId } from './skill-discovery-classification' import { @@ -35,6 +39,56 @@ function source( return { id, label, path, sourceKind, providers, owner } } +// Why: Claude has its own SkillProvider tag; every other agent shares agent-skills. +function skillProvidersFor(id: SkillInstallProviderId): SkillProvider[] { + return id === 'claude' ? ['claude'] : ['agent-skills'] +} + +function providerHomeSources( + home: string, + pathApi: SkillDiscoveryPathApi, + overrides: SkillProviderRootOverrides +): SkillScanRoot[] { + return SKILL_INSTALL_PROVIDERS.flatMap((provider) => { + if (!provider.globalSegments) { + return [] + } + return [ + source( + `home-${provider.id}`, + `${provider.displayName} home`, + overrides[provider.id] ?? pathApi.join(home, ...provider.globalSegments), + 'home', + skillProvidersFor(provider.id), + provider.id + ) + ] + }) +} + +function providerRepoSources( + repoPath: string, + repoLabel: string, + pathApi: SkillDiscoveryPathApi +): SkillScanRoot[] { + return SKILL_INSTALL_PROVIDERS.flatMap((provider) => { + const segments = provider.workspaceSegments + if (!segments) { + return [] + } + return [ + source( + `repo-${provider.id}-${stablePathId(repoPath)}`, + `${repoLabel} ${segments[0]}`, + pathApi.join(repoPath, ...segments), + 'repo', + skillProvidersFor(provider.id), + provider.id + ) + ] + }) +} + export function buildSkillDiscoverySources( args: { homeDir?: string @@ -58,6 +112,7 @@ export function buildSkillDiscoverySources( ? pathApi.join(home, '.hermes', 'skills') : (resolveEnvironmentHermesSkillsRoot() ?? resolveDefaultHermesSkillsRoot({ homeDir: home })) const roots: SkillScanRoot[] = [ + // Why: Codex reads `.agents/skills` today; this legacy home still holds older installs. source( 'home-codex', 'Codex home', @@ -74,14 +129,6 @@ export function buildSkillDiscoverySources( ['agent-skills'], null ), - source( - 'home-claude', - 'Claude home', - providerRootOverrides.claude ?? pathApi.join(home, '.claude', 'skills'), - 'home', - ['claude'], - 'claude' - ), source( 'codex-plugin-cache', 'Codex plugin cache', @@ -92,14 +139,7 @@ export function buildSkillDiscoverySources( ), // Why: `npx skills add --global` writes into each agent's own home skills // directory, so coverage misses them unless we scan every provider root. - source( - 'home-grok', - 'Grok home', - providerRootOverrides.grok ?? pathApi.join(home, '.grok', 'skills'), - 'home', - ['agent-skills'], - 'grok' - ), + ...providerHomeSources(home, pathApi, providerRootOverrides), source( 'home-opencode', 'OpenCode home', @@ -133,14 +173,6 @@ export function buildSkillDiscoverySources( ['agent-skills'], 'prime-agent' ), - source( - 'home-gemini', - 'Gemini home', - pathApi.join(home, '.gemini', 'skills'), - 'home', - ['agent-skills'], - 'gemini' - ), source( 'home-antigravity', 'Antigravity home', @@ -149,49 +181,7 @@ export function buildSkillDiscoverySources( ['agent-skills'], 'antigravity' ), - source( - 'home-cursor', - 'Cursor home', - pathApi.join(home, '.cursor', 'skills'), - 'home', - ['agent-skills'], - 'cursor' - ), - source( - 'home-droid', - 'Droid home', - pathApi.join(home, '.factory', 'skills'), - 'home', - ['agent-skills'], - 'droid' - ), - source( - 'home-continue', - 'Continue home', - pathApi.join(home, '.continue', 'skills'), - 'home', - ['agent-skills'], - 'continue' - ), - source( - 'home-trae', - 'Trae home', - pathApi.join(home, '.trae-cn', 'skills'), - 'home', - ['agent-skills'], - 'trae' - ), - source( - 'home-aug', - 'Augment home', - pathApi.join(home, '.augment', 'skills'), - 'home', - ['agent-skills'], - 'aug' - ), - // Why: user skills live under XDG config home (`~/.config/muse/skills` by - // default); project skills are the canonical `.agents/skills` root already - // covered by home-agents/repo-agents, so no agent-specific repo source. + // Why: Muse installs to `.agents/skills` but also reads XDG config home. source( 'home-muse', 'Muse home', @@ -199,16 +189,6 @@ export function buildSkillDiscoverySources( 'home', ['agent-skills'], 'muse' - ), - // Why: ZCode loads user skills from `~/.zcode/skills`; project skills are the canonical - // `.agents/skills` root already covered by home-agents/repo-agents. - source( - 'home-zcode', - 'ZCode home', - pathApi.join(home, '.zcode', 'skills'), - 'home', - ['agent-skills'], - 'zcode' ) ] @@ -236,54 +216,7 @@ export function buildSkillDiscoverySources( ['agent-skills'], null ), - source( - `repo-claude-${stablePathId(repoPath)}`, - `${label} .claude`, - pathApi.join(repoPath, '.claude', 'skills'), - 'repo', - ['claude'], - 'claude' - ), - source( - `repo-droid-${stablePathId(repoPath)}`, - `${label} .factory`, - pathApi.join(repoPath, '.factory', 'skills'), - 'repo', - ['agent-skills'], - 'droid' - ), - source( - `repo-continue-${stablePathId(repoPath)}`, - `${label} .continue`, - pathApi.join(repoPath, '.continue', 'skills'), - 'repo', - ['agent-skills'], - 'continue' - ), - source( - `repo-trae-${stablePathId(repoPath)}`, - `${label} .trae`, - pathApi.join(repoPath, '.trae', 'skills'), - 'repo', - ['agent-skills'], - 'trae' - ), - source( - `repo-grok-${stablePathId(repoPath)}`, - `${label} .grok`, - pathApi.join(repoPath, '.grok', 'skills'), - 'repo', - ['agent-skills'], - 'grok' - ), - source( - `repo-aug-${stablePathId(repoPath)}`, - `${label} .augment`, - pathApi.join(repoPath, '.augment', 'skills'), - 'repo', - ['agent-skills'], - 'aug' - ) + ...providerRepoSources(repoPath, label, pathApi) ) } diff --git a/src/main/skills/skill-provider-discovery-coverage.test.ts b/src/main/skills/skill-provider-discovery-coverage.test.ts new file mode 100644 index 00000000000..ab5dce46e40 --- /dev/null +++ b/src/main/skills/skill-provider-discovery-coverage.test.ts @@ -0,0 +1,67 @@ +import { join, posix, resolve } from 'node:path' +import { describe, expect, it } from 'vitest' +import { SKILL_INSTALL_PROVIDERS } from '../../shared/skill-install-providers' +import { buildSkillDiscoverySources } from './skill-discovery-sources' +import { resolveEnvironmentSkillProviderRoots } from './skill-provider-runtime-roots' + +const HOME = '/home/test' +const WORKSPACE = '/workspace/current' + +function discover(providerRootOverrides = {}) { + return buildSkillDiscoverySources({ + homeDir: HOME, + cwd: WORKSPACE, + pathApi: posix, + providerRootOverrides + }) +} + +describe('skill install provider table drives discovery', () => { + it('lists every provider install folder at each scope it has one', () => { + const sources = discover() + for (const provider of SKILL_INSTALL_PROVIDERS) { + if (provider.globalSegments) { + expect(sources, provider.id).toContainEqual( + expect.objectContaining({ + path: posix.join(HOME, ...provider.globalSegments), + label: `${provider.displayName} home`, + sourceKind: 'home', + owner: provider.id + }) + ) + } + if (provider.workspaceSegments) { + expect(sources, provider.id).toContainEqual( + expect.objectContaining({ + path: posix.join(WORKSPACE, ...provider.workspaceSegments), + sourceKind: 'repo', + owner: provider.id + }) + ) + } + } + // Agents with a null scope read the canonical root, which is always listed. + expect(sources.map((source) => source.path)).toEqual( + expect.arrayContaining([ + posix.join(HOME, '.agents', 'skills'), + posix.join(WORKSPACE, '.agents', 'skills') + ]) + ) + }) + + it('relocates a provider home named by its env var for install and discovery alike', () => { + for (const provider of SKILL_INSTALL_PROVIDERS) { + if (!provider.homeEnv) { + continue + } + const relocated = resolve('/srv', provider.id) + const overrides = resolveEnvironmentSkillProviderRoots({ [provider.homeEnv]: relocated }) + expect(overrides).toEqual({ [provider.id]: join(relocated, 'skills') }) + expect( + discover(overrides).find( + (source) => source.sourceKind === 'home' && source.owner === provider.id + )?.path + ).toBe(overrides[provider.id]) + } + }) +}) diff --git a/src/main/skills/skill-provider-runtime-roots.ts b/src/main/skills/skill-provider-runtime-roots.ts index d4dec7bfba7..7d93b404cb9 100644 --- a/src/main/skills/skill-provider-runtime-roots.ts +++ b/src/main/skills/skill-provider-runtime-roots.ts @@ -1,5 +1,6 @@ import { statSync } from 'node:fs' import { isAbsolute, join, resolve } from 'node:path' +import { SKILL_INSTALL_PROVIDERS } from '../../shared/skill-install-providers' import { toWindowsWslPath } from '../../shared/wsl-paths' import { runWslProcess } from '../wsl/wsl-runner' import type { SkillProviderRootOverrides } from './skill-provider-destinations' @@ -30,12 +31,14 @@ function normalizedRoot(value: string | undefined): string | null { export function resolveEnvironmentSkillProviderRoots( env: NodeJS.ProcessEnv = process.env ): SkillProviderRootOverrides { - const claudeConfig = normalizedRoot(env.CLAUDE_CONFIG_DIR) - const grokHome = normalizedRoot(env.GROK_HOME) - return { - ...(claudeConfig ? { claude: join(claudeConfig, 'skills') } : {}), - ...(grokHome ? { grok: join(grokHome, 'skills') } : {}) + const roots: SkillProviderRootOverrides = {} + for (const provider of SKILL_INSTALL_PROVIDERS) { + const home = provider.homeEnv ? normalizedRoot(env[provider.homeEnv]) : null + if (home) { + roots[provider.id] = join(home, 'skills') + } } + return roots } // Why: `HERMES_HOME` relocates a whole Hermes profile tree (`hermes -p coder`), diff --git a/src/renderer/src/components/settings/OrchestrationPane.test.tsx b/src/renderer/src/components/settings/OrchestrationPane.test.tsx index 0ef947e2aeb..2ab024fa85b 100644 --- a/src/renderer/src/components/settings/OrchestrationPane.test.tsx +++ b/src/renderer/src/components/settings/OrchestrationPane.test.tsx @@ -67,7 +67,7 @@ vi.mock('@/hooks/useInstalledAgentSkills', () => ({ description: null, providers: ['claude'], sourceKind: 'home', - sourceLabel: 'Claude home', + sourceLabel: 'Claude Code home', rootPath: '/Users/test/.claude/skills', directoryPath: '/Users/test/.claude/skills/orchestration', skillFilePath: '/Users/test/.claude/skills/orchestration/SKILL.md', @@ -78,7 +78,7 @@ vi.mock('@/hooks/useInstalledAgentSkills', () => ({ sources: [ { id: 'home-claude', - label: 'Claude home', + label: 'Claude Code home', path: '/Users/test/.claude/skills', sourceKind: 'home', providers: ['claude'], diff --git a/src/renderer/src/components/settings/OrchestrationSkillAgentCoverage.test.tsx b/src/renderer/src/components/settings/OrchestrationSkillAgentCoverage.test.tsx index 294dc5fa783..0cce4cd9fa0 100644 --- a/src/renderer/src/components/settings/OrchestrationSkillAgentCoverage.test.tsx +++ b/src/renderer/src/components/settings/OrchestrationSkillAgentCoverage.test.tsx @@ -21,7 +21,7 @@ describe('OrchestrationSkillAgentCoverage', () => { sources={[ { id: 'claude-home', - label: 'Claude home', + label: 'Claude Code home', path: '/Users/test/.claude/skills', sourceKind: 'home', providers: ['claude'], @@ -36,7 +36,7 @@ describe('OrchestrationSkillAgentCoverage', () => { description: null, providers: ['claude'], sourceKind: 'home', - sourceLabel: 'Claude home', + sourceLabel: 'Claude Code home', rootPath: '/Users/test/.claude/skills', directoryPath: '/Users/test/.claude/skills/orchestration', skillFilePath: '/Users/test/.claude/skills/orchestration/SKILL.md', diff --git a/src/renderer/src/components/skills/skill-delete-copy.test.ts b/src/renderer/src/components/skills/skill-delete-copy.test.ts index 172e19e4823..431b8b2fc40 100644 --- a/src/renderer/src/components/skills/skill-delete-copy.test.ts +++ b/src/renderer/src/components/skills/skill-delete-copy.test.ts @@ -25,14 +25,14 @@ describe('skillDeletePlacementSummary', () => { kind: 'canonical', rootLabel: 'Agent skills home' }, - { path: '/home/.claude/skills/demo', kind: 'alias-dir', rootLabel: 'Claude home' } + { path: '/home/.claude/skills/demo', kind: 'alias-dir', rootLabel: 'Claude Code home' } ] } ]) ) expect(summary).toContain('1') expect(summary).toContain('Agent skills home') - expect(summary).toContain('Claude home') + expect(summary).toContain('Claude Code home') }) it('drops the zero half so a link-only delete never reads "0 folders"', () => { diff --git a/src/shared/skill-install-providers.ts b/src/shared/skill-install-providers.ts index ebd7112d182..9d130a43433 100644 --- a/src/shared/skill-install-providers.ts +++ b/src/shared/skill-install-providers.ts @@ -1,31 +1,22 @@ +import type { TuiAgent } from './tui-agent' + /** * Agents Orca can place an installed skill for. * * `null` segments mean the agent reads the canonical `.agents/skills` root at - * that scope, so it needs no placement of its own. Ids match the detection ids - * in `tui-agent-config.ts` so a detected agent maps straight to a destination. + * that scope, so it needs no placement of its own. Ids are Orca agent ids so a + * detected agent maps straight to a destination. */ -export type SkillInstallProviderId = - | 'codex' - | 'claude' - | 'cursor' - | 'gemini' - | 'droid' - | 'continue' - | 'trae' - | 'grok' - | 'aug' - | 'muse' - | 'zcode' - -export type SkillInstallProviderDefinition = { - id: SkillInstallProviderId +type SkillInstallProviderEntry = { + id: TuiAgent displayName: string globalSegments: readonly string[] | null workspaceSegments: readonly string[] | null + /** Env var relocating the agent's home (skills in `/skills`); the WSL probe is still Grok-only. */ + homeEnv?: string } -export const SKILL_INSTALL_PROVIDERS: readonly SkillInstallProviderDefinition[] = [ +export const SKILL_INSTALL_PROVIDERS = Object.freeze([ // Why: Codex reads the canonical .agents/skills root at both scopes. { id: 'codex', @@ -37,7 +28,8 @@ export const SKILL_INSTALL_PROVIDERS: readonly SkillInstallProviderDefinition[] id: 'claude', displayName: 'Claude Code', globalSegments: ['.claude', 'skills'], - workspaceSegments: ['.claude', 'skills'] + workspaceSegments: ['.claude', 'skills'], + homeEnv: 'CLAUDE_CONFIG_DIR' }, // Why: Cursor and Gemini read the canonical root inside a project but keep // their own home directory, so they only need a placement at global scope. @@ -75,7 +67,8 @@ export const SKILL_INSTALL_PROVIDERS: readonly SkillInstallProviderDefinition[] id: 'grok', displayName: 'Grok', globalSegments: ['.grok', 'skills'], - workspaceSegments: ['.grok', 'skills'] + workspaceSegments: ['.grok', 'skills'], + homeEnv: 'GROK_HOME' }, { id: 'aug', @@ -98,12 +91,13 @@ export const SKILL_INSTALL_PROVIDERS: readonly SkillInstallProviderDefinition[] globalSegments: ['.zcode', 'skills'], workspaceSegments: null } -] +] satisfies SkillInstallProviderEntry[]) -const PROVIDERS_BY_ID = new Map(SKILL_INSTALL_PROVIDERS.map((provider) => [provider.id, provider])) +export type SkillInstallProviderDefinition = (typeof SKILL_INSTALL_PROVIDERS)[number] +export type SkillInstallProviderId = SkillInstallProviderDefinition['id'] export function isSkillInstallProviderId(value: string): value is SkillInstallProviderId { - return PROVIDERS_BY_ID.has(value as SkillInstallProviderId) + return SKILL_INSTALL_PROVIDERS.some((provider) => provider.id === value) } /**