diff --git a/docs/integrations/vibe.md b/docs/integrations/vibe.md index c359bd54e5..f211ab6d64 100644 --- a/docs/integrations/vibe.md +++ b/docs/integrations/vibe.md @@ -36,6 +36,14 @@ unsupported handlers are skipped with a warning. `--integration-options="--events false"` disables Spec Kit event handlers. See [runtime events](cli.md#runtime-events). +Spec Kit refreshes and removes only the `[[hooks]]` entries it wrote, which +carry `speckit_marker = true`. Each entry ends after its last TOML line and the +blank lines right after it, so a comment that follows a Spec Kit entry, such as +a note above your own hook, stays in the file. If removing Spec Kit's entries +leaves only comments and blank lines, Spec Kit deletes `.vibe/hooks.toml`, and +those comments are deleted with it. Spec Kit applies the same rule to Codex's +`.codex/config.toml`. + ## Workflow Dispatch Workflow steps use `vibe -p ` and request `--output json` for structured diff --git a/src/specify_cli/integrations/vibe/__init__.py b/src/specify_cli/integrations/vibe/__init__.py index 692e303da6..c7616235e2 100644 --- a/src/specify_cli/integrations/vibe/__init__.py +++ b/src/specify_cli/integrations/vibe/__init__.py @@ -345,6 +345,10 @@ def _managed_hook_spans(source: str) -> list[tuple[int, int]] | None: headers. A parser validity gate and the scanner both return ``None`` for malformed or ambiguous input so callers can leave the source byte-for-byte untouched. + + A block ends after its last line of TOML and the blank lines right + after it. A standalone comment that follows stays with the content + after it, such as the user hook it introduces (#5079). """ try: tomllib.loads(source) @@ -353,6 +357,8 @@ def _managed_hook_spans(source: str) -> list[tuple[int, int]] | None: spans: list[tuple[int, int]] = [] current_start: int | None = None + current_end = 0 + current_after_comment = False current_is_hooks = False current_has_marker = False multiline: str | None = None @@ -436,7 +442,7 @@ def _managed_hook_spans(source: str) -> list[tuple[int, int]] | None: ) if header_path is not None: if current_start is not None and current_has_marker: - spans.append((current_start, offset)) + spans.append((current_start, current_end)) current_start = offset current_is_hooks = is_array and header_path == ("hooks",) current_has_marker = False @@ -465,12 +471,22 @@ def _managed_hook_spans(source: str) -> list[tuple[int, int]] | None: if array_depth < 0: return None + # Lines inside a multiline string or array are part of the value, + # even when they look like comments. + if line_starts_in_array or line_has_multiline_string or code.strip(" \t"): + current_end = offset + len(line) + current_after_comment = False + elif content.strip(" \t"): + current_after_comment = True + elif not current_after_comment: + current_end = offset + len(line) + offset += len(line) if multiline is not None or array_depth: return None if current_start is not None and current_has_marker: - spans.append((current_start, len(source))) + spans.append((current_start, current_end)) return spans @classmethod diff --git a/tests/integrations/test_integration_vibe.py b/tests/integrations/test_integration_vibe.py index 5ac37c2ea0..61a06cd183 100644 --- a/tests/integrations/test_integration_vibe.py +++ b/tests/integrations/test_integration_vibe.py @@ -314,6 +314,39 @@ 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( + "user_after", + ( + "# NOTE: keep guard-bash first\n" + "[[hooks]]\n" + 'name = "user-hook"\n' + 'type = "pre_tool"\n' + 'command = "guard-bash"\n', + "# notes about these hooks\n", + ), + ids=("comment-before-user-hook", "trailing-comment"), + ) + def test_refresh_and_teardown_keep_comments_after_a_managed_hook( + self, tmp_path, user_after + ): + """A comment after a managed block is user content, not part of the block (#5079).""" + path = tmp_path / ".vibe" / "hooks.toml" + path.parent.mkdir(parents=True) + path.write_bytes( + b'[[hooks]]\nname = "deny-rm-rf"\ntype = "pre_tool"\ncommand = "guard-rm"\n' + ) + events = {"pre_tool_use": [{"command": "speckit.tdd.validate"}]} + integration, manifest = self._install(tmp_path, events) + path.write_bytes(path.read_bytes() + ("\n" + user_after).encode("utf-8")) + + integration, manifest = self._install(tmp_path, events) + assert user_after in path.read_text(encoding="utf-8") + + remove_integration_events(integration, tmp_path, manifest) + remaining = path.read_text(encoding="utf-8") + assert user_after in remaining + assert "speckit_marker" not in remaining + @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.""" @@ -464,6 +497,20 @@ def test_teardown_deletes_file_without_user_content(self, tmp_path): remove_integration_events(integration, tmp_path, manifest) assert not (tmp_path / ".vibe" / "hooks.toml").exists() + def test_teardown_deletes_file_when_only_comments_remain(self, tmp_path): + """A trailing comment survives refresh, but a file left with only + comments is deleted, as for Codex's config.toml (#5079).""" + path = tmp_path / ".vibe" / "hooks.toml" + events = {"pre_tool_use": [{"command": "speckit.tdd.validate"}]} + integration, manifest = self._install(tmp_path, events) + path.write_bytes(path.read_bytes() + b"\n# notes about these hooks\n") + + integration, manifest = self._install(tmp_path, events) + assert "# notes about these hooks" in path.read_text(encoding="utf-8") + + remove_integration_events(integration, tmp_path, manifest) + assert not path.exists() + class TestVibeTomlNoOpRemoval: """Unowned Vibe hooks.toml files survive no-op cleanup byte-for-byte.""" @@ -549,6 +596,73 @@ def test_removal_recognizes_valid_marker_with_toml_line_endings(self, tmp_path, assert removed_file is False assert path.read_bytes() == user.encode() + @pytest.mark.parametrize( + ("managed", "user"), + ( + ( + '[[hooks]]\nname = "managed"\nspeckit_marker = true\n\n', + "# NOTE: keep guard-bash first\n" + "[[hooks]]\n" + 'name = "user-hook"\n' + 'command = "guard-bash"\n', + ), + # Blank lines after a kept comment stay with it. + ( + '[[hooks]]\nname = "managed"\nspeckit_marker = true\n\n', + '# NOTE: keep guard-bash first\n\n\n[[hooks]]\nname = "user-hook"\n', + ), + # A comment followed by more keys of the managed block is inside it. + ( + '[[hooks]]\nname = "managed"\n# inside\n\nspeckit_marker = true\n\n', + '[[hooks]]\nname = "user-hook"\n', + ), + ), + ids=( + "comment-before-user-hook", + "blank-lines-after-comment", + "comment-between-managed-keys", + ), + ) + @pytest.mark.parametrize("newline", ("\n", "\r\n"), ids=("lf", "crlf")) + def test_removal_ends_a_managed_hook_at_its_last_toml_line( + self, tmp_path, managed, user, newline + ): + """The blank separator goes with the managed block; a comment after it stays (#5079).""" + managed = managed.replace("\n", newline) + user = user.replace("\n", 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_still_takes_comment_like_lines_inside_managed_values(self, tmp_path): + """Lines inside a managed multiline string or array belong to its block.""" + managed = ( + "[[hooks]]\n" + 'name = "managed"\n' + "speckit_marker = true\n" + "args = [\n" + " # part of the array\n" + ' "a",\n' + "]\n" + 'command = """\n' + "# part of the command\n" + '"""\n' + ) + user = '[[hooks]]\nname = "user-hook"\n' + 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", "'''")):