Skip to content

Commit a52c2aa

Browse files
fix(integrations): stop frontmatter injection gluing onto a missing trailing newline (#4570)
* fix(integrations): stop frontmatter injection gluing onto a missing trailing newline ClaudeIntegration/VibeIntegration/AlquimiaAIIntegration's _inject_frontmatter_flag() detected the closing "---" line's existing EOL and reused it when injecting a new key -- so when that "---" was the file's last line with no trailing newline, the injected text was appended with no newline at all, producing "user-invocable: true---" instead of a properly separated line. This corrupts the frontmatter (the closing delimiter is no longer alone on its own line) and, since post_process_skill_content() chains multiple injection calls, silently drops every subsequent key: a second call's pre-scan can no longer find a second "---" line to inject before, so e.g. "disable-model-invocation: false" is never added at all. post_process_skill_content() runs on content from "external skill generators (presets, extensions)" per its own docstring, so a trailing newline after the closing delimiter isn't guaranteed. DroidIntegration's own copy of this helper already emits an unconditional "\n" instead of detecting/reusing the existing EOL, exactly avoiding this bug -- ported that fix to the other three implementations. Added a regression test to each of the three affected integrations' test files, covering both the single-call corruption and the chained-calls silent-drop. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01U74yBbvVQCPwB7Ed8Dzeu6 * fix(integrations): preserve existing EOL when injecting frontmatter flags Per Copilot review on PR #4570: unconditionally emitting "\n" after the injected key regressed CRLF-authored skills into mixed line endings. Detect the closing delimiter's existing EOL (\r\n or \n) and only fall back to "\n" when the delimiter has none at all (the original no-trailing-newline corruption bug). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
1 parent 6a8870f commit a52c2aa

6 files changed

Lines changed: 174 additions & 6 deletions

File tree

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

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -152,7 +152,13 @@ def _inject_frontmatter_flag(content: str, key: str, value: str = "true") -> str
152152
if dash_count == 1 and stripped.startswith(f"{key}:"):
153153
return content
154154

155-
# Inject before the closing --- of frontmatter
155+
# Inject before the closing --- of frontmatter. Preserve the
156+
# existing EOL style, but default to "\n" (rather than "") when the
157+
# closing delimiter is the last line of the file with no trailing
158+
# newline -- otherwise the injected text glues onto the "---"
159+
# (e.g. "user-invocable: true---"), destroying the delimiter so a
160+
# later call's pre-scan/injection never finds a second "---" and
161+
# silently drops that key entirely.
156162
out: list[str] = []
157163
dash_count = 0
158164
injected = False
@@ -166,7 +172,7 @@ def _inject_frontmatter_flag(content: str, key: str, value: str = "true") -> str
166172
elif line.endswith("\n"):
167173
eol = "\n"
168174
else:
169-
eol = ""
175+
eol = "\n"
170176
out.append(f"{key}: {value}{eol}")
171177
injected = True
172178
out.append(line)

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

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -173,7 +173,13 @@ def _inject_frontmatter_flag(content: str, key: str, value: str = "true") -> str
173173
if dash_count == 1 and stripped.startswith(f"{key}:"):
174174
return content
175175

176-
# Inject before the closing --- of frontmatter
176+
# Inject before the closing --- of frontmatter. Preserve the
177+
# existing EOL style, but default to "\n" (rather than "") when the
178+
# closing delimiter is the last line of the file with no trailing
179+
# newline -- otherwise the injected text glues onto the "---"
180+
# (e.g. "user-invocable: true---"), destroying the delimiter so a
181+
# later call's pre-scan/injection never finds a second "---" and
182+
# silently drops that key entirely.
177183
out: list[str] = []
178184
dash_count = 0
179185
injected = False
@@ -187,7 +193,7 @@ def _inject_frontmatter_flag(content: str, key: str, value: str = "true") -> str
187193
elif line.endswith("\n"):
188194
eol = "\n"
189195
else:
190-
eol = ""
196+
eol = "\n"
191197
out.append(f"{key}: {value}{eol}")
192198
injected = True
193199
out.append(line)

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

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,13 @@ def _inject_frontmatter_flag(content: str, key: str, value: str = "true") -> str
115115
if dash_count == 1 and stripped.startswith(f"{key}:"):
116116
return content
117117

118-
# Inject before the closing --- of frontmatter
118+
# Inject before the closing --- of frontmatter. Preserve the
119+
# existing EOL style, but default to "\n" (rather than "") when the
120+
# closing delimiter is the last line of the file with no trailing
121+
# newline -- otherwise the injected text glues onto the "---"
122+
# (e.g. "user-invocable: true---"), destroying the delimiter so a
123+
# later call's pre-scan/injection never finds a second "---" and
124+
# silently drops that key entirely.
119125
out: list[str] = []
120126
dash_count = 0
121127
injected = False
@@ -129,7 +135,7 @@ def _inject_frontmatter_flag(content: str, key: str, value: str = "true") -> str
129135
elif line.endswith("\n"):
130136
eol = "\n"
131137
else:
132-
eol = ""
138+
eol = "\n"
133139
out.append(f"{key}: {value}{eol}")
134140
injected = True
135141
out.append(line)

‎tests/integrations/test_integration_alquimia.py‎

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -621,6 +621,56 @@ def test_skills_default_post_process_preserves_content_without_hooks(
621621
assert agy.post_process_skill_content(content) == content
622622

623623

624+
class TestAlquimiaInjectFrontmatterFlagNoTrailingNewline:
625+
"""`_inject_frontmatter_flag` must not corrupt content whose closing
626+
frontmatter delimiter is the file's last line with no trailing newline.
627+
628+
`post_process_skill_content` calls this helper on content from
629+
"external skill generators (presets, extensions)" (per the base
630+
class's docstring) -- not guaranteed to end with a trailing newline.
631+
Without a newline after the injected line, the injected text glues
632+
onto the closing `---`, destroying the delimiter.
633+
"""
634+
635+
def test_single_call_keeps_delimiter_on_its_own_line(self):
636+
from specify_cli.integrations.alquimia import AlquimiaAIIntegration
637+
638+
content = "---\nname: x\n---"
639+
result = AlquimiaAIIntegration._inject_frontmatter_flag(
640+
content, "user-invocable"
641+
)
642+
assert result == "---\nname: x\nuser-invocable: true\n---"
643+
644+
def test_chained_calls_both_apply(self):
645+
"""The exact sequence `post_process_skill_content` runs: a second
646+
injected key must still land, not be silently dropped because the
647+
first call already destroyed the closing `---` line."""
648+
from specify_cli.integrations.alquimia import AlquimiaAIIntegration
649+
650+
content = "---\nname: x\n---"
651+
result = AlquimiaAIIntegration._inject_frontmatter_flag(
652+
content, "user-invocable"
653+
)
654+
result = AlquimiaAIIntegration._inject_frontmatter_flag(
655+
result, "disable-model-invocation", "false"
656+
)
657+
assert result == (
658+
"---\nname: x\nuser-invocable: true\n"
659+
"disable-model-invocation: false\n---"
660+
)
661+
662+
def test_preserves_crlf_line_endings(self):
663+
"""When the closing delimiter *does* end with \\r\\n, the injected
664+
line must reuse that EOL rather than switching the file to LF."""
665+
from specify_cli.integrations.alquimia import AlquimiaAIIntegration
666+
667+
content = "---\r\nname: x\r\n---\r\n"
668+
result = AlquimiaAIIntegration._inject_frontmatter_flag(
669+
content, "user-invocable"
670+
)
671+
assert result == "---\r\nname: x\r\nuser-invocable: true\r\n---\r\n"
672+
673+
624674
class TestAlquimiaHookCommandNote:
625675
"""Verify dot-to-hyphen normalization note is injected in hook sections."""
626676

‎tests/integrations/test_integration_claude.py‎

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -591,6 +591,56 @@ def test_skills_default_post_process_preserves_content_without_hooks(self, tmp_p
591591
assert agy.post_process_skill_content(content) == content
592592

593593

594+
class TestClaudeInjectFrontmatterFlagNoTrailingNewline:
595+
"""`_inject_frontmatter_flag` must not corrupt content whose closing
596+
frontmatter delimiter is the file's last line with no trailing newline.
597+
598+
`post_process_skill_content` calls this helper on content from
599+
"external skill generators (presets, extensions)" (per its own
600+
docstring) -- not guaranteed to end with a trailing newline. Without a
601+
newline after the injected line, the injected text glues onto the
602+
closing `---`, destroying the delimiter.
603+
"""
604+
605+
def test_single_call_keeps_delimiter_on_its_own_line(self):
606+
from specify_cli.integrations.claude import ClaudeIntegration
607+
608+
content = "---\nname: x\n---"
609+
result = ClaudeIntegration._inject_frontmatter_flag(
610+
content, "user-invocable"
611+
)
612+
assert result == "---\nname: x\nuser-invocable: true\n---"
613+
614+
def test_chained_calls_both_apply(self):
615+
"""The exact sequence `post_process_skill_content` runs: a second
616+
injected key must still land, not be silently dropped because the
617+
first call already destroyed the closing `---` line."""
618+
from specify_cli.integrations.claude import ClaudeIntegration
619+
620+
content = "---\nname: x\n---"
621+
result = ClaudeIntegration._inject_frontmatter_flag(
622+
content, "user-invocable"
623+
)
624+
result = ClaudeIntegration._inject_frontmatter_flag(
625+
result, "disable-model-invocation", "false"
626+
)
627+
assert result == (
628+
"---\nname: x\nuser-invocable: true\n"
629+
"disable-model-invocation: false\n---"
630+
)
631+
632+
def test_preserves_crlf_line_endings(self):
633+
"""When the closing delimiter *does* end with \\r\\n, the injected
634+
line must reuse that EOL rather than switching the file to LF."""
635+
from specify_cli.integrations.claude import ClaudeIntegration
636+
637+
content = "---\r\nname: x\r\n---\r\n"
638+
result = ClaudeIntegration._inject_frontmatter_flag(
639+
content, "user-invocable"
640+
)
641+
assert result == "---\r\nname: x\r\nuser-invocable: true\r\n---\r\n"
642+
643+
594644
class TestClaudeForkContext:
595645
"""Verify context: fork is injected only for commands listed in FORK_CONTEXT_COMMANDS."""
596646

‎tests/integrations/test_integration_vibe.py‎

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -334,3 +334,53 @@ def test_all_skills_have_disable_model_invocation(self, tmp_path):
334334
assert parsed.get("disable-model-invocation") is False, (
335335
f"{f.parent.name}/SKILL.md is missing disable-model-invocation: false in frontmatter"
336336
)
337+
338+
339+
class TestVibeInjectFrontmatterFlagNoTrailingNewline:
340+
"""`_inject_frontmatter_flag` must not corrupt content whose closing
341+
frontmatter delimiter is the file's last line with no trailing newline.
342+
343+
`post_process_skill_content` calls this helper on content from
344+
"external skill generators (presets, extensions)" (per the base
345+
class's docstring) -- not guaranteed to end with a trailing newline.
346+
Without a newline after the injected line, the injected text glues
347+
onto the closing `---`, destroying the delimiter.
348+
"""
349+
350+
def test_single_call_keeps_delimiter_on_its_own_line(self):
351+
from specify_cli.integrations.vibe import VibeIntegration
352+
353+
content = "---\nname: x\n---"
354+
result = VibeIntegration._inject_frontmatter_flag(
355+
content, "user-invocable"
356+
)
357+
assert result == "---\nname: x\nuser-invocable: true\n---"
358+
359+
def test_chained_calls_both_apply(self):
360+
"""The exact sequence `post_process_skill_content` runs: a second
361+
injected key must still land, not be silently dropped because the
362+
first call already destroyed the closing `---` line."""
363+
from specify_cli.integrations.vibe import VibeIntegration
364+
365+
content = "---\nname: x\n---"
366+
result = VibeIntegration._inject_frontmatter_flag(
367+
content, "user-invocable"
368+
)
369+
result = VibeIntegration._inject_frontmatter_flag(
370+
result, "disable-model-invocation", "false"
371+
)
372+
assert result == (
373+
"---\nname: x\nuser-invocable: true\n"
374+
"disable-model-invocation: false\n---"
375+
)
376+
377+
def test_preserves_crlf_line_endings(self):
378+
"""When the closing delimiter *does* end with \\r\\n, the injected
379+
line must reuse that EOL rather than switching the file to LF."""
380+
from specify_cli.integrations.vibe import VibeIntegration
381+
382+
content = "---\r\nname: x\r\n---\r\n"
383+
result = VibeIntegration._inject_frontmatter_flag(
384+
content, "user-invocable"
385+
)
386+
assert result == "---\r\nname: x\r\nuser-invocable: true\r\n---\r\n"

0 commit comments

Comments
 (0)