fix(installer): tighten lean shard-doc review follow-ups

This commit is contained in:
Dicky Moore 2026-03-08 17:17:58 +00:00
parent 1f7879f6c1
commit c56af3296e
3 changed files with 77 additions and 59 deletions

View File

@ -17,6 +17,7 @@ const fs = require('fs-extra');
const { YamlXmlBuilder } = require('../tools/cli/lib/yaml-xml-builder'); const { YamlXmlBuilder } = require('../tools/cli/lib/yaml-xml-builder');
const { ManifestGenerator } = require('../tools/cli/installers/lib/core/manifest-generator'); const { ManifestGenerator } = require('../tools/cli/installers/lib/core/manifest-generator');
const { IdeManager } = require('../tools/cli/installers/lib/ide/manager'); const { IdeManager } = require('../tools/cli/installers/lib/ide/manager');
const { ConfigDrivenIdeSetup } = require('../tools/cli/installers/lib/ide/_config-driven');
const { clearCache, loadPlatformCodes } = require('../tools/cli/installers/lib/ide/platform-codes'); const { clearCache, loadPlatformCodes } = require('../tools/cli/installers/lib/ide/platform-codes');
// ANSI colors // ANSI colors
@ -120,6 +121,15 @@ async function createShardDocPrototypeFixture() {
'', '',
].join('\n'), ].join('\n'),
); );
await fs.writeFile(
path.join(fixtureDir, 'core', 'tasks', 'bmad-shard-doc-skill-prototype', 'bmad-skill-manifest.yaml'),
[
'canonicalId: bmad-shard-doc-skill-prototype',
'type: task',
'description: "Prototype native skill wrapper for shard-doc during installer transition"',
'',
].join('\n'),
);
await fs.writeFile( await fs.writeFile(
path.join(fixtureDir, '_config', 'task-manifest.csv'), path.join(fixtureDir, '_config', 'task-manifest.csv'),
@ -882,68 +892,97 @@ async function runTests() {
console.log(''); console.log('');
// ============================================================ // ============================================================
// Test 11: Shard-doc Prototype Duplication (Skill-Format Only) // Test 11: Shard-doc Prototype Duplication (Skill/Non-Skill Scope)
// ============================================================ // ============================================================
console.log(`${colors.yellow}Test Suite 11: Shard-doc Prototype Duplication${colors.reset}\n`); console.log(`${colors.yellow}Test Suite 11: Shard-doc Prototype Duplication${colors.reset}\n`);
let tempCodexProjectDir; let tempSkillProjectDir;
let tempGeminiProjectDir; let tempNonSkillProjectDir;
let installedBmadDir; let installedBmadDir;
try { try {
clearCache(); clearCache();
const platformCodes = await loadPlatformCodes(); const platformCodes = await loadPlatformCodes();
const codexInstaller = platformCodes.platforms.codex?.installer; const skillFormatEntry = Object.entries(platformCodes.platforms || {}).find(([_, platform]) => {
const geminiInstaller = platformCodes.platforms.gemini?.installer; const installer = platform?.installer;
if (!installer || installer.skill_format !== true || typeof installer.target_dir !== 'string') return false;
if (Array.isArray(installer.artifact_types) && !installer.artifact_types.includes('tasks')) return false;
return true;
});
assert(codexInstaller?.skill_format === true, 'Codex installer uses skill_format output'); assert(Boolean(skillFormatEntry), 'Found a skill_format platform that installs task artifacts');
assert(geminiInstaller?.skill_format === true, 'Gemini installer uses skill_format output'); if (!skillFormatEntry) throw new Error('No suitable skill_format platform found for shard-doc prototype test');
tempCodexProjectDir = await fs.mkdtemp(path.join(os.tmpdir(), 'bmad-codex-prototype-test-')); const [skillFormatPlatformCode, skillFormatPlatform] = skillFormatEntry;
tempGeminiProjectDir = await fs.mkdtemp(path.join(os.tmpdir(), 'bmad-gemini-prototype-test-')); const skillInstaller = skillFormatPlatform.installer;
tempSkillProjectDir = await fs.mkdtemp(path.join(os.tmpdir(), 'bmad-skill-prototype-test-'));
tempNonSkillProjectDir = await fs.mkdtemp(path.join(os.tmpdir(), 'bmad-nonskill-prototype-test-'));
installedBmadDir = await createShardDocPrototypeFixture(); installedBmadDir = await createShardDocPrototypeFixture();
const ideManager = new IdeManager(); const ideManager = new IdeManager();
await ideManager.ensureInitialized(); await ideManager.ensureInitialized();
const codexResult = await ideManager.setup('codex', tempCodexProjectDir, installedBmadDir, { const skillResult = await ideManager.setup(skillFormatPlatformCode, tempSkillProjectDir, installedBmadDir, {
silent: true, silent: true,
selectedModules: ['bmm'], selectedModules: ['bmm'],
}); });
assert(codexResult.success === true, 'Codex setup succeeds for shard-doc prototype fixture'); assert(skillResult.success === true, `${skillFormatPlatformCode} setup succeeds for shard-doc prototype fixture`);
const codexCanonicalSkill = path.join(tempCodexProjectDir, '.agents', 'skills', 'bmad-shard-doc', 'SKILL.md'); const canonicalSkillPath = path.join(tempSkillProjectDir, skillInstaller.target_dir, 'bmad-shard-doc', 'SKILL.md');
const codexPrototypeSkill = path.join(tempCodexProjectDir, '.agents', 'skills', 'bmad-shard-doc-skill-prototype', 'SKILL.md'); const prototypeSkillPath = path.join(tempSkillProjectDir, skillInstaller.target_dir, 'bmad-shard-doc-skill-prototype', 'SKILL.md');
assert(await fs.pathExists(codexCanonicalSkill), 'Codex install writes canonical shard-doc skill'); assert(await fs.pathExists(canonicalSkillPath), `${skillFormatPlatformCode} install writes canonical shard-doc skill`);
assert(await fs.pathExists(codexPrototypeSkill), 'Codex install writes duplicated shard-doc prototype skill'); assert(await fs.pathExists(prototypeSkillPath), `${skillFormatPlatformCode} install writes duplicated shard-doc prototype skill`);
const codexCanonicalContent = await fs.readFile(codexCanonicalSkill, 'utf8'); const canonicalSkillContent = await fs.readFile(canonicalSkillPath, 'utf8');
const codexPrototypeContent = await fs.readFile(codexPrototypeSkill, 'utf8'); const prototypeSkillContent = await fs.readFile(prototypeSkillPath, 'utf8');
assert(codexCanonicalContent.includes('name: bmad-shard-doc'), 'Canonical shard-doc skill keeps canonical frontmatter name');
assert(canonicalSkillContent.includes('name: bmad-shard-doc'), 'Canonical shard-doc skill keeps canonical frontmatter name');
assert( assert(
codexPrototypeContent.includes('name: bmad-shard-doc-skill-prototype'), canonicalSkillContent.includes('Read the entire task file at: {project-root}/_bmad/core/tasks/shard-doc.xml'),
'Canonical shard-doc skill points to shard-doc.xml',
);
assert(
prototypeSkillContent.includes('name: bmad-shard-doc-skill-prototype'),
'Prototype shard-doc skill uses prototype frontmatter name', 'Prototype shard-doc skill uses prototype frontmatter name',
); );
assert( assert(
codexPrototypeContent.includes('Prototype marker: source-authored-skill'), prototypeSkillContent.includes('Prototype marker: source-authored-skill'),
'Prototype shard-doc skill is copied from source SKILL.md', 'Prototype shard-doc skill is copied from source SKILL.md',
); );
assert(
prototypeSkillContent.includes('Read and execute from: {project-root}/_bmad/core/tasks/shard-doc.xml'),
'Prototype shard-doc skill preserves source-authored shard-doc.xml reference',
);
const geminiResult = await ideManager.setup('gemini', tempGeminiProjectDir, installedBmadDir, { const nonSkillInstaller = {
...skillInstaller,
target_dir: '.legacy/prototype-commands',
skill_format: false,
artifact_types: ['tasks'],
};
const nonSkillHandler = new ConfigDrivenIdeSetup('prototype-nonskill-test', {
name: 'Prototype Non-Skill Test',
preferred: false,
installer: nonSkillInstaller,
});
const nonSkillResult = await nonSkillHandler.setup(tempNonSkillProjectDir, installedBmadDir, {
silent: true, silent: true,
selectedModules: ['bmm'], selectedModules: ['bmm'],
}); });
assert(geminiResult.success === true, 'Gemini setup succeeds for shard-doc prototype fixture'); assert(nonSkillResult.success === true, 'Synthetic non-skill-format setup succeeds for shard-doc prototype fixture');
const geminiCanonicalSkill = path.join(tempGeminiProjectDir, '.gemini', 'skills', 'bmad-shard-doc', 'SKILL.md'); const nonSkillTargetDir = path.join(tempNonSkillProjectDir, nonSkillInstaller.target_dir);
const geminiPrototypeSkill = path.join(tempGeminiProjectDir, '.gemini', 'skills', 'bmad-shard-doc-skill-prototype', 'SKILL.md'); const nonSkillEntries = await fs.readdir(nonSkillTargetDir);
assert(await fs.pathExists(geminiCanonicalSkill), 'Gemini install writes canonical shard-doc skill'); const hasCanonical = nonSkillEntries.some((entry) => /^bmad-shard-doc(\.|$)/.test(entry));
assert(await fs.pathExists(geminiPrototypeSkill), 'Gemini install writes duplicated shard-doc prototype skill'); const hasPrototype = nonSkillEntries.some((entry) => /^bmad-shard-doc-skill-prototype(\.|$)/.test(entry));
assert(hasCanonical, 'Non-skill-format install writes canonical shard-doc artifact');
assert(!hasPrototype, 'Non-skill-format install does not write duplicated shard-doc prototype artifact');
} catch (error) { } catch (error) {
assert(false, 'Shard-doc prototype duplication test succeeds', error.message); assert(false, 'Shard-doc prototype duplication test succeeds', error.message);
} finally { } finally {
await Promise.allSettled([tempCodexProjectDir, tempGeminiProjectDir, installedBmadDir].filter(Boolean).map((dir) => fs.remove(dir))); await Promise.allSettled([tempSkillProjectDir, tempNonSkillProjectDir, installedBmadDir].filter(Boolean).map((dir) => fs.remove(dir)));
} }
// Test 17: GitHub Copilot Native Skills Install // Test 17: GitHub Copilot Native Skills Install

View File

@ -558,6 +558,7 @@ LOAD and execute from: {project-root}/{{bmadFolderName}}/{{path}}
resolveArtifactSourceRef(artifact, bmadDir) { resolveArtifactSourceRef(artifact, bmadDir) {
if (artifact.type !== 'task' || !artifact.path) return null; if (artifact.type !== 'task' || !artifact.path) return null;
const sourcePath = artifact.path; const sourcePath = artifact.path;
const resolvedBmadDir = path.resolve(bmadDir);
let normalized = sourcePath.replaceAll('\\', '/'); let normalized = sourcePath.replaceAll('\\', '/');
if (path.isAbsolute(normalized)) { if (path.isAbsolute(normalized)) {
@ -578,7 +579,10 @@ LOAD and execute from: {project-root}/{{bmadFolderName}}/{{path}}
if (!filename || filename === '.' || filename === '..') return null; if (!filename || filename === '.' || filename === '..') return null;
const relativeDir = path.dirname(normalized); const relativeDir = path.dirname(normalized);
const dirPath = relativeDir === '.' ? bmadDir : path.join(bmadDir, relativeDir); const dirPath = relativeDir === '.' ? resolvedBmadDir : path.resolve(resolvedBmadDir, relativeDir);
const relativeToRoot = path.relative(resolvedBmadDir, dirPath);
if (relativeToRoot === '..' || relativeToRoot.startsWith(`..${path.sep}`) || path.isAbsolute(relativeToRoot)) return null;
return { dirPath, filename }; return { dirPath, filename };
} }

View File

@ -37,7 +37,7 @@ function getCanonicalId(manifest, filename) {
/** /**
* Resolve a manifest entry for a source filename. * Resolve a manifest entry for a source filename.
* Handles single-entry manifests and extension fallbacks. * Strict by default: supports single-entry manifests and exact filename keys only.
* @param {Object|null} manifest - Loaded manifest * @param {Object|null} manifest - Loaded manifest
* @param {string} filename - Source filename * @param {string} filename - Source filename
* @returns {Object|null} Manifest entry object * @returns {Object|null} Manifest entry object
@ -48,12 +48,6 @@ function resolveManifestEntry(manifest, filename) {
if (manifest.__single) return manifest.__single; if (manifest.__single) return manifest.__single;
// Multi-entry: look up by filename directly // Multi-entry: look up by filename directly
if (manifest[filename]) return manifest[filename]; if (manifest[filename]) return manifest[filename];
// Fallback: try alternate extensions for compiled files
const baseName = filename.replace(/\.(md|xml)$/i, '');
const agentKey = `${baseName}.agent.yaml`;
if (manifest[agentKey]) return manifest[agentKey];
const xmlKey = `${baseName}.xml`;
if (manifest[xmlKey]) return manifest[xmlKey];
return null; return null;
} }
@ -64,18 +58,8 @@ function resolveManifestEntry(manifest, filename) {
* @returns {string|null} type or null * @returns {string|null} type or null
*/ */
function getArtifactType(manifest, filename) { function getArtifactType(manifest, filename) {
if (!manifest) return null; const manifestEntry = resolveManifestEntry(manifest, filename);
// Single-entry manifest applies to all files in the directory return manifestEntry?.type || null;
if (manifest.__single) return manifest.__single.type || null;
// Multi-entry: look up by filename directly
if (manifest[filename]) return manifest[filename].type || null;
// Fallback: try alternate extensions for compiled files
const baseName = filename.replace(/\.(md|xml)$/i, '');
const agentKey = `${baseName}.agent.yaml`;
if (manifest[agentKey]) return manifest[agentKey].type || null;
const xmlKey = `${baseName}.xml`;
if (manifest[xmlKey]) return manifest[xmlKey].type || null;
return null;
} }
/** /**
@ -85,18 +69,9 @@ function getArtifactType(manifest, filename) {
* @returns {boolean} install_to_bmad value (defaults to true) * @returns {boolean} install_to_bmad value (defaults to true)
*/ */
function getInstallToBmad(manifest, filename) { function getInstallToBmad(manifest, filename) {
if (!manifest) return true; const manifestEntry = resolveManifestEntry(manifest, filename);
// Single-entry manifest applies to all files in the directory if (!manifestEntry) return true;
if (manifest.__single) return manifest.__single.install_to_bmad !== false; return manifestEntry.install_to_bmad !== false;
// Multi-entry: look up by filename directly
if (manifest[filename]) return manifest[filename].install_to_bmad !== false;
// Fallback: try alternate extensions for compiled files
const baseName = filename.replace(/\.(md|xml)$/i, '');
const agentKey = `${baseName}.agent.yaml`;
if (manifest[agentKey]) return manifest[agentKey].install_to_bmad !== false;
const xmlKey = `${baseName}.xml`;
if (manifest[xmlKey]) return manifest[xmlKey].install_to_bmad !== false;
return true;
} }
module.exports = { loadSkillManifest, getCanonicalId, getArtifactType, getInstallToBmad }; module.exports = { loadSkillManifest, getCanonicalId, getArtifactType, getInstallToBmad };