Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 37 additions & 7 deletions src/specify_cli/integrations/hermes/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -121,13 +121,27 @@ def setup(
command_name = src_file.stem # e.g. "plan"
skill_name = f"speckit-{command_name.replace('.', '-')}"

# Parse frontmatter for description
# Parse frontmatter for description. Locate the closing ``---`` on
# its own line rather than with ``raw.split("---", 2)`` — a bare
# substring split stops at the first ``---`` *anywhere*, including
# one inside a value such as ``description: Separate sections
# with ---``, which truncates the frontmatter and drops later keys.
# The block between the delimiters is parsed unstripped so trailing
# newlines in literal (``|``) block scalars survive.
frontmatter: dict[str, Any] = {}
if raw.startswith("---"):
parts = raw.split("---", 2)
if len(parts) >= 3:
fm_lines = raw.splitlines(keepends=True)
fm_close = next(
(
i
for i in range(1, len(fm_lines))
if fm_lines[i].rstrip() == "---"
),
None,
)
if fm_close is not None:
try:
fm = yaml.safe_load(parts[1])
fm = yaml.safe_load("".join(fm_lines[1:fm_close]))
if isinstance(fm, dict):
frontmatter = fm
except yaml.YAMLError:
Expand All @@ -143,10 +157,26 @@ def setup(
project_root=project_root,
)
# Strip the processed frontmatter — we rebuild it for skills.
# Scan for the closing ``---`` on its own line rather than
# ``split("---", 2)`` so a ``---`` embedded in a value does not
# truncate the frontmatter and spill it into the body.
if processed_body.startswith("---"):
parts = processed_body.split("---", 2)
if len(parts) >= 3:
processed_body = parts[2]
body_lines = processed_body.splitlines(keepends=True)
close_idx = next(
(
i
for i in range(1, len(body_lines))
if body_lines[i].rstrip() == "---"
),
None,
)
if close_idx is not None:
# Keep whatever trails the ``---`` marker on the closing
# line so the body stays byte-for-byte identical to
# ``split("---", 2)[2]`` for well-formed templates.
processed_body = body_lines[close_idx][3:] + "".join(
body_lines[close_idx + 1 :]
)

# Select description
description = frontmatter.get("description", "")
Expand Down
16 changes: 13 additions & 3 deletions src/specify_cli/integrations/kimi/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -323,14 +323,24 @@ def _is_speckit_generated_skill(skill_dir: Path) -> bool:
if not content.startswith("---"):
return False

parts = content.split("---", 2)
if len(parts) < 3:
# Locate the closing ``---`` on its own line rather than with
# ``content.split("---", 2)`` — a bare substring split stops at the first
# ``---`` *anywhere*, including one inside a value such as
# ``description: Separate sections with ---``, which truncates the parsed
# frontmatter and can drop the metadata block this check relies on (so a
# Speckit-generated skill would not be recognized on teardown).
lines = content.splitlines(keepends=True)
close_idx = next(
(i for i in range(1, len(lines)) if lines[i].rstrip() == "---"),
None,
)
if close_idx is None:
return False

try:
import yaml

frontmatter = yaml.safe_load(parts[1])
frontmatter = yaml.safe_load("".join(lines[1:close_idx]))
except Exception:
return False

Expand Down
63 changes: 63 additions & 0 deletions tests/integrations/test_skill_frontmatter_quoting.py
Original file line number Diff line number Diff line change
Expand Up @@ -178,3 +178,66 @@ def test_multiline_description_survives(self, tmp_path, monkeypatch):

fm = _parse_frontmatter(skill_files[0])
assert fm["description"] == MULTILINE

def test_dashed_description_is_preserved(self, tmp_path, monkeypatch):
"""Hermes overrides setup(), so it needs the same line-anchored parse."""
home = tmp_path / "home"
home.mkdir(exist_ok=True)
monkeypatch.setattr(Path, "home", lambda: home)

integration = get_integration("hermes")
monkeypatch.setattr(
integration,
"shared_commands_dir",
lambda: _fake_templates(tmp_path, DASHED_TEMPLATE),
)
manifest = IntegrationManifest("hermes", tmp_path)
created = integration.setup(tmp_path, manifest)
skill_files = [f for f in created if f.name == "SKILL.md"]
assert len(skill_files) == 1

fm = _parse_frontmatter_line_anchored(skill_files[0])
assert fm["description"] == DASHED_DESCRIPTION

content = skill_files[0].read_text(encoding="utf-8")
lines = content.splitlines(keepends=True)
end = next(i for i in range(1, len(lines)) if lines[i].rstrip() == "---")
body = "".join(lines[end + 1 :])
assert "name-marker: sentinel" not in body


class TestKimiGeneratedSkillDetection:
"""``_is_speckit_generated_skill`` must survive a ``---`` in a value.

Teardown only removes a legacy skill directory it recognizes as
Speckit-generated via the frontmatter ``metadata`` block. A substring split
truncated the frontmatter before ``metadata`` when a description embedded
``---``, so the directory was left behind on uninstall.
"""

def _write_skill(self, skill_dir: Path, description: str) -> None:
skill_dir.mkdir(parents=True, exist_ok=True)
(skill_dir / "SKILL.md").write_text(
"---\n"
'name: "speckit-plan"\n'
f"description: {description}\n"
"metadata:\n"
' author: "github-spec-kit"\n'
' source: "templates/commands/plan.md"\n'
"---\n\nBody.\n",
encoding="utf-8",
)

def test_detects_skill_with_dashes_in_description(self, tmp_path):
from specify_cli.integrations.kimi import _is_speckit_generated_skill

skill_dir = tmp_path / "speckit-plan"
self._write_skill(skill_dir, "Separate sections with --- markers")
assert _is_speckit_generated_skill(skill_dir) is True

def test_still_detects_plain_description(self, tmp_path):
from specify_cli.integrations.kimi import _is_speckit_generated_skill

skill_dir = tmp_path / "speckit-plan"
self._write_skill(skill_dir, "Plain description")
assert _is_speckit_generated_skill(skill_dir) is True