From c417330d550dc1d35dd599d7ad8d0e98284fee4b Mon Sep 17 00:00:00 2001 From: NightWatcher314 Date: Sun, 24 May 2026 10:59:32 +0800 Subject: [PATCH] fix(skills): scope completions by session flavor (#667) --- cli/src/modules/common/handlers/skills.ts | 4 +- cli/src/modules/common/skills.test.ts | 149 +++++++++++++++++++++- cli/src/modules/common/skills.ts | 123 ++++++++++++++---- hub/src/sync/rpcGateway.ts | 4 +- hub/src/sync/syncEngine.ts | 4 +- hub/src/web/routes/sessions.ts | 5 +- 6 files changed, 252 insertions(+), 37 deletions(-) diff --git a/cli/src/modules/common/handlers/skills.ts b/cli/src/modules/common/handlers/skills.ts index 36fa96fb..4a3f7858 100644 --- a/cli/src/modules/common/handlers/skills.ts +++ b/cli/src/modules/common/handlers/skills.ts @@ -5,11 +5,11 @@ import { listSkills, type ListSkillsRequest, type ListSkillsResponse } from '../ import { getErrorMessage, rpcError } from '../rpcResponses' export function registerSkillsHandlers(rpcHandlerManager: RpcHandlerManager, workingDirectory: string): void { - rpcHandlerManager.registerHandler(RPC_METHODS.ListSkills, async () => { + rpcHandlerManager.registerHandler(RPC_METHODS.ListSkills, async (request) => { logger.debug('List skills request') try { - const skills = await listSkills(workingDirectory) + const skills = await listSkills(workingDirectory, { flavor: request.flavor }) return { success: true, skills } } catch (error) { logger.debug('Failed to list skills:', error) diff --git a/cli/src/modules/common/skills.test.ts b/cli/src/modules/common/skills.test.ts index c7ee948e..e646df6b 100644 --- a/cli/src/modules/common/skills.test.ts +++ b/cli/src/modules/common/skills.test.ts @@ -18,6 +18,8 @@ async function writeSkill(skillDir: string, name: string, description: string): describe('listSkills', () => { const originalHome = process.env.HOME + const originalClaudeConfigDir = process.env.CLAUDE_CONFIG_DIR + const originalCodexHome = process.env.CODEX_HOME let sandboxDir: string let homeDir: string @@ -25,6 +27,8 @@ describe('listSkills', () => { sandboxDir = await mkdtemp(join(tmpdir(), 'hapi-skills-')) homeDir = join(sandboxDir, 'home') process.env.HOME = homeDir + delete process.env.CLAUDE_CONFIG_DIR + delete process.env.CODEX_HOME await mkdir(homeDir, { recursive: true }) }) @@ -35,6 +39,18 @@ describe('listSkills', () => { process.env.HOME = originalHome } + if (originalClaudeConfigDir === undefined) { + delete process.env.CLAUDE_CONFIG_DIR + } else { + process.env.CLAUDE_CONFIG_DIR = originalClaudeConfigDir + } + + if (originalCodexHome === undefined) { + delete process.env.CODEX_HOME + } else { + process.env.CODEX_HOME = originalCodexHome + } + await rm(sandboxDir, { recursive: true, force: true }) }) @@ -69,16 +85,139 @@ describe('listSkills', () => { expect(skills.find((s) => s.name === 'alpha')?.description).toBe('Alpha from agents') }) - it('lists user skills from ~/.codex/skills including Codex bundled system skills', async () => { + it('lists user skills from ~/.codex/skills including Codex bundled system skills for Codex', async () => { await writeSkill(join(homeDir, '.agents', 'skills', 'amis'), 'amis', 'AMIS guide') await writeSkill(join(homeDir, '.codex', 'skills', 'hello-agents'), 'helloagents', 'Main skill') await writeSkill(join(homeDir, '.codex', 'skills', '.system', 'skill-creator'), 'skill-creator', 'Create skills') - const skills = await listSkills() + const skills = await listSkills(undefined, { flavor: 'codex' }) expect(skills.map((skill) => skill.name)).toEqual(['amis', 'helloagents', 'skill-creator']) }) + it('scopes user skills to the requested flavor', async () => { + await writeSkill(join(homeDir, '.agents', 'skills', 'shared'), 'shared', 'Shared skill') + await writeSkill(join(homeDir, '.claude', 'skills', 'claude-only'), 'claude-only', 'Claude skill') + await writeSkill(join(homeDir, '.codex', 'skills', 'codex-only'), 'codex-only', 'Codex skill') + + const claudeSkills = await listSkills(undefined, { flavor: 'claude' }) + const codexSkills = await listSkills(undefined, { flavor: 'codex' }) + + expect(claudeSkills.map((skill) => skill.name)).toEqual(['claude-only', 'shared']) + expect(codexSkills.map((skill) => skill.name)).toEqual(['codex-only', 'shared']) + }) + + it('uses configured Claude and Codex homes for agent-specific user skills', async () => { + const claudeConfigDir = join(sandboxDir, 'custom-claude') + const codexHome = join(sandboxDir, 'custom-codex') + process.env.CLAUDE_CONFIG_DIR = claudeConfigDir + process.env.CODEX_HOME = codexHome + + await writeSkill(join(homeDir, '.agents', 'skills', 'shared'), 'shared', 'Shared skill') + await writeSkill(join(homeDir, '.claude', 'skills', 'default-claude'), 'default-claude', 'Default Claude skill') + await writeSkill(join(homeDir, '.codex', 'skills', 'default-codex'), 'default-codex', 'Default Codex skill') + await writeSkill(join(claudeConfigDir, 'skills', 'custom-claude'), 'custom-claude', 'Custom Claude skill') + await writeSkill(join(codexHome, 'skills', 'custom-codex'), 'custom-codex', 'Custom Codex skill') + + const claudeSkills = await listSkills(undefined, { flavor: 'claude' }) + const codexSkills = await listSkills(undefined, { flavor: 'codex' }) + + expect(claudeSkills.map((skill) => skill.name)).toEqual(['custom-claude', 'shared']) + expect(codexSkills.map((skill) => skill.name)).toEqual(['custom-codex', 'shared']) + }) + + it('includes Codex system skills from a configured CODEX_HOME', async () => { + const codexHome = join(sandboxDir, 'custom-codex') + process.env.CODEX_HOME = codexHome + + await writeSkill(join(codexHome, 'skills', 'custom-codex'), 'custom-codex', 'Custom Codex skill') + await writeSkill(join(codexHome, 'skills', '.system', 'custom-system'), 'custom-system', 'Custom Codex system skill') + + const skills = await listSkills(undefined, { flavor: 'codex' }) + + expect(skills.map((skill) => skill.name)).toEqual(['custom-codex', 'custom-system']) + }) + + it('includes installed marketplace skills for the requested flavor', async () => { + const claudeInstallPath = join(homeDir, '.claude', 'plugins', 'cache', 'owner', 'claude-plugin', '1.0.0') + const codexInstallPath = join(homeDir, '.codex', 'plugins', 'cache', 'owner', 'codex-plugin', '1.0.0') + await writeSkill(join(claudeInstallPath, 'skills', 'claude-market'), 'claude-market', 'Claude marketplace skill') + await writeSkill(join(codexInstallPath, 'skills', 'codex-market'), 'codex-market', 'Codex marketplace skill') + await writeFile(join(homeDir, '.claude', 'plugins', 'installed_plugins.json'), JSON.stringify({ + plugins: { + 'claude-plugin@owner': [{ installPath: claudeInstallPath, lastUpdated: '2026-01-02T00:00:00.000Z' }] + } + })) + await writeFile(join(homeDir, '.codex', 'plugins', 'installed_plugins.json'), JSON.stringify({ + plugins: { + 'codex-plugin@owner': [{ installPath: codexInstallPath, lastUpdated: '2026-01-02T00:00:00.000Z' }] + } + })) + + const claudeSkills = await listSkills(undefined, { flavor: 'claude' }) + const codexSkills = await listSkills(undefined, { flavor: 'codex' }) + + expect(claudeSkills.map((skill) => skill.name)).toEqual(['claude-market']) + expect(codexSkills.map((skill) => skill.name)).toEqual(['codex-market']) + }) + + it('uses configured Claude and Codex homes for installed marketplace skills', async () => { + const claudeConfigDir = join(sandboxDir, 'custom-claude') + const codexHome = join(sandboxDir, 'custom-codex') + process.env.CLAUDE_CONFIG_DIR = claudeConfigDir + process.env.CODEX_HOME = codexHome + + const claudeInstallPath = join(claudeConfigDir, 'plugins', 'cache', 'owner', 'claude-plugin', '1.0.0') + const codexInstallPath = join(codexHome, 'plugins', 'cache', 'owner', 'codex-plugin', '1.0.0') + await writeSkill(join(claudeInstallPath, 'skills', 'claude-market'), 'claude-market', 'Claude marketplace skill') + await writeSkill(join(codexInstallPath, 'skills', 'codex-market'), 'codex-market', 'Codex marketplace skill') + await writeFile(join(claudeConfigDir, 'plugins', 'installed_plugins.json'), JSON.stringify({ + plugins: { + 'claude-plugin@owner': [{ installPath: claudeInstallPath, lastUpdated: '2026-01-02T00:00:00.000Z' }] + } + })) + await writeFile(join(codexHome, 'plugins', 'installed_plugins.json'), JSON.stringify({ + plugins: { + 'codex-plugin@owner': [{ installPath: codexInstallPath, lastUpdated: '2026-01-02T00:00:00.000Z' }] + } + })) + + const claudeSkills = await listSkills(undefined, { flavor: 'claude' }) + const codexSkills = await listSkills(undefined, { flavor: 'codex' }) + + expect(claudeSkills.map((skill) => skill.name)).toEqual(['claude-market']) + expect(codexSkills.map((skill) => skill.name)).toEqual(['codex-market']) + }) + + it('does not list cached marketplace skills that are not installed', async () => { + await writeSkill(join(homeDir, '.codex', 'plugins', 'cache', 'owner', 'stale-plugin', '1.0.0', 'skills', 'stale-market'), 'stale-market', 'Stale marketplace skill') + await mkdir(join(homeDir, '.codex', 'plugins'), { recursive: true }) + await writeFile(join(homeDir, '.codex', 'plugins', 'installed_plugins.json'), JSON.stringify({ plugins: {} })) + + const skills = await listSkills(undefined, { flavor: 'codex' }) + + expect(skills.map((skill) => skill.name)).toEqual([]) + }) + + it('uses the newest installed plugin path when multiple installations exist', async () => { + const oldInstallPath = join(homeDir, '.codex', 'plugins', 'cache', 'owner', 'codex-plugin', '1.0.0') + const newInstallPath = join(homeDir, '.codex', 'plugins', 'cache', 'owner', 'codex-plugin', '2.0.0') + await writeSkill(join(oldInstallPath, 'skills', 'plugin-skill'), 'plugin-skill', 'Old marketplace skill') + await writeSkill(join(newInstallPath, 'skills', 'plugin-skill'), 'plugin-skill', 'New marketplace skill') + await writeFile(join(homeDir, '.codex', 'plugins', 'installed_plugins.json'), JSON.stringify({ + plugins: { + 'codex-plugin@owner': [ + { installPath: oldInstallPath, lastUpdated: '2026-01-01T00:00:00.000Z' }, + { installPath: newInstallPath, lastUpdated: '2026-01-02T00:00:00.000Z' } + ] + } + })) + + const skills = await listSkills(undefined, { flavor: 'codex' }) + + expect(skills).toEqual([{ name: 'plugin-skill', description: 'New marketplace skill' }]) + }) + it('falls back to directory name when frontmatter is missing', async () => { const skillDir = join(homeDir, '.agents', 'skills', 'no-frontmatter') await mkdir(skillDir, { recursive: true }) @@ -100,7 +239,7 @@ describe('listSkills', () => { await writeSkill(join(workingDirectory, '.agents', 'skills', 'local-skill'), 'local-skill', 'Local skill') await writeSkill(join(sandboxDir, '.agents', 'skills', 'outside-skill'), 'outside-skill', 'Outside repo skill') - const skills = await listSkills(workingDirectory) + const skills = await listSkills(workingDirectory, { flavor: 'claude' }) expect(skills.map((skill) => skill.name)).toEqual(['local-skill', 'package-skill', 'root-skill']) }) @@ -113,7 +252,7 @@ describe('listSkills', () => { await writeSkill(join(repoRoot, '.claude', 'skills', 'claude-root'), 'claude-root', 'Claude root skill') await writeSkill(join(workingDirectory, '.claude', 'skills', 'claude-local'), 'claude-local', 'Claude local skill') - const skills = await listSkills(workingDirectory) + const skills = await listSkills(workingDirectory, { flavor: 'claude' }) expect(skills.map((skill) => skill.name)).toEqual(['claude-local', 'claude-root']) }) @@ -127,7 +266,7 @@ describe('listSkills', () => { await writeSkill(join(workingDirectory, '.codex', 'skills', 'codex-local'), 'codex-local', 'Codex local skill') await writeSkill(join(workingDirectory, '.codex', 'skills', '.system', 'codex-system'), 'codex-system', 'Codex system skill') - const skills = await listSkills(workingDirectory) + const skills = await listSkills(workingDirectory, { flavor: 'codex' }) expect(skills.map((skill) => skill.name)).toEqual(['codex-local', 'codex-root', 'codex-system']) }) diff --git a/cli/src/modules/common/skills.ts b/cli/src/modules/common/skills.ts index 46f44b66..47b169da 100644 --- a/cli/src/modules/common/skills.ts +++ b/cli/src/modules/common/skills.ts @@ -9,6 +9,7 @@ export interface SkillSummary { } export interface ListSkillsRequest { + flavor?: string; } export interface ListSkillsResponse { @@ -17,29 +18,65 @@ export interface ListSkillsResponse { error?: string; } +type InstalledPlugin = { + installPath?: string; + installedAt?: string; + lastUpdated?: string; +}; + +type InstalledPluginsFile = { + plugins?: Record; +}; + function getHomeDirectory(): string { return process.env.HOME ?? process.env.USERPROFILE ?? homedir(); } -function getUserSkillsRoots(): string[] { +function normalizeFlavor(flavor?: string): string { + return (flavor ?? 'claude').trim().toLowerCase(); +} + +function getAgentConfigDir(flavor?: string): string { + const normalizedFlavor = normalizeFlavor(flavor); + switch (normalizedFlavor) { + case 'claude': + return process.env.CLAUDE_CONFIG_DIR || join(getHomeDirectory(), '.claude'); + case 'codex': + return process.env.CODEX_HOME || join(getHomeDirectory(), '.codex'); + default: + return join(getHomeDirectory(), `.${normalizedFlavor}`); + } +} + +function getUserSkillsRoots(flavor?: string): string[] { const home = getHomeDirectory(); - return [ - join(home, '.agents', 'skills'), - join(home, '.claude', 'skills'), - join(home, '.codex', 'skills'), - ]; + const roots = [join(home, '.agents', 'skills')]; + switch (normalizeFlavor(flavor)) { + case 'claude': + roots.push(join(getAgentConfigDir(flavor), 'skills')); + break; + case 'codex': + roots.push(join(getAgentConfigDir(flavor), 'skills')); + break; + } + return roots; } function getAdminSkillsRoot(): string { return join('/etc', 'codex', 'skills'); } -function getProjectSkillsRoots(directory: string): string[] { - return [ - join(directory, '.agents', 'skills'), - join(directory, '.claude', 'skills'), - join(directory, '.codex', 'skills'), - ]; +function getProjectSkillsRoots(directory: string, flavor?: string): string[] { + const roots = [join(directory, '.agents', 'skills')]; + switch (normalizeFlavor(flavor)) { + case 'claude': + roots.push(join(directory, '.claude', 'skills')); + break; + case 'codex': + roots.push(join(directory, '.codex', 'skills')); + break; + } + return roots; } async function pathExists(path: string): Promise { @@ -51,7 +88,7 @@ async function pathExists(path: string): Promise { } } -async function listProjectSkillsRoots(workingDirectory?: string): Promise { +async function listProjectSkillsRoots(workingDirectory?: string, flavor?: string): Promise { if (!workingDirectory) { return []; } @@ -62,12 +99,12 @@ async function listProjectSkillsRoots(workingDirectory?: string): Promise getProjectSkillsRoots(directory, flavor)); } const parentDirectory = dirname(currentDirectory); if (parentDirectory === currentDirectory) { - return getProjectSkillsRoots(resolvedWorkingDirectory); + return getProjectSkillsRoots(resolvedWorkingDirectory, flavor); } currentDirectory = parentDirectory; @@ -151,23 +188,58 @@ async function readSkillsFromDirs(skillDirs: string[]): Promise return skills.filter((skill): skill is SkillSummary => skill !== null); } -function isCodexSkillsRoot(root: string): boolean { - return root.endsWith(join('.codex', 'skills')); +function shouldIncludeCodexSystem(root: string, flavor: string): boolean { + if (flavor !== 'codex') { + return false; + } + + return root.endsWith(join('.codex', 'skills')) + || root === join(getAgentConfigDir('codex'), 'skills'); } -export async function listSkills(workingDirectory?: string): Promise { - const projectRoots = await listProjectSkillsRoots(workingDirectory); - const userRoots = getUserSkillsRoots(); +async function listPluginCacheSkillsRoots(flavor?: string): Promise { + const installedPath = join(getAgentConfigDir(flavor), 'plugins', 'installed_plugins.json'); + let installed: InstalledPluginsFile; + + try { + installed = JSON.parse(await readFile(installedPath, 'utf-8')) as InstalledPluginsFile; + } catch { + return []; + } + + const getInstallTime = (installation: InstalledPlugin): number => { + const lastUpdated = Date.parse(installation.lastUpdated ?? ''); + if (Number.isFinite(lastUpdated)) return lastUpdated; + const installedAt = Date.parse(installation.installedAt ?? ''); + return Number.isFinite(installedAt) ? installedAt : 0; + }; + + return Object.values(installed.plugins ?? {}) + .filter((installations): installations is InstalledPlugin[] => Array.isArray(installations)) + .map((installations) => [...installations] + .sort((a, b) => getInstallTime(b) - getInstallTime(a))[0]?.installPath) + .filter((installPath): installPath is string => typeof installPath === 'string' && installPath.length > 0) + .map((installPath) => join(installPath, 'skills')); +} + +export async function listSkills(workingDirectory?: string, options: { flavor?: string } = {}): Promise { + const flavor = normalizeFlavor(options.flavor); + const projectRoots = await listProjectSkillsRoots(workingDirectory, flavor); + const userRoots = getUserSkillsRoots(flavor); + const pluginRoots = await listPluginCacheSkillsRoots(flavor); const adminRoot = getAdminSkillsRoot(); - const [projectSkillDirs, userSkillDirs, adminSkillDirs] = await Promise.all([ - Promise.all(projectRoots.map(async (root) => await listTopLevelSkillDirs(root, { includeCodexSystem: isCodexSkillsRoot(root) }))).then((dirs) => dirs.flat()), - Promise.all(userRoots.map(async (root) => await listTopLevelSkillDirs(root, { includeCodexSystem: isCodexSkillsRoot(root) }))).then((dirs) => dirs.flat()), - listTopLevelSkillDirs(adminRoot, { includeCodexSystem: true }), + const includeAdminRoots = flavor === 'codex'; + const [projectSkillDirs, userSkillDirs, pluginSkillDirs, adminSkillDirs] = await Promise.all([ + Promise.all(projectRoots.map(async (root) => await listTopLevelSkillDirs(root, { includeCodexSystem: shouldIncludeCodexSystem(root, flavor) }))).then((dirs) => dirs.flat()), + Promise.all(userRoots.map(async (root) => await listTopLevelSkillDirs(root, { includeCodexSystem: shouldIncludeCodexSystem(root, flavor) }))).then((dirs) => dirs.flat()), + Promise.all(pluginRoots.map(async (root) => await listTopLevelSkillDirs(root, { includeCodexSystem: false }))).then((dirs) => dirs.flat()), + includeAdminRoots ? listTopLevelSkillDirs(adminRoot, { includeCodexSystem: true }) : [], ]); - const [projectSkills, userSkills, adminSkills] = await Promise.all([ + const [projectSkills, userSkills, pluginSkills, adminSkills] = await Promise.all([ readSkillsFromDirs(projectSkillDirs), readSkillsFromDirs(userSkillDirs), + readSkillsFromDirs(pluginSkillDirs), readSkillsFromDirs(adminSkillDirs), ]); @@ -175,6 +247,7 @@ export async function listSkills(workingDirectory?: string): Promise error?: string }> { - return await this.sessionRpc(sessionId, RPC_METHODS.ListSkills, {}) as { + return await this.sessionRpc(sessionId, RPC_METHODS.ListSkills, { flavor }) as { success: boolean skills?: Array<{ name: string; description?: string }> error?: string diff --git a/hub/src/sync/syncEngine.ts b/hub/src/sync/syncEngine.ts index d55563ae..cc824bd6 100644 --- a/hub/src/sync/syncEngine.ts +++ b/hub/src/sync/syncEngine.ts @@ -889,12 +889,12 @@ export class SyncEngine { return await this.rpcGateway.listSlashCommands(sessionId, agent) } - async listSkills(sessionId: string): Promise<{ + async listSkills(sessionId: string, flavor?: string): Promise<{ success: boolean skills?: Array<{ name: string; description?: string }> error?: string }> { - return await this.rpcGateway.listSkills(sessionId) + return await this.rpcGateway.listSkills(sessionId, flavor) } async listCodexModelsForSession(sessionId: string): Promise { diff --git a/hub/src/web/routes/sessions.ts b/hub/src/web/routes/sessions.ts index dcfc66b6..ef672ec4 100644 --- a/hub/src/web/routes/sessions.ts +++ b/hub/src/web/routes/sessions.ts @@ -540,7 +540,10 @@ export function createSessionsRoutes(getSyncEngine: () => SyncEngine | null): Ho } try { - const result = await engine.listSkills(sessionResult.sessionId) + const result = await engine.listSkills( + sessionResult.sessionId, + sessionResult.session.metadata?.flavor ?? 'claude' + ) return c.json(result) } catch (error) { return c.json({