Merge pull request #1639 from QwenLM/fix/claude-skills-config

Fix Claude plugin resource collection to respect marketplace config
This commit is contained in:
tanzhenxin 2026-01-29 11:30:49 +08:00 committed by GitHub
commit 43ea425278
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 263 additions and 21 deletions

View file

@ -4,13 +4,18 @@
* SPDX-License-Identifier: Apache-2.0
*/
import { describe, it, expect } from 'vitest';
import { describe, it, expect, beforeEach, afterEach } from 'vitest';
import * as fs from 'node:fs';
import * as path from 'node:path';
import * as os from 'node:os';
import {
convertClaudeToQwenConfig,
mergeClaudeConfigs,
isClaudePluginConfig,
convertClaudePluginPackage,
type ClaudePluginConfig,
type ClaudeMarketplacePluginConfig,
type ClaudeMarketplaceConfig,
} from './claude-converter.js';
describe('convertClaudeToQwenConfig', () => {
@ -119,3 +124,228 @@ describe('isClaudePluginConfig', () => {
);
});
});
describe('convertClaudePluginPackage', () => {
let testDir: string;
beforeEach(() => {
// Create a temporary directory for test files
testDir = fs.mkdtempSync(path.join(os.tmpdir(), 'claude-test-'));
});
afterEach(() => {
// Clean up test directory
if (fs.existsSync(testDir)) {
fs.rmSync(testDir, { recursive: true, force: true });
}
});
it('should only collect specified skills when config provides explicit list', async () => {
// Setup: Create a plugin source with multiple skills
const pluginSourceDir = path.join(testDir, 'plugin-source');
fs.mkdirSync(pluginSourceDir, { recursive: true });
// Create skills directory with 6 skills
const skillsDir = path.join(pluginSourceDir, 'skills');
fs.mkdirSync(skillsDir, { recursive: true });
const allSkills = ['xlsx', 'docx', 'pptx', 'pdf', 'csv', 'txt'];
for (const skill of allSkills) {
const skillDir = path.join(skillsDir, skill);
fs.mkdirSync(skillDir, { recursive: true });
fs.writeFileSync(
path.join(skillDir, 'SKILL.md'),
`# ${skill} skill`,
'utf-8',
);
fs.writeFileSync(
path.join(skillDir, 'index.js'),
`module.exports = {};`,
'utf-8',
);
}
// Create marketplace.json that only specifies 4 skills
const marketplaceDir = path.join(pluginSourceDir, '.claude-plugin');
fs.mkdirSync(marketplaceDir, { recursive: true });
const marketplaceConfig: ClaudeMarketplaceConfig = {
name: 'test-marketplace',
owner: { name: 'Test Owner', email: 'test@example.com' },
plugins: [
{
name: 'document-skills',
version: '1.0.0',
description: 'Test document skills',
source: './',
strict: false,
skills: [
'./skills/xlsx',
'./skills/docx',
'./skills/pptx',
'./skills/pdf',
],
},
],
};
fs.writeFileSync(
path.join(marketplaceDir, 'marketplace.json'),
JSON.stringify(marketplaceConfig, null, 2),
'utf-8',
);
// Execute: Convert the plugin
const result = await convertClaudePluginPackage(
pluginSourceDir,
'document-skills',
);
// Verify: Only specified skills should be present
const convertedSkillsDir = path.join(result.convertedDir, 'skills');
expect(fs.existsSync(convertedSkillsDir)).toBe(true);
const installedSkills = fs.readdirSync(convertedSkillsDir);
expect(installedSkills.sort()).toEqual(['docx', 'pdf', 'pptx', 'xlsx']);
// Verify each skill has its own directory with proper structure
for (const skill of ['xlsx', 'docx', 'pptx', 'pdf']) {
const skillDir = path.join(convertedSkillsDir, skill);
expect(fs.existsSync(skillDir)).toBe(true);
expect(fs.existsSync(path.join(skillDir, 'SKILL.md'))).toBe(true);
expect(fs.existsSync(path.join(skillDir, 'index.js'))).toBe(true);
}
// Verify csv and txt skills are NOT installed
expect(fs.existsSync(path.join(convertedSkillsDir, 'csv'))).toBe(false);
expect(fs.existsSync(path.join(convertedSkillsDir, 'txt'))).toBe(false);
// Clean up converted directory
fs.rmSync(result.convertedDir, { recursive: true, force: true });
});
it('should use all skills from folder when config does not specify skills', async () => {
// Setup: Create a plugin source with skills but no skills config
const pluginSourceDir = path.join(testDir, 'plugin-source-default');
fs.mkdirSync(pluginSourceDir, { recursive: true });
// Create skills directory with 3 skills
const skillsDir = path.join(pluginSourceDir, 'skills');
fs.mkdirSync(skillsDir, { recursive: true });
const allSkills = ['skill-a', 'skill-b', 'skill-c'];
for (const skill of allSkills) {
const skillDir = path.join(skillsDir, skill);
fs.mkdirSync(skillDir, { recursive: true });
fs.writeFileSync(path.join(skillDir, 'SKILL.md'), `# ${skill}`, 'utf-8');
}
// Create marketplace.json WITHOUT skills field
const marketplaceDir = path.join(pluginSourceDir, '.claude-plugin');
fs.mkdirSync(marketplaceDir, { recursive: true });
const marketplaceConfig: ClaudeMarketplaceConfig = {
name: 'test-marketplace',
owner: { name: 'Test Owner', email: 'test@example.com' },
plugins: [
{
name: 'default-skills',
version: '1.0.0',
description: 'Test default skills behavior',
source: './',
strict: false,
// No skills field - should use all skills from folder
},
],
};
fs.writeFileSync(
path.join(marketplaceDir, 'marketplace.json'),
JSON.stringify(marketplaceConfig, null, 2),
'utf-8',
);
// Execute: Convert the plugin
const result = await convertClaudePluginPackage(
pluginSourceDir,
'default-skills',
);
// Verify: All skills should be present
const convertedSkillsDir = path.join(result.convertedDir, 'skills');
expect(fs.existsSync(convertedSkillsDir)).toBe(true);
const installedSkills = fs.readdirSync(convertedSkillsDir);
expect(installedSkills.sort()).toEqual(['skill-a', 'skill-b', 'skill-c']);
// Clean up converted directory
fs.rmSync(result.convertedDir, { recursive: true, force: true });
});
it('should preserve directory structure when collecting skills', async () => {
// Setup: Create a plugin with nested skill structure
const pluginSourceDir = path.join(testDir, 'plugin-nested');
fs.mkdirSync(pluginSourceDir, { recursive: true });
// Create nested skill directory
const skillsDir = path.join(pluginSourceDir, 'skills');
const nestedSkillDir = path.join(skillsDir, 'nested-skill', 'subdir');
fs.mkdirSync(nestedSkillDir, { recursive: true });
fs.writeFileSync(
path.join(skillsDir, 'nested-skill', 'SKILL.md'),
'# Nested Skill',
'utf-8',
);
fs.writeFileSync(
path.join(nestedSkillDir, 'helper.js'),
'module.exports = {};',
'utf-8',
);
// Create marketplace.json
const marketplaceDir = path.join(pluginSourceDir, '.claude-plugin');
fs.mkdirSync(marketplaceDir, { recursive: true });
const marketplaceConfig: ClaudeMarketplaceConfig = {
name: 'test-marketplace',
owner: { name: 'Test Owner', email: 'test@example.com' },
plugins: [
{
name: 'nested-plugin',
version: '1.0.0',
description: 'Test nested structure',
source: './',
strict: false,
skills: ['./skills/nested-skill'],
},
],
};
fs.writeFileSync(
path.join(marketplaceDir, 'marketplace.json'),
JSON.stringify(marketplaceConfig, null, 2),
'utf-8',
);
// Execute: Convert the plugin
const result = await convertClaudePluginPackage(
pluginSourceDir,
'nested-plugin',
);
// Verify: Nested structure should be preserved
const convertedSkillsDir = path.join(result.convertedDir, 'skills');
expect(fs.existsSync(convertedSkillsDir)).toBe(true);
const nestedSkillPath = path.join(convertedSkillsDir, 'nested-skill');
expect(fs.existsSync(nestedSkillPath)).toBe(true);
expect(fs.existsSync(path.join(nestedSkillPath, 'SKILL.md'))).toBe(true);
expect(
fs.existsSync(path.join(nestedSkillPath, 'subdir', 'helper.js')),
).toBe(true);
// Clean up converted directory
fs.rmSync(result.convertedDir, { recursive: true, force: true });
});
});

