Skip to content

Commit c9ffcc1

Browse files
committed
fix: address eleventh round of Copilot PR review feedback
- Validate strategy is a string before set membership check: raise PresetValidationError for non-string values (null, list, etc.) and normalize to lowercase for case-insensitive matching - Fix core command name mapping: add speckit.<stem> → <stem>.md fallback in resolve(), resolve_with_source(), and collect_all_layers() so core command files (named by stem, e.g. plan.md) are found when commands use dot notation (e.g. speckit.plan) - Fix test fixture to use real core layout (plan.md not speckit.plan.md)
1 parent 454e614 commit c9ffcc1

2 files changed

Lines changed: 34 additions & 2 deletions

File tree

src/specify_cli/presets.py

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -156,6 +156,12 @@ def _validate(self):
156156

157157
# Validate strategy field (optional, defaults to "replace")
158158
strategy = tmpl.get("strategy", "replace")
159+
if not isinstance(strategy, str):
160+
raise PresetValidationError(
161+
f"Invalid strategy value: must be a string, "
162+
f"got {type(strategy).__name__}"
163+
)
164+
strategy = strategy.lower()
159165
if strategy not in VALID_PRESET_STRATEGIES:
160166
raise PresetValidationError(
161167
f"Invalid strategy '{strategy}': "
@@ -2037,6 +2043,19 @@ def _get_all_extensions_by_priority(self) -> list[tuple[int, str, dict | None]]:
20372043
all_extensions.sort(key=lambda x: (x[0], x[1]))
20382044
return all_extensions
20392045

2046+
@staticmethod
2047+
def _core_stem(template_name: str) -> Optional[str]:
2048+
"""Extract the stem for core command lookup.
2049+
2050+
Commands use dot notation (e.g. ``speckit.specify``), but core
2051+
command files are named by stem (e.g. ``specify.md``). Returns
2052+
the stem if *template_name* follows the ``speckit.<stem>`` pattern,
2053+
or ``None`` otherwise.
2054+
"""
2055+
if template_name.startswith("speckit."):
2056+
return template_name[len("speckit."):]
2057+
return None
2058+
20402059
def resolve(
20412060
self,
20422061
template_name: str,
@@ -2111,6 +2130,12 @@ def resolve(
21112130
core = self.templates_dir / "commands" / f"{template_name}.md"
21122131
if core.exists():
21132132
return core
2133+
# Fallback: speckit.<stem> → <stem>.md
2134+
stem = self._core_stem(template_name)
2135+
if stem:
2136+
core = self.templates_dir / "commands" / f"{stem}.md"
2137+
if core.exists():
2138+
return core
21142139
elif template_type == "script":
21152140
core = self.templates_dir / "scripts" / f"{template_name}{ext}"
21162141
if core.exists():
@@ -2300,6 +2325,13 @@ def _find_in_subdirs(base_dir: Path) -> Optional[Path]:
23002325
c = self.templates_dir / "commands" / f"{template_name}.md"
23012326
if c.exists():
23022327
core = c
2328+
else:
2329+
# Fallback: speckit.<stem> → <stem>.md
2330+
stem = self._core_stem(template_name)
2331+
if stem:
2332+
c = self.templates_dir / "commands" / f"{stem}.md"
2333+
if c.exists():
2334+
core = c
23032335
elif template_type == "script":
23042336
c = self.templates_dir / "scripts" / f"{template_name}{ext}"
23052337
if c.exists():

tests/test_presets.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3429,10 +3429,10 @@ def test_resolve_content_override_trumps_composition(self, project_dir, temp_dir
34293429

34303430
def test_resolve_content_command_type(self, project_dir, temp_dir, valid_pack_data):
34313431
"""Test resolve_content with command template type."""
3432-
# Create core command
3432+
# Create core command using stem naming (matches real layout: plan.md, not speckit.plan.md)
34333433
commands_dir = project_dir / ".specify" / "templates" / "commands"
34343434
commands_dir.mkdir(parents=True, exist_ok=True)
3435-
(commands_dir / "speckit.plan.md").write_text("# Core Plan Command\n")
3435+
(commands_dir / "plan.md").write_text("# Core Plan Command\n")
34363436

34373437
pack_data = {**valid_pack_data}
34383438
pack_data["preset"] = {**valid_pack_data["preset"], "id": "cmd-append", "name": "CmdAppend"}

0 commit comments

Comments
 (0)