fix(validate-module): harden standalone module validation (#97)

* fix(validate-module): harden standalone module validation

Three robustness fixes to validate-module.py for standalone
single-skill modules:

- Accept importable underscore-named merge scripts (merge_config.py)
  in addition to the scaffolder's dash form (merge-config.py). Either
  is valid; a module may rename them to be importable from
  module-setup.md without that being a structural defect.
- Treat colon-less preceded-by/followed-by refs as cross-module
  positional references (bare sibling-module skill names) and skip
  them. Other installed modules aren't visible to the validator, so
  they can't be resolved and aren't defects; intra-module
  skill:action refs are still validated.
- Detect a standalone module when handed the skill directory itself,
  not only its parent folder (as the reference doc already promised).
  Skill-folder and orphan-entry checks now resolve in both cases.

Adds 3 regression tests (20 pass total); updates the reference doc.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(validate-module): scope standalone orphan check to the module

Addresses CodeRabbit review on #97: in direct skill-dir mode the
orphan check resolved CSV skills against the parent folder, so an
unrelated sibling skill directory could mask a true orphan entry.

For a standalone module, skill_folders already enumerates every valid
skill (just the standalone skill), so any other CSV skill is an orphan
— drop the parent-folder filesystem lookup entirely. The multi-skill
path still re-checks the filesystem for the setup skill.

Adds a regression test (sibling dir must not mask the orphan); 21 tests
pass.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* test(validate-module): assert exit code in orphan regression test

Addresses CodeRabbit nit on #97: the orphan regression test unpacked
`code` without using it. Assert `code == 1` (the orphan makes
validation fail), which also strengthens the test.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Author: armelhbobdad

skills/bmad-module-builder/references/validate-module.md

@@ -13,7 +13,7 @@ You are a module quality reviewer. Your job is to verify that a BMad module's st
 Ask the user for the path to their module's skills folder (or a single skill folder for standalone modules). The validation script auto-detects the module type:
 
 - **Multi-skill module:** Identifies the setup skill (`*-setup`) and all other skill folders
-- **Standalone module:** Detected when no setup skill exists and the folder contains a single skill with `assets/module.yaml`. Validates: `assets/module-setup.md`, `assets/module.yaml`, `assets/module-help.csv`, `scripts/merge-config.py`, `scripts/merge-help-csv.py`
+- **Standalone module:** Detected when no setup skill exists and either the folder contains a single skill with `assets/module.yaml`, or the path given _is_ the standalone skill directory itself. Validates: `assets/module-setup.md`, `assets/module.yaml`, `assets/module-help.csv`, and the two merge scripts — either the scaffolder's dash form (`scripts/merge-config.py`, `scripts/merge-help-csv.py`) or the importable underscore form (`scripts/merge_config.py`, `scripts/merge_help_csv.py`) is accepted.
 
 ### 2. Run Structural Validation
 

skills/bmad-module-builder/scripts/tests/test-validate-module.py

@@ -235,8 +235,14 @@ def test_short_row_does_not_crash():
 def create_standalone_module(tmp: Path, skill_name: str = "my-skill",
                              csv_rows: str = "", yaml_content: str = "",
                              include_setup_md: bool = True,
-                             include_merge_scripts: bool = True) -> Path:
-    """Create a minimal standalone module structure for testing."""
+                             include_merge_scripts: bool = True,
+                             merge_script_style: str = "dash") -> Path:
+    """Create a minimal standalone module structure for testing.
+
+    ``merge_script_style`` selects the merge-script naming form: "dash" for the
+    scaffolder default (merge-config.py) or "underscore" for the importable form
+    (merge_config.py). Both are valid.
+    """
     module_dir = tmp / "module"
     module_dir.mkdir()
 