View file

@ -433,28 +433,36 @@ export async function convertClaudePluginPackage(
// Step 6: Copy plugin files to temporary directory
await copyDirectory(pluginSource, tmpDir);
// Step 7: Collect commands to commands folder
if (mergedConfig.commands) {
const commandsDestDir = path.join(tmpDir, 'commands');
await collectResources(
mergedConfig.commands,
pluginSource,
commandsDestDir,
);
// Step 6.1: Handle commands/skills/agents folders based on configuration
// If configuration specifies resources, only collect those
// If configuration doesn't specify, keep the existing folder (if exists)
const resourceConfigs = [
{ name: 'commands', config: mergedConfig.commands },
{ name: 'skills', config: mergedConfig.skills },
{ name: 'agents', config: mergedConfig.agents },
];
for (const { name, config } of resourceConfigs) {
const folderPath = path.join(tmpDir, name);
const sourceFolderPath = path.join(pluginSource, name);
// If config explicitly specifies resources, remove existing folder and collect only specified ones
if (config) {
if (fs.existsSync(folderPath)) {
fs.rmSync(folderPath, { recursive: true, force: true });
}
await collectResources(config, pluginSource, folderPath);
}
// If config doesn't specify and source folder doesn't exist in pluginSource,
// remove it from tmpDir (it was copied but not needed)
else if (!fs.existsSync(sourceFolderPath) && fs.existsSync(folderPath)) {
fs.rmSync(folderPath, { recursive: true, force: true });
}
// Otherwise, keep the existing folder from pluginSource (default behavior)
}
// Step 8: Collect skills to skills folder
if (mergedConfig.skills) {
const skillsDestDir = path.join(tmpDir, 'skills');
await collectResources(mergedConfig.skills, pluginSource, skillsDestDir);
}
// Step 9: Collect agents to agents folder
const agentsDestDir = path.join(tmpDir, 'agents');
if (mergedConfig.agents) {
await collectResources(mergedConfig.agents, pluginSource, agentsDestDir);
}
// Step 9.1: Convert collected agent files from Claude format to Qwen format
const agentsDestDir = path.join(tmpDir, 'agents');
await convertAgentFiles(agentsDestDir);
// Step 10: Convert to Qwen format config
@ -531,6 +539,10 @@ async function collectResources(
continue;
}
// Determine destination: preserve the directory name
// e.g., ./skills/xlsx -> tmpDir/skills/xlsx/
const finalDestDir = path.join(destDir, dirName);
// Copy all files from the directory
const files = await glob('**/*', {
cwd: resolvedPath,
@ -540,7 +552,7 @@ async function collectResources(
for (const file of files) {
const srcFile = path.join(resolvedPath, file);
const destFile = path.join(destDir, file);
const destFile = path.join(finalDestDir, file);
// Ensure parent directory exists
const destFileDir = path.dirname(destFile);