fix(cli): address PR review findings (mkdtemp, regex escape, recursive filter)

- Replace Date.now() temp dir with fs.mkdtemp() in Suite 30 tests (F5)
- Replace unescaped RegExp with startsWith/slice for path prefix stripping (F7)
- Apply artifact filter recursively via fs.copy filter option (F8)

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
Alex Verkhovsky 2026-03-09 01:54:51 -06:00
parent 8d0e49cc28
commit e9afa39e7e
2 changed files with 14 additions and 14 deletions

View File

@ -1710,8 +1710,7 @@ async function runTests() {
let tempFixture30; let tempFixture30;
try { try {
tempFixture30 = path.join(os.tmpdir(), `bmad-test-30-${Date.now()}`); tempFixture30 = await fs.mkdtemp(path.join(os.tmpdir(), 'bmad-test-30-'));
await fs.ensureDir(tempFixture30);
const generator30 = new ManifestGenerator(); const generator30 = new ManifestGenerator();
generator30.bmadFolderName = '_bmad'; generator30.bmadFolderName = '_bmad';

View File

@ -637,6 +637,7 @@ LOAD and execute from: {project-root}/{{bmadFolderName}}/{{path}}
*/ */
async installVerbatimSkills(projectDir, bmadDir, targetPath, config) { async installVerbatimSkills(projectDir, bmadDir, targetPath, config) {
const bmadFolderName = path.basename(bmadDir); const bmadFolderName = path.basename(bmadDir);
const bmadPrefix = bmadFolderName + '/';
const csvPath = path.join(bmadDir, '_config', 'skill-manifest.csv'); const csvPath = path.join(bmadDir, '_config', 'skill-manifest.csv');
if (!(await fs.pathExists(csvPath))) return 0; if (!(await fs.pathExists(csvPath))) return 0;
@ -656,7 +657,7 @@ LOAD and execute from: {project-root}/{{bmadFolderName}}/{{path}}
// Derive source directory from path column // Derive source directory from path column
// path is like "_bmad/bmm/workflows/bmad-quick-flow/bmad-quick-dev-new-preview/SKILL.md" // path is like "_bmad/bmm/workflows/bmad-quick-flow/bmad-quick-dev-new-preview/SKILL.md"
// Strip bmadFolderName prefix and join with bmadDir, then get dirname // Strip bmadFolderName prefix and join with bmadDir, then get dirname
const relativePath = record.path.replace(new RegExp(`^${bmadFolderName}/`), ''); const relativePath = record.path.startsWith(bmadPrefix) ? record.path.slice(bmadPrefix.length) : record.path;
const sourceFile = path.join(bmadDir, relativePath); const sourceFile = path.join(bmadDir, relativePath);
const sourceDir = path.dirname(sourceFile); const sourceDir = path.dirname(sourceFile);
@ -667,18 +668,18 @@ LOAD and execute from: {project-root}/{{bmadFolderName}}/{{path}}
await fs.remove(skillDir); await fs.remove(skillDir);
await fs.ensureDir(skillDir); await fs.ensureDir(skillDir);
// Copy all skill files, filtering OS/editor artifacts // Copy all skill files, filtering OS/editor artifacts recursively
const skipPatterns = new Set(['.DS_Store', 'Thumbs.db', 'desktop.ini']); const skipPatterns = new Set(['.DS_Store', 'Thumbs.db', 'desktop.ini']);
const skipSuffixes = ['~', '.swp', '.swo', '.bak']; const skipSuffixes = ['~', '.swp', '.swo', '.bak'];
const entries = await fs.readdir(sourceDir, { withFileTypes: true }); const filter = (src) => {
for (const entry of entries) { const name = path.basename(src);
if (skipPatterns.has(entry.name)) continue; if (src === sourceDir) return true;
if (entry.name.startsWith('.') && entry.name !== '.gitkeep') continue; if (skipPatterns.has(name)) return false;
if (skipSuffixes.some((s) => entry.name.endsWith(s))) continue; if (name.startsWith('.') && name !== '.gitkeep') return false;
const srcPath = path.join(sourceDir, entry.name); if (skipSuffixes.some((s) => name.endsWith(s))) return false;
const destPath = path.join(skillDir, entry.name); return true;
await fs.copy(srcPath, destPath); };
} await fs.copy(sourceDir, skillDir, { filter });
count++; count++;
} }
@ -686,7 +687,7 @@ LOAD and execute from: {project-root}/{{bmadFolderName}}/{{path}}
// Post-install cleanup: remove _bmad/ directories for skills with install_to_bmad === "false" // Post-install cleanup: remove _bmad/ directories for skills with install_to_bmad === "false"
for (const record of records) { for (const record of records) {
if (record.install_to_bmad === 'false') { if (record.install_to_bmad === 'false') {
const relativePath = record.path.replace(new RegExp(`^${bmadFolderName}/`), ''); const relativePath = record.path.startsWith(bmadPrefix) ? record.path.slice(bmadPrefix.length) : record.path;
const sourceFile = path.join(bmadDir, relativePath); const sourceFile = path.join(bmadDir, relativePath);
const sourceDir = path.dirname(sourceFile); const sourceDir = path.dirname(sourceFile);
if (await fs.pathExists(sourceDir)) { if (await fs.pathExists(sourceDir)) {