From 32e210618c5a1f0e6cc6888fa7ebe8506f80493d Mon Sep 17 00:00:00 2001 From: LaZzyMan Date: Wed, 28 Jan 2026 11:15:57 +0800 Subject: [PATCH] fix: preserve directory structure when collecting Claude plugin resources - Fixed issue where all skills were installed instead of only configured ones - Resource paths now preserve subdirectory names (e.g., skills/xlsx -> skills/xlsx/) - Support default fallback: use all resources from folder if not specified in config - Added comprehensive tests covering explicit config, default behavior, and nested structures --- .../src/extension/claude-converter.test.ts | 232 +++++++++++++++++- .../core/src/extension/claude-converter.ts | 52 ++-- 2 files changed, 263 insertions(+), 21 deletions(-) diff --git a/packages/core/src/extension/claude-converter.test.ts b/packages/core/src/extension/claude-converter.test.ts index 9e74b07bf..079d46b19 100644 --- a/packages/core/src/extension/claude-converter.test.ts +++ b/packages/core/src/extension/claude-converter.test.ts @@ -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 }); + }); +}); diff --git a/packages/core/src/extension/claude-converter.ts b/packages/core/src/extension/claude-converter.ts index 224a22b11..a27084ed7 100644 --- a/packages/core/src/extension/claude-converter.ts +++ b/packages/core/src/extension/claude-converter.ts @@ -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);