@@ -259,8 +265,12 @@ def create_standalone_module(tmp: Path, skill_name: str = "my-skill",
     if include_merge_scripts:
         scripts = skill / "scripts"
         scripts.mkdir()
-        (scripts / "merge-config.py").write_text("# merge-config\n")
-        (scripts / "merge-help-csv.py").write_text("# merge-help-csv\n")
+        if merge_script_style == "underscore":
+            (scripts / "merge_config.py").write_text("# merge_config\n")
+            (scripts / "merge_help_csv.py").write_text("# merge_help_csv\n")
+        else:
+            (scripts / "merge-config.py").write_text("# merge-config\n")
+            (scripts / "merge-help-csv.py").write_text("# merge-help-csv\n")
 
     return module_dir
 
@@ -319,6 +329,70 @@ def test_standalone_csv_validation():
         assert "DT" in dupes[0]["message"]
 
 
+def test_standalone_underscore_merge_scripts():
+    """Importable underscore-named merge scripts (merge_config.py) should pass."""
+    with tempfile.TemporaryDirectory() as tmp:
+        tmp = Path(tmp)
+        module_dir = create_standalone_module(tmp, merge_script_style="underscore")
+
+        code, data = run_validate(module_dir)
+        assert code == 0, f"Expected pass: {data}"
+        assert data["status"] == "pass"
+        assert data["info"].get("standalone") is True
+        assert data["summary"]["total_findings"] == 0
+
+
+def test_standalone_cross_module_before_after_ref():
+    """Bare (colon-less) preceded-by/followed-by refs are cross-module positional, not flagged."""
+    with tempfile.TemporaryDirectory() as tmp:
+        tmp = Path(tmp)
+        csv_rows = ('Test Module,my-skill,Do Thing,DT,Does thing,,,anytime,'
+                    'bmad-sprint-planning,bmad-retrospective,false,output_folder,artifact\n')
+        module_dir = create_standalone_module(tmp, csv_rows=csv_rows)
+
+        code, data = run_validate(module_dir)
+        assert code == 0, f"Expected pass: {data}"
+        refs = [f for f in data["findings"] if f["category"] == "invalid-ref"]
+        assert refs == [], f"Cross-module bare refs should not be flagged: {refs}"
+
+
+def test_standalone_given_skill_dir_directly():
+    """Passing the standalone skill directory itself (not its parent) should work."""
+    with tempfile.TemporaryDirectory() as tmp:
+        tmp = Path(tmp)
+        module_dir = create_standalone_module(tmp, skill_name="my-skill")
+        skill_dir = module_dir / "my-skill"
+
+        code, data = run_validate(skill_dir)
+        assert code == 0, f"Expected pass: {data}"
+        assert data["status"] == "pass"
+        assert data["info"].get("standalone") is True
+        assert data["info"].get("skill_dir") == "my-skill"
+
+
+def test_standalone_skill_dir_orphan_not_masked_by_sibling():
+    """Validating a skill dir directly must still flag a CSV skill that only
+    exists as an unrelated sibling directory (not part of this standalone module)."""
+    with tempfile.TemporaryDirectory() as tmp:
+        tmp = Path(tmp)
+        csv_rows = (
+            'Test Module,my-skill,Do Thing,DT,Does thing,run,,anytime,,,false,output_folder,artifact\n'
+            'Test Module,other-skill,Other,OT,Other thing,run,,anytime,,,false,output_folder,report\n'
+        )
+        module_dir = create_standalone_module(tmp, skill_name="my-skill", csv_rows=csv_rows)
+        # A sibling skill dir next to the standalone skill (a different module).
+        sibling = module_dir / "other-skill"
+        sibling.mkdir()
+        (sibling / "SKILL.md").write_text("---\nname: other-skill\n---\n# other-skill\n")
+
+        skill_dir = module_dir / "my-skill"
+        code, data = run_validate(skill_dir)
+        assert code == 1, f"Orphan entry should fail validation: {data}"
+        orphans = [f for f in data["findings"] if f["category"] == "orphan-entry"]
+        assert any("other-skill" in f["message"] for f in orphans), \
+            f"Sibling skill must not mask the orphan: {data['findings']}"
+
+
 def test_multi_skill_not_detected_as_standalone():
     """A folder with two skills and no setup skill should fail (not detected as standalone)."""
     with tempfile.TemporaryDirectory() as tmp:
@@ -367,6 +441,10 @@ def test_nonexistent_directory():
         test_standalone_missing_module_setup_md,
         test_standalone_missing_merge_scripts,
         test_standalone_csv_validation,
+        test_standalone_underscore_merge_scripts,
+        test_standalone_cross_module_before_after_ref,
+        test_standalone_given_skill_dir_directly,
+        test_standalone_skill_dir_orphan_not_masked_by_sibling,
         test_multi_skill_not_detected_as_standalone,
         test_nonexistent_directory,
     ]

skills/bmad-module-builder/scripts/validate-module.py

@@ -51,7 +51,16 @@ def find_skill_folders(module_dir: Path, exclude_name: str = "") -> list[str]:
 
 
 def detect_standalone_module(module_dir: Path) -> Path | None:
