From d8bd788a0faa1d893ae0b2e02e3130396610c80f Mon Sep 17 00:00:00 2001 From: WOLIKIMCHENG <35391914+WOLIKIMCHENG@users.noreply.github.com> Date: Sun, 20 Sep 2026 10:51:10 +0800 Subject: [PATCH 1/3] fix(events): preserve unowned Vibe hooks files Avoid rewriting or deleting hooks.toml when cleanup finds no Spec Kit-managed hooks to remove. Preserve existing cleanup behavior for managed entries. --- src/specify_cli/events.py | 4 ++ tests/integrations/test_events.py | 69 +++++++++++++++++++++++++++++++ 2 files changed, 73 insertions(+) diff --git a/src/specify_cli/events.py b/src/specify_cli/events.py index 3632661711..d2c419df43 100644 --- a/src/specify_cli/events.py +++ b/src/specify_cli/events.py @@ -2251,6 +2251,8 @@ def _remove_vibe_toml_entries(dst: Path) -> bool: """Remove Specify-marked Vibe TOML hook entries; delete the file if now empty. Returns True if the file was deleted (no user content remained). + + Leaves an unowned file untouched when no Specify-marked hook was removed. """ if not dst.exists(): return False @@ -2272,6 +2274,8 @@ def _remove_vibe_toml_entries(dst: Path) -> bool: existing, flags=re.DOTALL, ) + if cleaned == existing: + return False # If only whitespace/comments remain, the file had no user content stripped = "\n".join( line for line in cleaned.splitlines() diff --git a/tests/integrations/test_events.py b/tests/integrations/test_events.py index 3c32891288..eff175ff5b 100644 --- a/tests/integrations/test_events.py +++ b/tests/integrations/test_events.py @@ -1046,6 +1046,75 @@ def test_owned_only_config_still_deleted_on_teardown(self, tmp_path): assert not config_path.exists() +class TestVibeTomlNoOpRemoval: + """An unowned Vibe hooks.toml must survive no-op event cleanup intact.""" + + _FIXED_MTIME_NS = 1_700_000_000_123_456_789 + + def _user_hooks_file(self, tmp_path, content): + path = tmp_path / ".vibe" / "hooks.toml" + path.parent.mkdir(parents=True) + path.write_bytes(content) + os.utime(path, ns=(self._FIXED_MTIME_NS, self._FIXED_MTIME_NS)) + return path, path.stat().st_mtime_ns + + def _assert_untouched(self, path, original, original_mtime_ns): + assert path.exists() + assert path.read_bytes() == original + assert path.stat().st_mtime_ns == original_mtime_ns + + def test_empty_events_leave_user_crlf_file_untracked_and_untouched(self, tmp_path): + from specify_cli.integrations import get_integration + + integration = get_integration("vibe") + manifest = _claude_manifest(tmp_path) + original = b'user_option = "keep"\r\nsecond_option = true' + path, original_mtime_ns = self._user_hooks_file(tmp_path, original) + + install_integration_events(integration, tmp_path, manifest, {}) + + self._assert_untouched(path, original, original_mtime_ns) + manifest.record_existing.assert_not_called() + + def test_empty_events_preserve_comments_only_file(self, tmp_path): + from specify_cli.integrations import get_integration + + original = b"# maintained by the user\n# no hooks yet\n" + path, original_mtime_ns = self._user_hooks_file(tmp_path, original) + + install_integration_events( + get_integration("vibe"), tmp_path, _claude_manifest(tmp_path), {} + ) + + self._assert_untouched(path, original, original_mtime_ns) + + def test_empty_events_preserve_whitespace_only_file(self, tmp_path): + from specify_cli.integrations import get_integration + + original = b"\r\n \t\r\n" + path, original_mtime_ns = self._user_hooks_file(tmp_path, original) + + install_integration_events( + get_integration("vibe"), tmp_path, _claude_manifest(tmp_path), {} + ) + + self._assert_untouched(path, original, original_mtime_ns) + + def test_forced_teardown_preserves_unowned_file_with_manifest_claim(self, tmp_path): + from specify_cli.integrations import get_integration + + integration = get_integration("vibe") + original = b'user_option = "keep"\r\n' + path, original_mtime_ns = self._user_hooks_file(tmp_path, original) + manifest = IntegrationManifest(integration.key, tmp_path, version="test") + manifest.record_existing(".vibe/hooks.toml") + manifest.save() + + integration.teardown(tmp_path, manifest, force=True) + + self._assert_untouched(path, original, original_mtime_ns) + + # -- Opencode TS Plugin merging --------------------------------------------- class TestOpencodePluginMerging: From 1359d75efc1caf462ba68a567f977a2a778cabab Mon Sep 17 00:00:00 2001 From: WOLIKIMCHENG <35391914+WOLIKIMCHENG@users.noreply.github.com> Date: Thu, 24 Sep 2026 11:30:49 +0800 Subject: [PATCH 2/3] fix(events): preserve Vibe hook Unicode and POSIX paths --- src/specify_cli/events/__init__.py | 2 +- src/specify_cli/integrations/vibe/__init__.py | 4 ++-- tests/integrations/test_integration_vibe.py | 14 ++++++++++++++ 3 files changed, 17 insertions(+), 3 deletions(-) diff --git a/src/specify_cli/events/__init__.py b/src/specify_cli/events/__init__.py index 5fe6c14bc7..36e5d8f494 100644 --- a/src/specify_cli/events/__init__.py +++ b/src/specify_cli/events/__init__.py @@ -1511,7 +1511,7 @@ def install_integration_events( native_timeout=lambda seconds: _native_timeout(integration, seconds), ensure_safe_destination=_ensure_safe_destination, ): - rel = str(config_path.relative_to(project_root)) + rel = config_path.relative_to(project_root).as_posix() if rel not in manifest.files: manifest.record_existing(rel) created.append(config_path) diff --git a/src/specify_cli/integrations/vibe/__init__.py b/src/specify_cli/integrations/vibe/__init__.py index 6c0e5bf8c1..6de58f95db 100644 --- a/src/specify_cli/integrations/vibe/__init__.py +++ b/src/specify_cli/integrations/vibe/__init__.py @@ -6,7 +6,6 @@ from __future__ import annotations -import json import logging import os import re @@ -15,6 +14,7 @@ from ..base import IntegrationOption, SkillsIntegration from ..manifest import IntegrationManifest +from ..._toml_string import escape_toml_basic from ..._utils import dump_frontmatter # Per-command frontmatter overrides for skills that should run in a forked @@ -220,7 +220,7 @@ def _hook_target_os() -> str: @staticmethod def _toml_quote(value: str) -> str: """Render a TOML basic string without exposing Vibe syntax to events.""" - return json.dumps(value) + return escape_toml_basic(value) @staticmethod def _managed_hooks_pattern() -> re.Pattern[str]: diff --git a/tests/integrations/test_integration_vibe.py b/tests/integrations/test_integration_vibe.py index 020ece178f..b46de417b6 100644 --- a/tests/integrations/test_integration_vibe.py +++ b/tests/integrations/test_integration_vibe.py @@ -171,6 +171,20 @@ def test_wildcard_matcher_omitted(self, tmp_path): (hook,) = self._parse(tmp_path)["hooks"] assert "match" not in hook + def test_non_bmp_matcher_round_trips_through_toml(self, tmp_path): + """Vibe hook strings retain non-BMP Unicode rather than TOML-invalid surrogates.""" + matcher = "Edit|😀" + self._install(tmp_path, { + "pre_tool_use": [{"command": "speckit.tdd.validate", "matcher": matcher}], + }) + (hook,) = self._parse(tmp_path)["hooks"] + assert hook["match"] == f"re:{matcher}" + + def test_toml_quote_round_trips_special_characters(self): + value = 'quote: " backslash: \\ newline: \n carriage: \r tab: \t nul: \x00 unit: \x1f del: \x7f' + quoted = VibeIntegration._toml_quote(value) + assert tomllib.loads(f"value = {quoted}\n")["value"] == value + def test_unsupported_events_are_skipped(self, tmp_path, capsys): self._install(tmp_path, { "session_start": [{"command": "speckit.agent-context.update"}], From 77309618b27a8473e89adce4934ce547bda2ca57 Mon Sep 17 00:00:00 2001 From: WOLIKIMCHENG <35391914+WOLIKIMCHENG@users.noreply.github.com> Date: Fri, 16 Oct 2026 12:40:55 +0800 Subject: [PATCH 3/3] fix(vibe): preserve TOML physical lines Assisted-by: Codex CLI (model: gpt-5.6-terra, autonomous) --- src/specify_cli/integrations/vibe/__init__.py | 31 ++++-- tests/integrations/test_integration_vibe.py | 104 +++++++++++++++++- 2 files changed, 127 insertions(+), 8 deletions(-) diff --git a/src/specify_cli/integrations/vibe/__init__.py b/src/specify_cli/integrations/vibe/__init__.py index a528396742..692e303da6 100644 --- a/src/specify_cli/integrations/vibe/__init__.py +++ b/src/specify_cli/integrations/vibe/__init__.py @@ -10,12 +10,12 @@ import os import re import tomllib -from collections.abc import Callable, Mapping, Sequence +from collections.abc import Callable, Iterator, Mapping, Sequence from pathlib import Path from typing import Any from ..base import IntegrationOption, SkillsIntegration -from .._file_changes import unlink as _unlink_file, write_text as _write_text +from .._file_changes import unlink as _unlink_file, write_bytes as _write_bytes from ..manifest import IntegrationManifest from ..._toml_string import escape_toml_basic from ..._utils import dump_frontmatter @@ -41,6 +41,21 @@ logger = logging.getLogger(__name__) +def _toml_physical_lines(source: str) -> Iterator[str]: + """Yield TOML physical lines, preserving their original terminators. + + TOML recognizes LF and CRLF only. In particular, Python's ``splitlines`` + would incorrectly treat bare CR and Unicode line-separator characters as + physical line boundaries. + """ + start = 0 + while (newline := source.find("\n", start)) >= 0: + yield source[start:newline + 1] + start = newline + 1 + if start < len(source): + yield source[start:] + + class VibeIntegration(SkillsIntegration): """Integration for Mistral Vibe skills.""" @@ -344,7 +359,7 @@ def _managed_hook_spans(source: str) -> list[tuple[int, int]] | None: array_depth = 0 offset = 0 - for line in source.splitlines(keepends=True): + for line in _toml_physical_lines(source): content = line.rstrip("\r\n") index = 0 code_end = len(content) @@ -547,7 +562,8 @@ def remove_vibe_event_hooks( return False ensure_safe_destination(path) try: - existing = path.read_text(encoding="utf-8") + with path.open(encoding="utf-8", newline="") as file: + existing = file.read() except (OSError, UnicodeDecodeError) as exc: logger.warning( "Could not read %s (it may be unreadable or not UTF-8); " @@ -570,7 +586,7 @@ def remove_vibe_event_hooks( if not non_comment_content: _unlink_file(path, missing_ok=True) return True - _write_text(path, cleaned, encoding="utf-8") + _write_bytes(path, cleaned.encode("utf-8")) return False def _merge_managed_hooks( @@ -589,7 +605,8 @@ def _merge_managed_hooks( existing = "" if path.exists(): try: - existing = path.read_text(encoding="utf-8") + with path.open(encoding="utf-8", newline="") as file: + existing = file.read() except (OSError, UnicodeDecodeError) as exc: logger.warning( "Could not read %s (it may be unreadable or not UTF-8); " @@ -602,5 +619,5 @@ def _merge_managed_hooks( if cleaned is None: return False path.parent.mkdir(parents=True, exist_ok=True) - _write_text(path, cleaned.rstrip() + "\n\n" + fragment + "\n", encoding="utf-8") + _write_bytes(path, (cleaned.rstrip() + "\n\n" + fragment + "\n").encode("utf-8")) return True diff --git a/tests/integrations/test_integration_vibe.py b/tests/integrations/test_integration_vibe.py index a56721e857..349c15fb17 100644 --- a/tests/integrations/test_integration_vibe.py +++ b/tests/integrations/test_integration_vibe.py @@ -229,6 +229,27 @@ def test_non_bmp_matcher_round_trips_through_toml(self, tmp_path): (hook,) = self._parse(tmp_path)["hooks"] assert hook["match"] == f"re:{matcher}" + @pytest.mark.parametrize("separator", ("\u0085", "\u2028", "\u2029")) + def test_generated_matcher_unicode_separator_lifecycle(self, tmp_path, separator): + """Unicode separators in TOML strings do not split managed hook spans.""" + matcher = f"Edit{separator}Write" + integration, _ = self._install(tmp_path, { + "pre_tool_use": [{"command": "speckit.tdd.validate", "matcher": matcher}], + }) + assert self._parse(tmp_path)["hooks"][0]["match"] == f"re:{matcher}" + + self._install(tmp_path, { + "pre_tool_use": [{"command": "speckit.review.verify", "matcher": matcher}], + }) + (hook,) = self._parse(tmp_path)["hooks"] + assert hook["name"] == "speckit-pre_tool-verify" + assert hook["match"] == f"re:{matcher}" + + assert integration.remove_vibe_event_hooks( + tmp_path, ensure_safe_destination=lambda _path: None + ) is True + assert not (tmp_path / ".vibe" / "hooks.toml").exists() + def test_toml_quote_round_trips_special_characters(self): value = 'quote: " backslash: \\ newline: \n carriage: \r tab: \t nul: \x00 unit: \x1f del: \x7f' quoted = VibeIntegration._toml_quote(value) @@ -293,6 +314,29 @@ def test_merge_and_teardown_preserve_user_hooks(self, tmp_path): remaining = self._parse(tmp_path)["hooks"] assert [h["name"] for h in remaining] == ["deny-rm-rf"] + @pytest.mark.parametrize("newline", ("\n", "\r\n"), ids=("lf", "crlf")) + def test_merge_preserves_user_physical_line_bytes(self, tmp_path, newline): + """A native Windows write must not expand retained CRLF to CRCRLF.""" + user_content = ( + f"[[hooks]]{newline}" + f'name = "user-hook"{newline}' + f'type = "pre_tool"{newline}' + f'command = "guard-bash"' + ) + path = tmp_path / ".vibe" / "hooks.toml" + path.parent.mkdir(parents=True) + path.write_bytes((user_content + newline).encode("utf-8")) + + self._install(tmp_path, {"pre_tool_use": [{"command": "speckit.tdd.validate"}]}) + + merged = path.read_bytes() + assert merged.startswith(user_content.encode("utf-8") + b"\n\n") + assert b"\r\r\n" not in merged + assert [hook["name"] for hook in self._parse(tmp_path)["hooks"]] == [ + "user-hook", + "speckit-pre_tool-validate", + ] + def test_merge_preserves_user_command_with_marker_text(self, tmp_path): """A marker-looking command string does not claim a user hook block.""" config_path = tmp_path / ".vibe" / "hooks.toml" @@ -313,6 +357,37 @@ def test_merge_preserves_user_command_with_marker_text(self, tmp_path): "speckit-pre_tool-validate", ] + @pytest.mark.parametrize("separator", ("\u0085", "\u2028", "\u2029")) + def test_comment_unicode_separator_cannot_spoof_marker(self, tmp_path, separator): + """Only TOML LF and CRLF can end a comment for ownership scanning.""" + user_block = ( + "[[hooks]]\n" + 'name = "user-hook"\n' + 'type = "pre_tool"\n' + f"# user note{separator}speckit_marker = true\n" + 'command = "guard-bash"\n' + ) + path = tmp_path / ".vibe" / "hooks.toml" + path.parent.mkdir(parents=True) + original = user_block.encode() + path.write_bytes(original) + mtime_ns = 1_700_000_000_123_456_789 + os.utime(path, ns=(mtime_ns, mtime_ns)) + + integration = get_integration("vibe") + assert integration.remove_vibe_event_hooks( + tmp_path, ensure_safe_destination=lambda _path: None + ) is False + assert path.read_bytes() == original + assert path.stat().st_mtime_ns == mtime_ns + + self._install(tmp_path, {"pre_tool_use": [{"command": "speckit.tdd.validate"}]}) + assert user_block in path.read_bytes().decode("utf-8") + assert [hook["name"] for hook in self._parse(tmp_path)["hooks"]] == [ + "user-hook", + "speckit-pre_tool-validate", + ] + def test_commands_carry_structured_output_envelope(self, tmp_path): """Vibe parses non-empty hook stdout as JSON (HookStructuredResponse); plain text is reported as a hook failure. Every generated hook command @@ -451,6 +526,28 @@ def test_removal_preserves_user_comment_with_marker_text(self, tmp_path): assert removed_file is False self._assert_untouched(path, original, original_mtime_ns) + @pytest.mark.parametrize("newline", ("\n", "\r\n"), ids=("lf", "crlf")) + def test_removal_recognizes_valid_marker_with_toml_line_endings(self, tmp_path, newline): + """Removal uses both TOML line endings without expanding retained CRLF.""" + managed = ( + f"[[hooks]]{newline}" + f"name = \"managed\"{newline}" + f"speckit_marker = true{newline}" + ) + user = ( + f"[[hooks]]{newline}" + f"name = \"user-hook\"{newline}" + f"command = \"guard-bash\"{newline}" + ) + path, _ = self._user_hooks_file(tmp_path, (managed + user).encode()) + + removed_file = get_integration("vibe").remove_vibe_event_hooks( + tmp_path, ensure_safe_destination=lambda _path: None + ) + + assert removed_file is False + assert path.read_bytes() == user.encode() + def test_removal_preserves_marker_inside_multiline_strings(self, tmp_path): integration = get_integration("vibe") for name, delimiter in (("basic", '"""'), ("literal", "'''")): @@ -588,8 +685,13 @@ def test_removal_leaves_malformed_multiline_source_untouched(self, tmp_path): "[other]\n" 'keep = "\u4fdd\u7559"\n' ).encode(), + ( + "[[hooks]]\r" + 'name = "user-hook"\r' + "speckit_marker = true\r" + ).encode(), ], - ids=("marker-like-array", "bare-invalid-text"), + ids=("marker-like-array", "bare-invalid-text", "bare-cr"), ) def test_malformed_toml_leaves_removal_and_merge_unowned_and_untouched( self, tmp_path, original