From 86556edfccefbaa8453ffe5add632bb6e0706c5c Mon Sep 17 00:00:00 2001 From: Dicky Moore Date: Sat, 7 Mar 2026 18:02:27 +0000 Subject: [PATCH] fix(installer): trim shard-doc prototype PR and address review feedback --- .../phase-1-analysis.md | 44 ----------------- .../phase-2-prd.md | 33 ------------- .../phase-3-architecture.md | 47 ------------------- .../phase-4-implementation.md | 32 ------------- test/test-installation-components.js | 20 +++++--- .../cli/installers/lib/ide/_config-driven.js | 18 +++++-- 6 files changed, 29 insertions(+), 165 deletions(-) delete mode 100644 docs/native-skills-lean-shard-doc-prototype/phase-1-analysis.md delete mode 100644 docs/native-skills-lean-shard-doc-prototype/phase-2-prd.md delete mode 100644 docs/native-skills-lean-shard-doc-prototype/phase-3-architecture.md delete mode 100644 docs/native-skills-lean-shard-doc-prototype/phase-4-implementation.md diff --git a/docs/native-skills-lean-shard-doc-prototype/phase-1-analysis.md b/docs/native-skills-lean-shard-doc-prototype/phase-1-analysis.md deleted file mode 100644 index 0427e945d..000000000 --- a/docs/native-skills-lean-shard-doc-prototype/phase-1-analysis.md +++ /dev/null @@ -1,44 +0,0 @@ -# Phase 1 Analysis: Native Skills Lean PoC - -Date: 2026-03-07 -Branch: `feature/native-skills-lean-shard-doc-prototype` -North-star reference: `docs/native-skills-transition-north-star-thread-2026-03-07.md` - -## Problem Statement - -The prior native-skills transition effort overshot scope. This recovery PoC must prove a single end-to-end duplicate native-skill path while preserving all current legacy task/help behavior. - -## Scope - -In scope: - -1. One duplicated native-skill prototype only: `bmad-shard-doc-skill-prototype` -2. Source capability remains `src/core/tasks/shard-doc.xml` -3. Installer behavior only for supported native-skill tools: - - discover prototype metadata - - register/copy to skill output surface -4. Minimal tests proving prototype duplication for skill tools and no regression for non-skill tools - -Out of scope: - -1. Multi-capability conversion -2. Broad authority/metadata redesign -3. Command/help surface changes -4. Repository-wide migration framework - -## Constraints - -1. Keep `module-help.csv` behavior unchanged -2. Keep legacy `bmad-shard-doc` capability intact -3. Keep PR lean and reviewable -4. Avoid touching unrelated installer paths - -## Risks and Mitigations - -1. Risk: duplicate visible command surfaces - Mitigation: apply prototype duplication only on `skill_format` installers -2. Risk: behavior drift for legacy task/help paths - Mitigation: no edits to legacy task file, task/help catalogs, or command generation rules -3. Risk: hidden regressions across tool outputs - Mitigation: add targeted install-component tests for one skill-format and one non-skill tool - diff --git a/docs/native-skills-lean-shard-doc-prototype/phase-2-prd.md b/docs/native-skills-lean-shard-doc-prototype/phase-2-prd.md deleted file mode 100644 index 19d966963..000000000 --- a/docs/native-skills-lean-shard-doc-prototype/phase-2-prd.md +++ /dev/null @@ -1,33 +0,0 @@ -# Phase 2 PRD: Native Skills Lean PoC - -Date: 2026-03-07 -Branch: `feature/native-skills-lean-shard-doc-prototype` - -## Goal - -Ship a narrow, testable PoC that installs a duplicated native skill for shard-doc as `bmad-shard-doc-skill-prototype` while preserving existing shard-doc command/help behavior. - -## Functional Requirements - -1. The core task skill metadata supports a prototype duplicate ID for `shard-doc.xml`. -2. Installer discovery reads the prototype duplicate ID from source metadata. -3. For `skill_format` tools, installer writes both: - - canonical skill: `bmad-shard-doc/SKILL.md` - - prototype skill: `bmad-shard-doc-skill-prototype/SKILL.md` -4. For non-`skill_format` tools, installer output remains unchanged (no prototype duplicate command file). -5. Existing shard-doc legacy artifact remains available via current task/help flows. - -## Non-Functional Requirements - -1. PR stays lean (minimal files and logic changes). -2. No behavior change for existing command/help interfaces. -3. Tests are deterministic and run in current installation component suite. - -## Acceptance Criteria - -1. `src/core/tasks/shard-doc.xml` remains unchanged as the legacy capability artifact. -2. Installing for Codex creates `bmad-shard-doc-skill-prototype/SKILL.md` in `.agents/skills`. -3. Installing for Codex still creates the existing `bmad-shard-doc/SKILL.md`. -4. Installing for Gemini does not create `bmad-shard-doc-skill-prototype` command output. -5. Existing install-component suite continues to pass with added assertions. - diff --git a/docs/native-skills-lean-shard-doc-prototype/phase-3-architecture.md b/docs/native-skills-lean-shard-doc-prototype/phase-3-architecture.md deleted file mode 100644 index e9441b133..000000000 --- a/docs/native-skills-lean-shard-doc-prototype/phase-3-architecture.md +++ /dev/null @@ -1,47 +0,0 @@ -# Phase 3 Architecture: Native Skills Lean PoC - -Date: 2026-03-07 -Branch: `feature/native-skills-lean-shard-doc-prototype` - -## Existing Baseline - -1. Installer already uses `bmad-skill-manifest.yaml` for canonical skill IDs. -2. `skill_format` platforms write directory-based skills (`/SKILL.md`). -3. Task/help command surfaces are driven by existing manifests/catalogs. - -## Proposed Minimal Design - -### 1) Metadata Extension - -Extend per-file skill metadata to optionally include duplicate prototype IDs: - -```yaml -shard-doc.xml: - canonicalId: bmad-shard-doc - prototypeIds: - - bmad-shard-doc-skill-prototype -``` - -### 2) Installer Duplication Rule - -In config-driven IDE setup, when `skill_format` is enabled: - -1. Write canonical skill output as today. -2. Resolve prototype IDs for the same source artifact from sidecar metadata. -3. Write additional `SKILL.md` outputs under each prototype ID directory. - -No duplication is applied for non-`skill_format` outputs. - -### 3) Invariants - -1. Legacy source artifact remains `src/core/tasks/shard-doc.xml`. -2. Existing help/command catalogs remain unchanged. -3. No new artifact category or broad migration framework introduced. - -## Touched Components - -1. `src/core/tasks/bmad-skill-manifest.yaml` (prototype metadata for shard-doc) -2. `tools/cli/installers/lib/ide/shared/skill-manifest.js` (read prototype IDs) -3. `tools/cli/installers/lib/ide/_config-driven.js` (duplicate skill write for skill-format installers) -4. `test/test-installation-components.js` (targeted Codex/Gemini assertions) - diff --git a/docs/native-skills-lean-shard-doc-prototype/phase-4-implementation.md b/docs/native-skills-lean-shard-doc-prototype/phase-4-implementation.md deleted file mode 100644 index f91116b7b..000000000 --- a/docs/native-skills-lean-shard-doc-prototype/phase-4-implementation.md +++ /dev/null @@ -1,32 +0,0 @@ -# Phase 4 Implementation: Native Skills Lean PoC - -Date: 2026-03-07 -Branch: `feature/native-skills-lean-shard-doc-prototype` - -## Story - -As BMAD installer maintainers, we need one duplicated native-skill prototype for shard-doc so we can validate intermediary migration behavior without changing existing task/help surfaces. - -## Tasks - -1. Add prototype ID metadata for `shard-doc.xml`. -2. Extend skill-manifest helper to expose prototype IDs. -3. Update config-driven installer to emit duplicate skill directories for `skill_format` targets only. -4. Add install-component tests: - - Codex (skill-format) writes canonical + prototype shard-doc skills - - Gemini (non-skill) does not write prototype duplicate output -5. Run installer component tests. - -## Verification Plan - -1. `node test/test-installation-components.js` -2. Confirm no edits to legacy `shard-doc.xml` behavior content. -3. Confirm no edits to `src/core/module-help.csv` command/help entries. - -## Done Criteria - -1. Four-phase artifacts exist in docs. -2. Prototype skill duplication works on supported skill-format install path. -3. Legacy shard-doc command/help behavior remains unchanged. -4. Test suite passes with new assertions. - diff --git a/test/test-installation-components.js b/test/test-installation-components.js index 07faab05d..ee99c0136 100644 --- a/test/test-installation-components.js +++ b/test/test-installation-components.js @@ -97,6 +97,8 @@ async function createShardDocPrototypeFixture() { ' canonicalId: bmad-shard-doc', ' prototypeIds:', ' - bmad-shard-doc-skill-prototype', + ' type: task', + ' description: "Splits large markdown documents into smaller, organized files based on sections"', '', ].join('\n'), ); @@ -564,6 +566,9 @@ async function runTests() { // ============================================================ console.log(`${colors.yellow}Test Suite 11: Shard-doc Prototype Duplication${colors.reset}\n`); + let tempCodexProjectDir; + let tempGeminiProjectDir; + let installedBmadDir; try { clearCache(); const platformCodes = await loadPlatformCodes(); @@ -573,9 +578,9 @@ async function runTests() { assert(codexInstaller?.skill_format === true, 'Codex installer uses skill_format output'); assert(geminiInstaller?.skill_format !== true, 'Gemini installer remains non-skill_format'); - const tempCodexProjectDir = await fs.mkdtemp(path.join(os.tmpdir(), 'bmad-codex-prototype-test-')); - const tempGeminiProjectDir = await fs.mkdtemp(path.join(os.tmpdir(), 'bmad-gemini-prototype-test-')); - const installedBmadDir = await createShardDocPrototypeFixture(); + tempCodexProjectDir = await fs.mkdtemp(path.join(os.tmpdir(), 'bmad-codex-prototype-test-')); + tempGeminiProjectDir = await fs.mkdtemp(path.join(os.tmpdir(), 'bmad-gemini-prototype-test-')); + installedBmadDir = await createShardDocPrototypeFixture(); const ideManager = new IdeManager(); await ideManager.ensureInitialized(); @@ -609,11 +614,14 @@ async function runTests() { assert(await fs.pathExists(geminiCanonicalTask), 'Gemini install writes canonical shard-doc command artifact'); assert(!(await fs.pathExists(geminiPrototypeTask)), 'Gemini install does not write duplicated shard-doc prototype artifact'); - await fs.remove(tempCodexProjectDir); - await fs.remove(tempGeminiProjectDir); - await fs.remove(installedBmadDir); } catch (error) { assert(false, 'Shard-doc prototype duplication test succeeds', error.message); + } finally { + await Promise.allSettled( + [tempCodexProjectDir, tempGeminiProjectDir, installedBmadDir] + .filter(Boolean) + .map((dir) => fs.remove(dir)), + ); } console.log(''); diff --git a/tools/cli/installers/lib/ide/_config-driven.js b/tools/cli/installers/lib/ide/_config-driven.js index 30f2c4ae3..285246548 100644 --- a/tools/cli/installers/lib/ide/_config-driven.js +++ b/tools/cli/installers/lib/ide/_config-driven.js @@ -27,6 +27,7 @@ class ConfigDrivenIdeSetup extends BaseIdeSetup { super(platformCode, platformConfig.name, platformConfig.preferred); this.platformConfig = platformConfig; this.installerConfig = platformConfig.installer || null; + this._manifestCache = new Map(); // Set configDir from target_dir so base-class detect() works if (this.installerConfig?.target_dir) { @@ -116,6 +117,7 @@ class ConfigDrivenIdeSetup extends BaseIdeSetup { */ async installToTarget(projectDir, bmadDir, config, options) { const { target_dir, template_type, artifact_types } = config; + this._manifestCache = new Map(); // Skip targets with explicitly empty artifact_types array // This prevents creating empty directories when no artifacts will be written @@ -521,7 +523,11 @@ LOAD and execute from: {project-root}/{{bmadFolderName}}/{{path}} const sourceRef = this.resolveArtifactSourceRef(artifact, bmadDir); if (!sourceRef) return []; - const manifest = await loadSkillManifest(sourceRef.dirPath); + let manifest = this._manifestCache.get(sourceRef.dirPath); + if (manifest === undefined) { + manifest = await loadSkillManifest(sourceRef.dirPath); + this._manifestCache.set(sourceRef.dirPath, manifest); + } return getPrototypeIds(manifest, sourceRef.filename); } @@ -558,15 +564,21 @@ LOAD and execute from: {project-root}/{{bmadFolderName}}/{{path}} } } - // eslint-disable-next-line unicorn/prefer-string-replace-all -- regex replacement is intentional normalized = normalized.replace(/^\/+/, ''); if (!normalized || normalized.startsWith('..')) return null; const filename = path.basename(normalized); if (!filename || filename === '.' || filename === '..') return null; + const resolvedBmadDir = path.resolve(bmadDir); const relativeDir = path.dirname(normalized); - const dirPath = relativeDir === '.' ? bmadDir : path.join(bmadDir, relativeDir); + const dirPath = relativeDir === '.' ? resolvedBmadDir : path.resolve(resolvedBmadDir, relativeDir); + const pathFromBmadRoot = path.relative(resolvedBmadDir, dirPath); + + if (pathFromBmadRoot === '..' || pathFromBmadRoot.startsWith(`..${path.sep}`) || path.isAbsolute(pathFromBmadRoot)) { + return null; + } + return { dirPath, filename }; }