ldy26098
/
BMAD-METHOD
mirror of https://github.com/bmad-code-org/BMAD-METHOD.git
1
0
Fork 0
Code Packages Projects Releases Wiki Activity

fix(installer): address PR review findings from #2078 

- Fix temp dir leak in test fixture cleanup (use path.dirname)
- Fail loudly when skill_format missing instead of silent success
- Add workflow.md to test fixture for verbatim-copy coverage

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
Alex Verkhovsky 2026-03-20 14:58:24 -06:00
parent f337d25ad6
commit 6c761bb896
2 changed files with 30 additions and 26 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

54
test/test-installation-components.js
View File

@ -80,6 +80,7 @@ async function createTestBmadFixture() {
].join('\n'), ].join('\n'),
); );
await fs.writeFile(path.join(skillDir, 'bmad-skill-manifest.yaml'), 'SKILL.md:\n type: skill\n'); await fs.writeFile(path.join(skillDir, 'bmad-skill-manifest.yaml'), 'SKILL.md:\n type: skill\n');
await fs.writeFile(path.join(skillDir, 'workflow.md'), '# Test Workflow\nStep 1: Do the thing.\n');
return fixtureDir; return fixtureDir;
} }
@ -256,7 +257,7 @@ async function runTests() {
assert(!(await fs.pathExists(path.join(tempProjectDir, '.windsurf', 'workflows'))), 'Windsurf setup removes legacy workflows dir'); assert(!(await fs.pathExists(path.join(tempProjectDir, '.windsurf', 'workflows'))), 'Windsurf setup removes legacy workflows dir');
await fs.remove(tempProjectDir); await fs.remove(tempProjectDir);
await fs.remove(installedBmadDir); await fs.remove(path.dirname(installedBmadDir));
} catch (error) { } catch (error) {
assert(false, 'Windsurf native skills migration test succeeds', error.message); assert(false, 'Windsurf native skills migration test succeeds', error.message);
} }
@ -304,7 +305,7 @@ async function runTests() {
assert(!(await fs.pathExists(path.join(tempProjectDir, '.kiro', 'steering'))), 'Kiro setup removes legacy steering dir'); assert(!(await fs.pathExists(path.join(tempProjectDir, '.kiro', 'steering'))), 'Kiro setup removes legacy steering dir');
await fs.remove(tempProjectDir); await fs.remove(tempProjectDir);
await fs.remove(installedBmadDir); await fs.remove(path.dirname(installedBmadDir));
} catch (error) { } catch (error) {
assert(false, 'Kiro native skills migration test succeeds', error.message); assert(false, 'Kiro native skills migration test succeeds', error.message);
} }
@ -352,7 +353,7 @@ async function runTests() {
assert(!(await fs.pathExists(path.join(tempProjectDir, '.agent', 'workflows'))), 'Antigravity setup removes legacy workflows dir'); assert(!(await fs.pathExists(path.join(tempProjectDir, '.agent', 'workflows'))), 'Antigravity setup removes legacy workflows dir');
await fs.remove(tempProjectDir); await fs.remove(tempProjectDir);
await fs.remove(installedBmadDir); await fs.remove(path.dirname(installedBmadDir));
} catch (error) { } catch (error) {
assert(false, 'Antigravity native skills migration test succeeds', error.message); assert(false, 'Antigravity native skills migration test succeeds', error.message);
} }
@ -405,7 +406,7 @@ async function runTests() {
assert(!(await fs.pathExists(path.join(tempProjectDir, '.augment', 'commands'))), 'Auggie setup removes legacy commands dir'); assert(!(await fs.pathExists(path.join(tempProjectDir, '.augment', 'commands'))), 'Auggie setup removes legacy commands dir');
await fs.remove(tempProjectDir); await fs.remove(tempProjectDir);
await fs.remove(installedBmadDir); await fs.remove(path.dirname(installedBmadDir));
} catch (error) { } catch (error) {
assert(false, 'Auggie native skills migration test succeeds', error.message); assert(false, 'Auggie native skills migration test succeeds', error.message);
} }
@ -471,7 +472,7 @@ async function runTests() {
} }
await fs.remove(tempProjectDir); await fs.remove(tempProjectDir);
await fs.remove(installedBmadDir); await fs.remove(path.dirname(installedBmadDir));
} catch (error) { } catch (error) {
assert(false, 'OpenCode native skills migration test succeeds', error.message); assert(false, 'OpenCode native skills migration test succeeds', error.message);
} }
@ -525,7 +526,7 @@ async function runTests() {
assert(!(await fs.pathExists(legacyDir9)), 'Claude Code setup removes legacy commands dir'); assert(!(await fs.pathExists(legacyDir9)), 'Claude Code setup removes legacy commands dir');
await fs.remove(tempProjectDir9); await fs.remove(tempProjectDir9);
await fs.remove(installedBmadDir9); await fs.remove(path.dirname(installedBmadDir9));
} catch (error) { } catch (error) {
assert(false, 'Claude Code native skills migration test succeeds', error.message); assert(false, 'Claude Code native skills migration test succeeds', error.message);
} }
@ -564,7 +565,7 @@ async function runTests() {
); );
await fs.remove(tempRoot10); await fs.remove(tempRoot10);
await fs.remove(installedBmadDir10); await fs.remove(path.dirname(installedBmadDir10));
} catch (error) { } catch (error) {
assert(false, 'Claude Code ancestor conflict protection test succeeds', error.message); assert(false, 'Claude Code ancestor conflict protection test succeeds', error.message);
} }
@ -618,7 +619,7 @@ async function runTests() {
assert(!(await fs.pathExists(legacyDir11)), 'Codex setup removes legacy prompts dir'); assert(!(await fs.pathExists(legacyDir11)), 'Codex setup removes legacy prompts dir');
await fs.remove(tempProjectDir11); await fs.remove(tempProjectDir11);
await fs.remove(installedBmadDir11); await fs.remove(path.dirname(installedBmadDir11));
} catch (error) { } catch (error) {
assert(false, 'Codex native skills migration test succeeds', error.message); assert(false, 'Codex native skills migration test succeeds', error.message);
} }
@ -654,7 +655,7 @@ async function runTests() {
assert(result12.handlerResult?.conflictDir === expectedConflictDir12, 'Codex ancestor rejection points at ancestor .agents/skills dir'); assert(result12.handlerResult?.conflictDir === expectedConflictDir12, 'Codex ancestor rejection points at ancestor .agents/skills dir');
await fs.remove(tempRoot12); await fs.remove(tempRoot12);
await fs.remove(installedBmadDir12); await fs.remove(path.dirname(installedBmadDir12));
} catch (error) { } catch (error) {
assert(false, 'Codex ancestor conflict protection test succeeds', error.message); assert(false, 'Codex ancestor conflict protection test succeeds', error.message);
} }
@ -708,7 +709,7 @@ async function runTests() {
assert(!(await fs.pathExists(legacyDir13c)), 'Cursor setup removes legacy commands dir'); assert(!(await fs.pathExists(legacyDir13c)), 'Cursor setup removes legacy commands dir');
await fs.remove(tempProjectDir13c); await fs.remove(tempProjectDir13c);
await fs.remove(installedBmadDir13c); await fs.remove(path.dirname(installedBmadDir13c));
} catch (error) { } catch (error) {
assert(false, 'Cursor native skills migration test succeeds', error.message); assert(false, 'Cursor native skills migration test succeeds', error.message);
} }
@ -773,7 +774,7 @@ async function runTests() {
assert(await fs.pathExists(skillFile13), 'Roo reinstall preserves SKILL.md output'); assert(await fs.pathExists(skillFile13), 'Roo reinstall preserves SKILL.md output');
await fs.remove(tempProjectDir13); await fs.remove(tempProjectDir13);
await fs.remove(installedBmadDir13); await fs.remove(path.dirname(installedBmadDir13));
} catch (error) { } catch (error) {
assert(false, 'Roo native skills migration test succeeds', error.message); assert(false, 'Roo native skills migration test succeeds', error.message);
} }
@ -812,7 +813,7 @@ async function runTests() {
); );
await fs.remove(tempRoot); await fs.remove(tempRoot);
await fs.remove(installedBmadDir); await fs.remove(path.dirname(installedBmadDir));
} catch (error) { } catch (error) {
assert(false, 'OpenCode ancestor conflict protection test succeeds', error.message); assert(false, 'OpenCode ancestor conflict protection test succeeds', error.message);
} }
@ -898,7 +899,7 @@ async function runTests() {
); );
await fs.remove(tempProjectDir17); await fs.remove(tempProjectDir17);
await fs.remove(installedBmadDir17); await fs.remove(path.dirname(installedBmadDir17));
} catch (error) { } catch (error) {
assert(false, 'GitHub Copilot native skills migration test succeeds', error.message); assert(false, 'GitHub Copilot native skills migration test succeeds', error.message);
} }
@ -960,7 +961,7 @@ async function runTests() {
assert(await fs.pathExists(skillFile18), 'Cline reinstall preserves SKILL.md output'); assert(await fs.pathExists(skillFile18), 'Cline reinstall preserves SKILL.md output');
await fs.remove(tempProjectDir18); await fs.remove(tempProjectDir18);
await fs.remove(installedBmadDir18); await fs.remove(path.dirname(installedBmadDir18));
} catch (error) { } catch (error) {
assert(false, 'Cline native skills migration test succeeds', error.message); assert(false, 'Cline native skills migration test succeeds', error.message);
} }
@ -1020,7 +1021,7 @@ async function runTests() {
assert(await fs.pathExists(skillFile19), 'CodeBuddy reinstall preserves SKILL.md output'); assert(await fs.pathExists(skillFile19), 'CodeBuddy reinstall preserves SKILL.md output');
await fs.remove(tempProjectDir19); await fs.remove(tempProjectDir19);
await fs.remove(installedBmadDir19); await fs.remove(path.dirname(installedBmadDir19));
} catch (error) { } catch (error) {
assert(false, 'CodeBuddy native skills migration test succeeds', error.message); assert(false, 'CodeBuddy native skills migration test succeeds', error.message);
} }
@ -1080,7 +1081,7 @@ async function runTests() {
assert(await fs.pathExists(skillFile20), 'Crush reinstall preserves SKILL.md output'); assert(await fs.pathExists(skillFile20), 'Crush reinstall preserves SKILL.md output');
await fs.remove(tempProjectDir20); await fs.remove(tempProjectDir20);
await fs.remove(installedBmadDir20); await fs.remove(path.dirname(installedBmadDir20));
} catch (error) { } catch (error) {
assert(false, 'Crush native skills migration test succeeds', error.message); assert(false, 'Crush native skills migration test succeeds', error.message);
} }
@ -1139,7 +1140,7 @@ async function runTests() {
assert(await fs.pathExists(skillFile21), 'Trae reinstall preserves SKILL.md output'); assert(await fs.pathExists(skillFile21), 'Trae reinstall preserves SKILL.md output');
await fs.remove(tempProjectDir21); await fs.remove(tempProjectDir21);
await fs.remove(installedBmadDir21); await fs.remove(path.dirname(installedBmadDir21));
} catch (error) { } catch (error) {
assert(false, 'Trae native skills migration test succeeds', error.message); assert(false, 'Trae native skills migration test succeeds', error.message);
} }
@ -1197,7 +1198,7 @@ async function runTests() {
); );
await fs.remove(tempProjectDir22); await fs.remove(tempProjectDir22);
await fs.remove(installedBmadDir22); await fs.remove(path.dirname(installedBmadDir22));
} catch (error) { } catch (error) {
assert(false, 'KiloCoder suspended test succeeds', error.message); assert(false, 'KiloCoder suspended test succeeds', error.message);
} }
@ -1256,7 +1257,7 @@ async function runTests() {
assert(await fs.pathExists(skillFile23), 'Gemini reinstall preserves SKILL.md output'); assert(await fs.pathExists(skillFile23), 'Gemini reinstall preserves SKILL.md output');
await fs.remove(tempProjectDir23); await fs.remove(tempProjectDir23);
await fs.remove(installedBmadDir23); await fs.remove(path.dirname(installedBmadDir23));
} catch (error) { } catch (error) {
assert(false, 'Gemini native skills migration test succeeds', error.message); assert(false, 'Gemini native skills migration test succeeds', error.message);
} }
@ -1306,7 +1307,7 @@ async function runTests() {
assert(!(await fs.pathExists(path.join(tempProjectDir24, '.iflow', 'commands'))), 'iFlow setup removes legacy commands dir'); assert(!(await fs.pathExists(path.join(tempProjectDir24, '.iflow', 'commands'))), 'iFlow setup removes legacy commands dir');
await fs.remove(tempProjectDir24); await fs.remove(tempProjectDir24);
await fs.remove(installedBmadDir24); await fs.remove(path.dirname(installedBmadDir24));
} catch (error) { } catch (error) {
assert(false, 'iFlow native skills migration test succeeds', error.message); assert(false, 'iFlow native skills migration test succeeds', error.message);
} }
@ -1356,7 +1357,7 @@ async function runTests() {
assert(!(await fs.pathExists(path.join(tempProjectDir25, '.qwen', 'commands'))), 'QwenCoder setup removes legacy commands dir'); assert(!(await fs.pathExists(path.join(tempProjectDir25, '.qwen', 'commands'))), 'QwenCoder setup removes legacy commands dir');
await fs.remove(tempProjectDir25); await fs.remove(tempProjectDir25);
await fs.remove(installedBmadDir25); await fs.remove(path.dirname(installedBmadDir25));
} catch (error) { } catch (error) {
assert(false, 'QwenCoder native skills migration test succeeds', error.message); assert(false, 'QwenCoder native skills migration test succeeds', error.message);
} }
@ -1425,7 +1426,7 @@ async function runTests() {
assert(cleanedPrompts26.prompts[0].name === 'my-custom-prompt', 'Rovo Dev cleanup preserves non-BMAD entries in prompts.yml'); assert(cleanedPrompts26.prompts[0].name === 'my-custom-prompt', 'Rovo Dev cleanup preserves non-BMAD entries in prompts.yml');
await fs.remove(tempProjectDir26); await fs.remove(tempProjectDir26);
await fs.remove(installedBmadDir26); await fs.remove(path.dirname(installedBmadDir26));
} catch (error) { } catch (error) {
assert(false, 'Rovo Dev native skills migration test succeeds', error.message); assert(false, 'Rovo Dev native skills migration test succeeds', error.message);
} }
@ -1490,7 +1491,7 @@ async function runTests() {
assert(!(await fs.pathExists(regularSkillDir27)), 'Cleanup removes stale non-bmad-os skills'); assert(!(await fs.pathExists(regularSkillDir27)), 'Cleanup removes stale non-bmad-os skills');
await fs.remove(tempProjectDir27); await fs.remove(tempProjectDir27);
await fs.remove(installedBmadDir27); await fs.remove(path.dirname(installedBmadDir27));
} catch (error) { } catch (error) {
assert(false, 'bmad-os-* skill preservation test succeeds', error.message); assert(false, 'bmad-os-* skill preservation test succeeds', error.message);
} }
@ -1582,7 +1583,7 @@ async function runTests() {
assert(false, 'Pi native skills test succeeds', error.message); assert(false, 'Pi native skills test succeeds', error.message);
} finally { } finally {
if (tempProjectDir28) await fs.remove(tempProjectDir28).catch(() => {}); if (tempProjectDir28) await fs.remove(tempProjectDir28).catch(() => {});
if (installedBmadDir28) await fs.remove(installedBmadDir28).catch(() => {}); if (installedBmadDir28) await fs.remove(path.dirname(installedBmadDir28)).catch(() => {});
} }
console.log(''); console.log('');
@ -1903,6 +1904,9 @@ async function runTests() {
const skillFile32 = path.join(tempProjectDir32, '.ona', 'skills', 'bmad-master', 'SKILL.md'); const skillFile32 = path.join(tempProjectDir32, '.ona', 'skills', 'bmad-master', 'SKILL.md');
assert(await fs.pathExists(skillFile32), 'Ona install writes SKILL.md directory output'); assert(await fs.pathExists(skillFile32), 'Ona install writes SKILL.md directory output');
const workflowFile32 = path.join(tempProjectDir32, '.ona', 'skills', 'bmad-master', 'workflow.md');
assert(await fs.pathExists(workflowFile32), 'Ona install copies non-SKILL.md files (workflow.md) verbatim');
// Parse YAML frontmatter between --- markers // Parse YAML frontmatter between --- markers
const skillContent32 = await fs.readFile(skillFile32, 'utf8'); const skillContent32 = await fs.readFile(skillFile32, 'utf8');
const fmMatch32 = skillContent32.match(/^---\n([\s\S]*?)\n---\n?([\s\S]*)$/); const fmMatch32 = skillContent32.match(/^---\n([\s\S]*?)\n---\n?([\s\S]*)$/);
@ -1941,7 +1945,7 @@ async function runTests() {
assert(false, 'Ona native skills test succeeds', error.message); assert(false, 'Ona native skills test succeeds', error.message);
} finally { } finally {
if (tempProjectDir32) await fs.remove(tempProjectDir32).catch(() => {}); if (tempProjectDir32) await fs.remove(tempProjectDir32).catch(() => {});
if (installedBmadDir32) await fs.remove(installedBmadDir32).catch(() => {}); if (installedBmadDir32) await fs.remove(path.dirname(installedBmadDir32)).catch(() => {});
} }
console.log(''); console.log('');

2
tools/cli/installers/lib/ide/_config-driven.js
View File

@ -115,7 +115,7 @@ class ConfigDrivenIdeSetup extends BaseIdeSetup {
const { target_dir } = config; const { target_dir } = config;
if (!config.skill_format) { if (!config.skill_format) {
return { success: true, results: { skills: 0 } }; return { success: false, reason: 'missing-skill-format', error: 'Installer config missing skill_format — cannot install skills' };
} }
const targetPath = path.join(projectDir, target_dir); const targetPath = path.join(projectDir, target_dir);