From 25d774e23a5399d13bfa3c76016bef1e56f8d3c7 Mon Sep 17 00:00:00 2001 From: Noor-ul-ain001 Date: Sun, 13 Sep 2026 20:54:10 +0500 Subject: [PATCH 1/2] 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 Claude-Session: https://claude.ai/code/session_01U74yBbvVQCPwB7Ed8Dzeu6 --- .../integrations/alquimia/__init__.py | 16 ++++---- .../integrations/claude/__init__.py | 16 ++++---- src/specify_cli/integrations/vibe/__init__.py | 16 ++++---- .../integrations/test_integration_alquimia.py | 39 +++++++++++++++++++ tests/integrations/test_integration_claude.py | 39 +++++++++++++++++++ tests/integrations/test_integration_vibe.py | 39 +++++++++++++++++++ 6 files changed, 141 insertions(+), 24 deletions(-) diff --git a/src/specify_cli/integrations/alquimia/__init__.py b/src/specify_cli/integrations/alquimia/__init__.py index 132615206d..003bffa889 100644 --- a/src/specify_cli/integrations/alquimia/__init__.py +++ b/src/specify_cli/integrations/alquimia/__init__.py @@ -152,7 +152,13 @@ def _inject_frontmatter_flag(content: str, key: str, value: str = "true") -> str if dash_count == 1 and stripped.startswith(f"{key}:"): return content - # Inject before the closing --- of frontmatter + # Inject before the closing --- of frontmatter. Always emit a + # newline after the injected key so the key and the closing --- + # stay on separate lines even when the closing delimiter is the + # last line of the file with no trailing newline -- otherwise the + # injected text glues onto the "---" (e.g. "user-invocable: true---"), + # destroying the delimiter so a later call's pre-scan/injection + # never finds a second "---" and silently drops that key entirely. out: list[str] = [] dash_count = 0 injected = False @@ -161,13 +167,7 @@ def _inject_frontmatter_flag(content: str, key: str, value: str = "true") -> str if stripped == "---": dash_count += 1 if dash_count == 2 and not injected: - if line.endswith("\r\n"): - eol = "\r\n" - elif line.endswith("\n"): - eol = "\n" - else: - eol = "" - out.append(f"{key}: {value}{eol}") + out.append(f"{key}: {value}\n") injected = True out.append(line) return "".join(out) diff --git a/src/specify_cli/integrations/claude/__init__.py b/src/specify_cli/integrations/claude/__init__.py index 2ce7fb6dcc..a056634f1e 100644 --- a/src/specify_cli/integrations/claude/__init__.py +++ b/src/specify_cli/integrations/claude/__init__.py @@ -173,7 +173,13 @@ def _inject_frontmatter_flag(content: str, key: str, value: str = "true") -> str if dash_count == 1 and stripped.startswith(f"{key}:"): return content - # Inject before the closing --- of frontmatter + # Inject before the closing --- of frontmatter. Always emit a + # newline after the injected key so the key and the closing --- + # stay on separate lines even when the closing delimiter is the + # last line of the file with no trailing newline -- otherwise the + # injected text glues onto the "---" (e.g. "user-invocable: true---"), + # destroying the delimiter so a later call's pre-scan/injection + # never finds a second "---" and silently drops that key entirely. out: list[str] = [] dash_count = 0 injected = False @@ -182,13 +188,7 @@ def _inject_frontmatter_flag(content: str, key: str, value: str = "true") -> str if stripped == "---": dash_count += 1 if dash_count == 2 and not injected: - if line.endswith("\r\n"): - eol = "\r\n" - elif line.endswith("\n"): - eol = "\n" - else: - eol = "" - out.append(f"{key}: {value}{eol}") + out.append(f"{key}: {value}\n") injected = True out.append(line) return "".join(out) diff --git a/src/specify_cli/integrations/vibe/__init__.py b/src/specify_cli/integrations/vibe/__init__.py index 4412239301..678f7693cc 100644 --- a/src/specify_cli/integrations/vibe/__init__.py +++ b/src/specify_cli/integrations/vibe/__init__.py @@ -115,7 +115,13 @@ def _inject_frontmatter_flag(content: str, key: str, value: str = "true") -> str if dash_count == 1 and stripped.startswith(f"{key}:"): return content - # Inject before the closing --- of frontmatter + # Inject before the closing --- of frontmatter. Always emit a + # newline after the injected key so the key and the closing --- + # stay on separate lines even when the closing delimiter is the + # last line of the file with no trailing newline -- otherwise the + # injected text glues onto the "---" (e.g. "user-invocable: true---"), + # destroying the delimiter so a later call's pre-scan/injection + # never finds a second "---" and silently drops that key entirely. out: list[str] = [] dash_count = 0 injected = False @@ -124,13 +130,7 @@ def _inject_frontmatter_flag(content: str, key: str, value: str = "true") -> str if stripped == "---": dash_count += 1 if dash_count == 2 and not injected: - if line.endswith("\r\n"): - eol = "\r\n" - elif line.endswith("\n"): - eol = "\n" - else: - eol = "" - out.append(f"{key}: {value}{eol}") + out.append(f"{key}: {value}\n") injected = True out.append(line) return "".join(out) diff --git a/tests/integrations/test_integration_alquimia.py b/tests/integrations/test_integration_alquimia.py index e8eab8281c..c03ce5a14a 100644 --- a/tests/integrations/test_integration_alquimia.py +++ b/tests/integrations/test_integration_alquimia.py @@ -621,6 +621,45 @@ def test_skills_default_post_process_preserves_content_without_hooks( assert agy.post_process_skill_content(content) == content +class TestAlquimiaInjectFrontmatterFlagNoTrailingNewline: + """`_inject_frontmatter_flag` must not corrupt content whose closing + frontmatter delimiter is the file's last line with no trailing newline. + + `post_process_skill_content` calls this helper on content from + "external skill generators (presets, extensions)" (per the base + class's docstring) -- not guaranteed to end with a trailing newline. + Without a newline after the injected line, the injected text glues + onto the closing `---`, destroying the delimiter. + """ + + def test_single_call_keeps_delimiter_on_its_own_line(self): + from specify_cli.integrations.alquimia import AlquimiaAIIntegration + + content = "---\nname: x\n---" + result = AlquimiaAIIntegration._inject_frontmatter_flag( + content, "user-invocable" + ) + assert result == "---\nname: x\nuser-invocable: true\n---" + + def test_chained_calls_both_apply(self): + """The exact sequence `post_process_skill_content` runs: a second + injected key must still land, not be silently dropped because the + first call already destroyed the closing `---` line.""" + from specify_cli.integrations.alquimia import AlquimiaAIIntegration + + content = "---\nname: x\n---" + result = AlquimiaAIIntegration._inject_frontmatter_flag( + content, "user-invocable" + ) + result = AlquimiaAIIntegration._inject_frontmatter_flag( + result, "disable-model-invocation", "false" + ) + assert result == ( + "---\nname: x\nuser-invocable: true\n" + "disable-model-invocation: false\n---" + ) + + class TestAlquimiaHookCommandNote: """Verify dot-to-hyphen normalization note is injected in hook sections.""" diff --git a/tests/integrations/test_integration_claude.py b/tests/integrations/test_integration_claude.py index 3718af9740..32dbc392c0 100644 --- a/tests/integrations/test_integration_claude.py +++ b/tests/integrations/test_integration_claude.py @@ -591,6 +591,45 @@ def test_skills_default_post_process_preserves_content_without_hooks(self, tmp_p assert agy.post_process_skill_content(content) == content +class TestClaudeInjectFrontmatterFlagNoTrailingNewline: + """`_inject_frontmatter_flag` must not corrupt content whose closing + frontmatter delimiter is the file's last line with no trailing newline. + + `post_process_skill_content` calls this helper on content from + "external skill generators (presets, extensions)" (per its own + docstring) -- not guaranteed to end with a trailing newline. Without a + newline after the injected line, the injected text glues onto the + closing `---`, destroying the delimiter. + """ + + def test_single_call_keeps_delimiter_on_its_own_line(self): + from specify_cli.integrations.claude import ClaudeIntegration + + content = "---\nname: x\n---" + result = ClaudeIntegration._inject_frontmatter_flag( + content, "user-invocable" + ) + assert result == "---\nname: x\nuser-invocable: true\n---" + + def test_chained_calls_both_apply(self): + """The exact sequence `post_process_skill_content` runs: a second + injected key must still land, not be silently dropped because the + first call already destroyed the closing `---` line.""" + from specify_cli.integrations.claude import ClaudeIntegration + + content = "---\nname: x\n---" + result = ClaudeIntegration._inject_frontmatter_flag( + content, "user-invocable" + ) + result = ClaudeIntegration._inject_frontmatter_flag( + result, "disable-model-invocation", "false" + ) + assert result == ( + "---\nname: x\nuser-invocable: true\n" + "disable-model-invocation: false\n---" + ) + + class TestClaudeForkContext: """Verify context: fork is injected only for commands listed in FORK_CONTEXT_COMMANDS.""" diff --git a/tests/integrations/test_integration_vibe.py b/tests/integrations/test_integration_vibe.py index 55f410c088..e2e461a9a1 100644 --- a/tests/integrations/test_integration_vibe.py +++ b/tests/integrations/test_integration_vibe.py @@ -334,3 +334,42 @@ def test_all_skills_have_disable_model_invocation(self, tmp_path): assert parsed.get("disable-model-invocation") is False, ( f"{f.parent.name}/SKILL.md is missing disable-model-invocation: false in frontmatter" ) + + +class TestVibeInjectFrontmatterFlagNoTrailingNewline: + """`_inject_frontmatter_flag` must not corrupt content whose closing + frontmatter delimiter is the file's last line with no trailing newline. + + `post_process_skill_content` calls this helper on content from + "external skill generators (presets, extensions)" (per the base + class's docstring) -- not guaranteed to end with a trailing newline. + Without a newline after the injected line, the injected text glues + onto the closing `---`, destroying the delimiter. + """ + + def test_single_call_keeps_delimiter_on_its_own_line(self): + from specify_cli.integrations.vibe import VibeIntegration + + content = "---\nname: x\n---" + result = VibeIntegration._inject_frontmatter_flag( + content, "user-invocable" + ) + assert result == "---\nname: x\nuser-invocable: true\n---" + + def test_chained_calls_both_apply(self): + """The exact sequence `post_process_skill_content` runs: a second + injected key must still land, not be silently dropped because the + first call already destroyed the closing `---` line.""" + from specify_cli.integrations.vibe import VibeIntegration + + content = "---\nname: x\n---" + result = VibeIntegration._inject_frontmatter_flag( + content, "user-invocable" + ) + result = VibeIntegration._inject_frontmatter_flag( + result, "disable-model-invocation", "false" + ) + assert result == ( + "---\nname: x\nuser-invocable: true\n" + "disable-model-invocation: false\n---" + ) From 67304e08819e76be3b85dd0a9ec4ff0e9d07e4de Mon Sep 17 00:00:00 2001 From: Noor-ul-ain001 Date: Sun, 20 Sep 2026 01:07:39 +0500 Subject: [PATCH 2/2] 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 --- .../integrations/alquimia/__init__.py | 22 ++++++++++++------- .../integrations/claude/__init__.py | 22 ++++++++++++------- src/specify_cli/integrations/vibe/__init__.py | 22 ++++++++++++------- .../integrations/test_integration_alquimia.py | 11 ++++++++++ tests/integrations/test_integration_claude.py | 11 ++++++++++ tests/integrations/test_integration_vibe.py | 11 ++++++++++ 6 files changed, 75 insertions(+), 24 deletions(-) diff --git a/src/specify_cli/integrations/alquimia/__init__.py b/src/specify_cli/integrations/alquimia/__init__.py index 003bffa889..54ea47f486 100644 --- a/src/specify_cli/integrations/alquimia/__init__.py +++ b/src/specify_cli/integrations/alquimia/__init__.py @@ -152,13 +152,13 @@ def _inject_frontmatter_flag(content: str, key: str, value: str = "true") -> str if dash_count == 1 and stripped.startswith(f"{key}:"): return content - # Inject before the closing --- of frontmatter. Always emit a - # newline after the injected key so the key and the closing --- - # stay on separate lines even when the closing delimiter is the - # last line of the file with no trailing newline -- otherwise the - # injected text glues onto the "---" (e.g. "user-invocable: true---"), - # destroying the delimiter so a later call's pre-scan/injection - # never finds a second "---" and silently drops that key entirely. + # Inject before the closing --- of frontmatter. Preserve the + # existing EOL style, but default to "\n" (rather than "") when the + # closing delimiter is the last line of the file with no trailing + # newline -- otherwise the injected text glues onto the "---" + # (e.g. "user-invocable: true---"), destroying the delimiter so a + # later call's pre-scan/injection never finds a second "---" and + # silently drops that key entirely. out: list[str] = [] dash_count = 0 injected = False @@ -167,7 +167,13 @@ def _inject_frontmatter_flag(content: str, key: str, value: str = "true") -> str if stripped == "---": dash_count += 1 if dash_count == 2 and not injected: - out.append(f"{key}: {value}\n") + if line.endswith("\r\n"): + eol = "\r\n" + elif line.endswith("\n"): + eol = "\n" + else: + eol = "\n" + out.append(f"{key}: {value}{eol}") injected = True out.append(line) return "".join(out) diff --git a/src/specify_cli/integrations/claude/__init__.py b/src/specify_cli/integrations/claude/__init__.py index a056634f1e..9a14cf50b0 100644 --- a/src/specify_cli/integrations/claude/__init__.py +++ b/src/specify_cli/integrations/claude/__init__.py @@ -173,13 +173,13 @@ def _inject_frontmatter_flag(content: str, key: str, value: str = "true") -> str if dash_count == 1 and stripped.startswith(f"{key}:"): return content - # Inject before the closing --- of frontmatter. Always emit a - # newline after the injected key so the key and the closing --- - # stay on separate lines even when the closing delimiter is the - # last line of the file with no trailing newline -- otherwise the - # injected text glues onto the "---" (e.g. "user-invocable: true---"), - # destroying the delimiter so a later call's pre-scan/injection - # never finds a second "---" and silently drops that key entirely. + # Inject before the closing --- of frontmatter. Preserve the + # existing EOL style, but default to "\n" (rather than "") when the + # closing delimiter is the last line of the file with no trailing + # newline -- otherwise the injected text glues onto the "---" + # (e.g. "user-invocable: true---"), destroying the delimiter so a + # later call's pre-scan/injection never finds a second "---" and + # silently drops that key entirely. out: list[str] = [] dash_count = 0 injected = False @@ -188,7 +188,13 @@ def _inject_frontmatter_flag(content: str, key: str, value: str = "true") -> str if stripped == "---": dash_count += 1 if dash_count == 2 and not injected: - out.append(f"{key}: {value}\n") + if line.endswith("\r\n"): + eol = "\r\n" + elif line.endswith("\n"): + eol = "\n" + else: + eol = "\n" + out.append(f"{key}: {value}{eol}") injected = True out.append(line) return "".join(out) diff --git a/src/specify_cli/integrations/vibe/__init__.py b/src/specify_cli/integrations/vibe/__init__.py index 678f7693cc..44ae6f96df 100644 --- a/src/specify_cli/integrations/vibe/__init__.py +++ b/src/specify_cli/integrations/vibe/__init__.py @@ -115,13 +115,13 @@ def _inject_frontmatter_flag(content: str, key: str, value: str = "true") -> str if dash_count == 1 and stripped.startswith(f"{key}:"): return content - # Inject before the closing --- of frontmatter. Always emit a - # newline after the injected key so the key and the closing --- - # stay on separate lines even when the closing delimiter is the - # last line of the file with no trailing newline -- otherwise the - # injected text glues onto the "---" (e.g. "user-invocable: true---"), - # destroying the delimiter so a later call's pre-scan/injection - # never finds a second "---" and silently drops that key entirely. + # Inject before the closing --- of frontmatter. Preserve the + # existing EOL style, but default to "\n" (rather than "") when the + # closing delimiter is the last line of the file with no trailing + # newline -- otherwise the injected text glues onto the "---" + # (e.g. "user-invocable: true---"), destroying the delimiter so a + # later call's pre-scan/injection never finds a second "---" and + # silently drops that key entirely. out: list[str] = [] dash_count = 0 injected = False @@ -130,7 +130,13 @@ def _inject_frontmatter_flag(content: str, key: str, value: str = "true") -> str if stripped == "---": dash_count += 1 if dash_count == 2 and not injected: - out.append(f"{key}: {value}\n") + if line.endswith("\r\n"): + eol = "\r\n" + elif line.endswith("\n"): + eol = "\n" + else: + eol = "\n" + out.append(f"{key}: {value}{eol}") injected = True out.append(line) return "".join(out) diff --git a/tests/integrations/test_integration_alquimia.py b/tests/integrations/test_integration_alquimia.py index c03ce5a14a..6e8b6dbadd 100644 --- a/tests/integrations/test_integration_alquimia.py +++ b/tests/integrations/test_integration_alquimia.py @@ -659,6 +659,17 @@ def test_chained_calls_both_apply(self): "disable-model-invocation: false\n---" ) + def test_preserves_crlf_line_endings(self): + """When the closing delimiter *does* end with \\r\\n, the injected + line must reuse that EOL rather than switching the file to LF.""" + from specify_cli.integrations.alquimia import AlquimiaAIIntegration + + content = "---\r\nname: x\r\n---\r\n" + result = AlquimiaAIIntegration._inject_frontmatter_flag( + content, "user-invocable" + ) + assert result == "---\r\nname: x\r\nuser-invocable: true\r\n---\r\n" + class TestAlquimiaHookCommandNote: """Verify dot-to-hyphen normalization note is injected in hook sections.""" diff --git a/tests/integrations/test_integration_claude.py b/tests/integrations/test_integration_claude.py index 32dbc392c0..8b8067a8a0 100644 --- a/tests/integrations/test_integration_claude.py +++ b/tests/integrations/test_integration_claude.py @@ -629,6 +629,17 @@ def test_chained_calls_both_apply(self): "disable-model-invocation: false\n---" ) + def test_preserves_crlf_line_endings(self): + """When the closing delimiter *does* end with \\r\\n, the injected + line must reuse that EOL rather than switching the file to LF.""" + from specify_cli.integrations.claude import ClaudeIntegration + + content = "---\r\nname: x\r\n---\r\n" + result = ClaudeIntegration._inject_frontmatter_flag( + content, "user-invocable" + ) + assert result == "---\r\nname: x\r\nuser-invocable: true\r\n---\r\n" + class TestClaudeForkContext: """Verify context: fork is injected only for commands listed in FORK_CONTEXT_COMMANDS.""" diff --git a/tests/integrations/test_integration_vibe.py b/tests/integrations/test_integration_vibe.py index e2e461a9a1..8bd26fe75f 100644 --- a/tests/integrations/test_integration_vibe.py +++ b/tests/integrations/test_integration_vibe.py @@ -373,3 +373,14 @@ def test_chained_calls_both_apply(self): "---\nname: x\nuser-invocable: true\n" "disable-model-invocation: false\n---" ) + + def test_preserves_crlf_line_endings(self): + """When the closing delimiter *does* end with \\r\\n, the injected + line must reuse that EOL rather than switching the file to LF.""" + from specify_cli.integrations.vibe import VibeIntegration + + content = "---\r\nname: x\r\n---\r\n" + result = VibeIntegration._inject_frontmatter_flag( + content, "user-invocable" + ) + assert result == "---\r\nname: x\r\nuser-invocable: true\r\n---\r\n"