fix(skills): scope completions by session flavor (#667)

This commit is contained in:
NightWatcher314
2026-05-24 10:59:32 +08:00
committed by GitHub
parent 04d3d02627
commit c417330d55
6 changed files with 252 additions and 37 deletions
+2 -2
View File
@@ -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<ListSkillsRequest, ListSkillsResponse>(RPC_METHODS.ListSkills, async () => {
rpcHandlerManager.registerHandler<ListSkillsRequest, ListSkillsResponse>(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)
+144 -5
View File
@@ -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'])
})
+98 -25
View File
@@ -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<string, InstalledPlugin[]>;
};
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<boolean> {
@@ -51,7 +88,7 @@ async function pathExists(path: string): Promise<boolean> {
}
}
async function listProjectSkillsRoots(workingDirectory?: string): Promise<string[]> {
async function listProjectSkillsRoots(workingDirectory?: string, flavor?: string): Promise<string[]> {
if (!workingDirectory) {
return [];
}
@@ -62,12 +99,12 @@ async function listProjectSkillsRoots(workingDirectory?: string): Promise<string
while (true) {
if (await pathExists(join(currentDirectory, '.git'))) {
return directories.flatMap(getProjectSkillsRoots);
return directories.flatMap((directory) => 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<SkillSummary[]>
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<SkillSummary[]> {
const projectRoots = await listProjectSkillsRoots(workingDirectory);
const userRoots = getUserSkillsRoots();
async function listPluginCacheSkillsRoots(flavor?: string): Promise<string[]> {
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<SkillSummary[]> {
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<SkillSummar
for (const skill of [
...projectSkills,
...userSkills,
...pluginSkills,
...adminSkills,
]) {
if (!dedupedSkills.has(skill.name)) {