-    """Detect a standalone module: single skill folder with assets/module.yaml."""
+    """Detect a standalone module: a single skill folder with assets/module.yaml.
+
+    Works whether ``module_dir`` is the parent folder that contains the skill, or
+    the standalone skill directory itself (the path a user is most likely to hand
+    over for a single-skill module).
+    """
+    # Given the skill directory directly.
+    if (module_dir / "SKILL.md").is_file() and (module_dir / "assets" / "module.yaml").is_file():
+        return module_dir
+    # Given the parent folder containing exactly one skill.
     skill_dirs = [
         d for d in module_dir.iterdir()
         if d.is_dir() and (d / "SKILL.md").is_file()
@@ -130,14 +139,32 @@ def finding(severity: str, category: str, message: str, detail: str = ""):
             "assets/module.yaml": skill_dir / "assets" / "module.yaml",
             "assets/module-help.csv": skill_dir / "assets" / "module-help.csv",
             "assets/module-setup.md": skill_dir / "assets" / "module-setup.md",
-            "scripts/merge-config.py": skill_dir / "scripts" / "merge-config.py",
-            "scripts/merge-help-csv.py": skill_dir / "scripts" / "merge-help-csv.py",
         }
+        # Merge scripts: accept either the dash form the scaffolder emits
+        # (merge-config.py) or the importable underscore form (merge_config.py).
+        # Both are valid — a module may rename them to be importable from
+        # module-setup.md without that being a structural defect.
+        required_any = {
+            "scripts/merge-config.py (or merge_config.py)": [
+                skill_dir / "scripts" / "merge-config.py",
+                skill_dir / "scripts" / "merge_config.py",
+            ],
+            "scripts/merge-help-csv.py (or merge_help_csv.py)": [
+                skill_dir / "scripts" / "merge-help-csv.py",
+                skill_dir / "scripts" / "merge_help_csv.py",
+            ],
+        }
+        ok = True
         for label, path in required_files.items():
             if not path.is_file():
                 finding("critical", "structure", f"Missing required file: {label}")
+                ok = False
+        for label, candidates in required_any.items():
+            if not any(p.is_file() for p in candidates):
+                finding("critical", "structure", f"Missing required file: {label}")
+                ok = False
 
-        if not all(p.is_file() for p in required_files.values()):
+        if not ok:
             return {"status": "fail", "findings": findings, "info": info}
 
         yaml_dir = skill_dir
@@ -202,8 +229,12 @@ def finding(severity: str, category: str, message: str, detail: str = ""):
 
     # 6. Collect skills from CSV and filesystem
     csv_skills = {row.get("skill", "") for row in rows}
-    exclude_name = setup_dir.name if setup_dir else ""
-    skill_folders = find_skill_folders(module_dir, exclude_name)
+    if standalone_dir:
+        # The only valid skill is the standalone skill itself, whether we were
+        # handed the module's parent folder or the skill directory directly.
+        skill_folders = [standalone_dir.name]
+    else:
+        skill_folders = find_skill_folders(module_dir, setup_dir.name)
     info["skill_folders"] = skill_folders
     info["csv_skills"] = sorted(csv_skills)
 
@@ -215,10 +246,15 @@ def finding(severity: str, category: str, message: str, detail: str = ""):
     # 8. Orphan CSV entries
     setup_name = setup_dir.name if setup_dir else ""
     for skill in csv_skills:
-        if skill not in skill_folders and skill != setup_name:
-            # Check if it's the setup skill itself (valid)
-            if not (module_dir / skill / "SKILL.md").is_file():
-                finding("high", "orphan-entry", f"CSV references skill '{skill}' which does not exist in the module folder")
+        if skill in skill_folders or skill == setup_name:
+            continue
+        # For a standalone module, skill_folders already enumerates every valid
+        # skill, so any other CSV skill is an orphan — never look at the parent
+        # folder (which may hold unrelated sibling skills when validating a skill
+        # dir directly). For a multi-skill module, re-check the filesystem: the
+        # setup skill lives alongside the others and is excluded from skill_folders.
+        if standalone_dir or not (module_dir / skill / "SKILL.md").is_file():
+            finding("high", "orphan-entry", f"CSV references skill '{skill}' which does not exist in the module folder")
 
     # 9. Unique menu codes
     menu_codes: dict[str, list[str]] = {}
@@ -249,7 +285,15 @@ def finding(severity: str, category: str, message: str, detail: str = ""):
             # Can be comma-separated
             for ref in value.split(","):
                 ref = ref.strip()
-                if ref and ref not in valid_refs:
+                if not ref:
+                    continue
+                # A colon-less ref is a cross-module positional reference (a bare
+                # sibling-module skill name, e.g. "bmad-sprint-planning"). Other
+                # installed modules aren't visible here, so it can't be resolved
+                # and isn't a defect — only validate intra-module skill:action refs.
+                if ":" not in ref:
+                    continue
+                if ref not in valid_refs:
                     finding("medium", "invalid-ref",
                             f"'{display}' {field} references '{ref}' which is not a valid capability",
                             "Expected format: skill-name:action-name")