fix(skills): apply the line-anchored delimiter scan to hermes and kimi (#3739)

Hermes overrides SkillsIntegration.setup() with its own copy of the
frontmatter parse and body strip, and Kimi's _is_speckit_generated_skill()
parses frontmatter independently, so all three carried the same
split("---", 2) bug the base class just fixed. A description such as
"Separate sections with --- markers" truncates the parsed frontmatter at the
embedded marker, dropping later keys and spilling the remainder into the
body; for Kimi that means a Speckit-generated skill is no longer recognized
on teardown and gets left behind.

Scan for a closing "---" on its own line instead. The body slice keeps
whatever trails the marker so output stays byte-for-byte identical for
well-formed templates.
This commit is contained in:
Quratulain-bilal
2026-08-05 11:07:10 -05:00
committed by GitHub
parent e9710ae45e
commit f8a448f0a9
3 changed files with 113 additions and 10 deletions
@@ -121,13 +121,27 @@ class HermesIntegration(SkillsIntegration):
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:
@@ -143,10 +157,26 @@ class HermesIntegration(SkillsIntegration):
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", "")
+13 -3
View File
@@ -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
@@ -178,3 +178,66 @@ class TestHermesSkillFrontmatterQuoting:
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