fix(installer): trim shard-doc prototype PR and address review feedback
This commit is contained in:
parent
a036381d84
commit
86556edfcc
|
|
@ -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
|
|
||||||
|
|
||||||
|
|
@ -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.
|
|
||||||
|
|
||||||
|
|
@ -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-name>/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)
|
|
||||||
|
|
||||||
|
|
@ -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.
|
|
||||||
|
|
||||||
|
|
@ -97,6 +97,8 @@ async function createShardDocPrototypeFixture() {
|
||||||
' canonicalId: bmad-shard-doc',
|
' canonicalId: bmad-shard-doc',
|
||||||
' prototypeIds:',
|
' prototypeIds:',
|
||||||
' - bmad-shard-doc-skill-prototype',
|
' - bmad-shard-doc-skill-prototype',
|
||||||
|
' type: task',
|
||||||
|
' description: "Splits large markdown documents into smaller, organized files based on sections"',
|
||||||
'',
|
'',
|
||||||
].join('\n'),
|
].join('\n'),
|
||||||
);
|
);
|
||||||
|
|
@ -564,6 +566,9 @@ async function runTests() {
|
||||||
// ============================================================
|
// ============================================================
|
||||||
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 tempGeminiProjectDir;
|
||||||
|
let installedBmadDir;
|
||||||
try {
|
try {
|
||||||
clearCache();
|
clearCache();
|
||||||
const platformCodes = await loadPlatformCodes();
|
const platformCodes = await loadPlatformCodes();
|
||||||
|
|
@ -573,9 +578,9 @@ async function runTests() {
|
||||||
assert(codexInstaller?.skill_format === true, 'Codex installer uses skill_format output');
|
assert(codexInstaller?.skill_format === true, 'Codex installer uses skill_format output');
|
||||||
assert(geminiInstaller?.skill_format !== true, 'Gemini installer remains non-skill_format');
|
assert(geminiInstaller?.skill_format !== true, 'Gemini installer remains non-skill_format');
|
||||||
|
|
||||||
const tempCodexProjectDir = await fs.mkdtemp(path.join(os.tmpdir(), 'bmad-codex-prototype-test-'));
|
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-'));
|
tempGeminiProjectDir = await fs.mkdtemp(path.join(os.tmpdir(), 'bmad-gemini-prototype-test-'));
|
||||||
const installedBmadDir = await createShardDocPrototypeFixture();
|
installedBmadDir = await createShardDocPrototypeFixture();
|
||||||
|
|
||||||
const ideManager = new IdeManager();
|
const ideManager = new IdeManager();
|
||||||
await ideManager.ensureInitialized();
|
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(geminiCanonicalTask), 'Gemini install writes canonical shard-doc command artifact');
|
||||||
assert(!(await fs.pathExists(geminiPrototypeTask)), 'Gemini install does not write duplicated shard-doc prototype 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) {
|
} catch (error) {
|
||||||
assert(false, 'Shard-doc prototype duplication test succeeds', error.message);
|
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('');
|
console.log('');
|
||||||
|
|
|
||||||
|
|
@ -27,6 +27,7 @@ class ConfigDrivenIdeSetup extends BaseIdeSetup {
|
||||||
super(platformCode, platformConfig.name, platformConfig.preferred);
|
super(platformCode, platformConfig.name, platformConfig.preferred);
|
||||||
this.platformConfig = platformConfig;
|
this.platformConfig = platformConfig;
|
||||||
this.installerConfig = platformConfig.installer || null;
|
this.installerConfig = platformConfig.installer || null;
|
||||||
|
this._manifestCache = new Map();
|
||||||
|
|
||||||
// Set configDir from target_dir so base-class detect() works
|
// Set configDir from target_dir so base-class detect() works
|
||||||
if (this.installerConfig?.target_dir) {
|
if (this.installerConfig?.target_dir) {
|
||||||
|
|
@ -116,6 +117,7 @@ class ConfigDrivenIdeSetup extends BaseIdeSetup {
|
||||||
*/
|
*/
|
||||||
async installToTarget(projectDir, bmadDir, config, options) {
|
async installToTarget(projectDir, bmadDir, config, options) {
|
||||||
const { target_dir, template_type, artifact_types } = config;
|
const { target_dir, template_type, artifact_types } = config;
|
||||||
|
this._manifestCache = new Map();
|
||||||
|
|
||||||
// Skip targets with explicitly empty artifact_types array
|
// Skip targets with explicitly empty artifact_types array
|
||||||
// This prevents creating empty directories when no artifacts will be written
|
// 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);
|
const sourceRef = this.resolveArtifactSourceRef(artifact, bmadDir);
|
||||||
if (!sourceRef) return [];
|
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);
|
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(/^\/+/, '');
|
normalized = normalized.replace(/^\/+/, '');
|
||||||
if (!normalized || normalized.startsWith('..')) return null;
|
if (!normalized || normalized.startsWith('..')) return null;
|
||||||
|
|
||||||
const filename = path.basename(normalized);
|
const filename = path.basename(normalized);
|
||||||
if (!filename || filename === '.' || filename === '..') return null;
|
if (!filename || filename === '.' || filename === '..') return null;
|
||||||
|
|
||||||
|
const resolvedBmadDir = path.resolve(bmadDir);
|
||||||
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 pathFromBmadRoot = path.relative(resolvedBmadDir, dirPath);
|
||||||
|
|
||||||
|
if (pathFromBmadRoot === '..' || pathFromBmadRoot.startsWith(`..${path.sep}`) || path.isAbsolute(pathFromBmadRoot)) {
|
||||||
|
return null;
|
||||||
|
}
|
||||||
|
|
||||||
return { dirPath, filename };
|
return { dirPath, filename };
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue