Skip to content

Commit cbc76e6

Browse files
fix(skills): apply the line-anchored delimiter scan to hermes and kimi
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.
1 parent 71125fc commit cbc76e6

3 files changed

Lines changed: 113 additions & 10 deletions

File tree

‎src/specify_cli/integrations/hermes/__init__.py‎

Lines changed: 37 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -121,13 +121,27 @@ def setup(
121121
command_name = src_file.stem # e.g. "plan"
122122
skill_name = f"speckit-{command_name.replace('.', '-')}"
123123

124-
# Parse frontmatter for description
124+
# Parse frontmatter for description. Locate the closing ``---`` on
125+
# its own line rather than with ``raw.split("---", 2)`` — a bare
126+
# substring split stops at the first ``---`` *anywhere*, including
127+
# one inside a value such as ``description: Separate sections
128+
# with ---``, which truncates the frontmatter and drops later keys.
129+
# The block between the delimiters is parsed unstripped so trailing
130+
# newlines in literal (``|``) block scalars survive.
125131
frontmatter: dict[str, Any] = {}
126132
if raw.startswith("---"):
127-
parts = raw.split("---", 2)
128-
if len(parts) >= 3:
133+
fm_lines = raw.splitlines(keepends=True)
134+
fm_close = next(
135+
(
136+
i
137+
for i in range(1, len(fm_lines))
138+
if fm_lines[i].rstrip() == "---"
139+
),
140+
None,
141+
)
142+
if fm_close is not None:
129143
try:
130-
fm = yaml.safe_load(parts[1])
144+
fm = yaml.safe_load("".join(fm_lines[1:fm_close]))
131145
if isinstance(fm, dict):
132146
frontmatter = fm
133147
except yaml.YAMLError:
@@ -143,10 +157,26 @@ def setup(
143157
project_root=project_root,
144158
)
145159
# Strip the processed frontmatter — we rebuild it for skills.
160+
# Scan for the closing ``---`` on its own line rather than
161+
# ``split("---", 2)`` so a ``---`` embedded in a value does not
162+
# truncate the frontmatter and spill it into the body.
146163
if processed_body.startswith("---"):
147-
parts = processed_body.split("---", 2)
148-
if len(parts) >= 3:
149-
processed_body = parts[2]
164+
body_lines = processed_body.splitlines(keepends=True)
165+
close_idx = next(
166+
(
167+
i
168+
for i in range(1, len(body_lines))
169+
if body_lines[i].rstrip() == "---"
170+
),
171+
None,
172+
)
173+
if close_idx is not None:
174+
# Keep whatever trails the ``---`` marker on the closing
175+
# line so the body stays byte-for-byte identical to
176+
# ``split("---", 2)[2]`` for well-formed templates.
177+
processed_body = body_lines[close_idx][3:] + "".join(
178+
body_lines[close_idx + 1 :]
179+
)
150180

151181
# Select description
152182
description = frontmatter.get("description", "")

‎src/specify_cli/integrations/kimi/__init__.py‎

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -323,14 +323,24 @@ def _is_speckit_generated_skill(skill_dir: Path) -> bool:
323323
if not content.startswith("---"):
324324
return False
325325

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

330340
try:
331341
import yaml
332342

333-
frontmatter = yaml.safe_load(parts[1])
343+
frontmatter = yaml.safe_load("".join(lines[1:close_idx]))
334344
except Exception:
335345
return False
336346

‎tests/integrations/test_skill_frontmatter_quoting.py‎

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -178,3 +178,66 @@ def test_multiline_description_survives(self, tmp_path, monkeypatch):
178178

179179
fm = _parse_frontmatter(skill_files[0])
180180
assert fm["description"] == MULTILINE
181+
182+
def test_dashed_description_is_preserved(self, tmp_path, monkeypatch):
183+
"""Hermes overrides setup(), so it needs the same line-anchored parse."""
184+
home = tmp_path / "home"
185+
home.mkdir(exist_ok=True)
186+
monkeypatch.setattr(Path, "home", lambda: home)
187+
188+
integration = get_integration("hermes")
189+
monkeypatch.setattr(
190+
integration,
191+
"shared_commands_dir",
192+
lambda: _fake_templates(tmp_path, DASHED_TEMPLATE),
193+
)
194+
manifest = IntegrationManifest("hermes", tmp_path)
195+
created = integration.setup(tmp_path, manifest)
196+
skill_files = [f for f in created if f.name == "SKILL.md"]
197+
assert len(skill_files) == 1
198+
199+
fm = _parse_frontmatter_line_anchored(skill_files[0])
200+
assert fm["description"] == DASHED_DESCRIPTION
201+
202+
content = skill_files[0].read_text(encoding="utf-8")
203+
lines = content.splitlines(keepends=True)
204+
end = next(i for i in range(1, len(lines)) if lines[i].rstrip() == "---")
205+
body = "".join(lines[end + 1 :])
206+
assert "name-marker: sentinel" not in body
207+
208+
209+
class TestKimiGeneratedSkillDetection:
210+
"""``_is_speckit_generated_skill`` must survive a ``---`` in a value.
211+
212+
Teardown only removes a legacy skill directory it recognizes as
213+
Speckit-generated via the frontmatter ``metadata`` block. A substring split
214+
truncated the frontmatter before ``metadata`` when a description embedded
215+
``---``, so the directory was left behind on uninstall.
216+
"""
217+
218+
def _write_skill(self, skill_dir: Path, description: str) -> None:
219+
skill_dir.mkdir(parents=True, exist_ok=True)
220+
(skill_dir / "SKILL.md").write_text(
221+
"---\n"
222+
'name: "speckit-plan"\n'
223+
f"description: {description}\n"
224+
"metadata:\n"
225+
' author: "github-spec-kit"\n'
226+
' source: "templates/commands/plan.md"\n'
227+
"---\n\nBody.\n",
228+
encoding="utf-8",
229+
)
230+
231+
def test_detects_skill_with_dashes_in_description(self, tmp_path):
232+
from specify_cli.integrations.kimi import _is_speckit_generated_skill
233+
234+
skill_dir = tmp_path / "speckit-plan"
235+
self._write_skill(skill_dir, "Separate sections with --- markers")
236+
assert _is_speckit_generated_skill(skill_dir) is True
237+
238+
def test_still_detects_plain_description(self, tmp_path):
239+
from specify_cli.integrations.kimi import _is_speckit_generated_skill
240+
241+
skill_dir = tmp_path / "speckit-plan"
242+
self._write_skill(skill_dir, "Plain description")
243+
assert _is_speckit_generated_skill(skill_dir) is True

0 commit comments

Comments
 (0)