From e1e8a09cab96305f5b97588a5ca27ce160370cc9 Mon Sep 17 00:00:00 2001 From: kartsan03 Date: Sat, 24 Oct 2026 17:48:08 +0000 Subject: [PATCH 1/6] fix(vibe): keep user comments that follow a managed hook block A managed [[hooks]] block ran until the next table header or the end of the file, so the comments after it were removed with it on every events refresh and teardown. A comment there usually introduces the user hook that follows it. End the block after its last line of TOML and the blank lines right after it. Lines inside a multiline string or array still belong to the value. Refs #5079 Assisted-by: Claude Code (model: claude-opus-5-5, autonomous) --- src/specify_cli/integrations/vibe/__init__.py | 20 ++++- tests/integrations/test_integration_vibe.py | 75 +++++++++++++++++++ 2 files changed, 93 insertions(+), 2 deletions(-) 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..30edc7e904 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")) + + 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.""" @@ -549,6 +582,48 @@ def test_removal_recognizes_valid_marker_with_toml_line_endings(self, tmp_path, assert removed_file is False assert path.read_bytes() == user.encode() + def test_removal_keeps_a_comment_that_follows_a_managed_hook(self, tmp_path): + """The blank separator goes with the managed block; the comment stays (#5079).""" + managed = '[[hooks]]\nname = "managed"\nspeckit_marker = true\n\n' + user = ( + "# NOTE: keep guard-bash first\n" + "[[hooks]]\n" + 'name = "user-hook"\n' + 'command = "guard-bash"\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_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" + 'command = """\n' + "# part of the command\n" + '"""\n' + "args = [\n" + " # part of the array\n" + ' "a",\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", "'''")): From 76867c310e3af4b9a57a3a581af2c4f841b65bb2 Mon Sep 17 00:00:00 2001 From: kartsan03 Date: Mon, 26 Oct 2026 14:44:37 +0000 Subject: [PATCH 2/6] test(vibe): cover the comment boundaries of a managed hook block The exact-byte removal test covered one comment right after a managed block. Add the two boundaries the comment-state logic decides: blank lines after a kept comment stay with it, and a comment followed by more keys of the managed block is removed with the block. Refs #5079 Assisted-by: Claude Code (model: claude-opus-5-5, autonomous) --- tests/integrations/test_integration_vibe.py | 40 ++++++++++++++++----- 1 file changed, 31 insertions(+), 9 deletions(-) diff --git a/tests/integrations/test_integration_vibe.py b/tests/integrations/test_integration_vibe.py index 30edc7e904..42ffc2952c 100644 --- a/tests/integrations/test_integration_vibe.py +++ b/tests/integrations/test_integration_vibe.py @@ -582,15 +582,37 @@ def test_removal_recognizes_valid_marker_with_toml_line_endings(self, tmp_path, assert removed_file is False assert path.read_bytes() == user.encode() - def test_removal_keeps_a_comment_that_follows_a_managed_hook(self, tmp_path): - """The blank separator goes with the managed block; the comment stays (#5079).""" - managed = '[[hooks]]\nname = "managed"\nspeckit_marker = true\n\n' - user = ( - "# NOTE: keep guard-bash first\n" - "[[hooks]]\n" - 'name = "user-hook"\n' - 'command = "guard-bash"\n' - ) + @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", + ), + ) + def test_removal_ends_a_managed_hook_at_its_last_toml_line( + self, tmp_path, managed, user + ): + """The blank separator goes with the managed block; a comment after it stays (#5079).""" path, _ = self._user_hooks_file(tmp_path, (managed + user).encode()) removed_file = get_integration("vibe").remove_vibe_event_hooks( From 4c78efd1ed4e436aa1f39bdbbf831bbdeb52c04b Mon Sep 17 00:00:00 2001 From: kartsan03 Date: Mon, 26 Oct 2026 15:22:14 +0000 Subject: [PATCH 3/6] test(vibe): end the multiline-value control on the string; document cleanup The control passed even without the multiline-string guard, because the array after the string extended the managed block anyway. Put the string last so a missing guard leaves its closing lines behind and the test fails. Add a Runtime Events note to the Vibe guide: cleanup touches only the entries Spec Kit wrote, an entry ends after its last TOML line and the blank lines after it, and the file is deleted when only comments and blank lines remain. Refs #5079 Assisted-by: Claude Code (model: claude-opus-5-5, autonomous) --- docs/integrations/vibe.md | 6 ++++++ tests/integrations/test_integration_vibe.py | 6 +++--- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/docs/integrations/vibe.md b/docs/integrations/vibe.md index c359bd54e5..520f4aa2ed 100644 --- a/docs/integrations/vibe.md +++ b/docs/integrations/vibe.md @@ -36,6 +36,12 @@ 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 stays in +the file. If removing Spec Kit's entries leaves only comments and blank lines, +Spec Kit deletes `.vibe/hooks.toml`. + ## Workflow Dispatch Workflow steps use `vibe -p ` and request `--output json` for structured diff --git a/tests/integrations/test_integration_vibe.py b/tests/integrations/test_integration_vibe.py index 42ffc2952c..cf2b391ea1 100644 --- a/tests/integrations/test_integration_vibe.py +++ b/tests/integrations/test_integration_vibe.py @@ -628,13 +628,13 @@ def test_removal_still_takes_comment_like_lines_inside_managed_values(self, tmp_ "[[hooks]]\n" 'name = "managed"\n' "speckit_marker = true\n" - 'command = """\n' - "# part of the command\n" - '"""\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()) From 3ad691760faac7758ce363f18bec83b5ded6e683 Mon Sep 17 00:00:00 2001 From: kartsan03 Date: Mon, 26 Oct 2026 18:06:04 +0000 Subject: [PATCH 4/6] test(vibe): run the comment-boundary cases with CRLF too The exact-byte removal cases ran only with LF. Run them with CRLF as well; they fail if the scanner keeps the CR in a line's content. The refresh test now tears down with the integration and manifest returned by the refresh call. Refs #5079 Assisted-by: Claude Code (model: claude-opus-5-5, autonomous) --- tests/integrations/test_integration_vibe.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/tests/integrations/test_integration_vibe.py b/tests/integrations/test_integration_vibe.py index cf2b391ea1..7a7938657a 100644 --- a/tests/integrations/test_integration_vibe.py +++ b/tests/integrations/test_integration_vibe.py @@ -339,7 +339,7 @@ def test_refresh_and_teardown_keep_comments_after_a_managed_hook( integration, manifest = self._install(tmp_path, events) path.write_bytes(path.read_bytes() + ("\n" + user_after).encode("utf-8")) - self._install(tmp_path, events) + integration, manifest = self._install(tmp_path, events) assert user_after in path.read_text(encoding="utf-8") remove_integration_events(integration, tmp_path, manifest) @@ -609,10 +609,13 @@ def test_removal_recognizes_valid_marker_with_toml_line_endings(self, tmp_path, "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 + 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( From 77147cc3b13fb0b91700adc4e019d45db7775607 Mon Sep 17 00:00:00 2001 From: kartsan03 Date: Mon, 26 Oct 2026 21:25:22 +0000 Subject: [PATCH 5/6] docs(vibe): say that comments go with a deleted hooks file The Runtime Events note said cleanup deletes .vibe/hooks.toml when only comments and blank lines remain, but not that those comments are lost with it, or that Codex's config.toml follows the same rule. Refs #5079 Assisted-by: Claude Code (model: claude-opus-5-5, autonomous) --- docs/integrations/vibe.md | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/docs/integrations/vibe.md b/docs/integrations/vibe.md index 520f4aa2ed..f211ab6d64 100644 --- a/docs/integrations/vibe.md +++ b/docs/integrations/vibe.md @@ -38,9 +38,11 @@ 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 stays in -the file. If removing Spec Kit's entries leaves only comments and blank lines, -Spec Kit deletes `.vibe/hooks.toml`. +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 From 8dbc5764c72b20767b42a54dd6d231eb4aeb4326 Mon Sep 17 00:00:00 2001 From: kartsan03 Date: Mon, 26 Oct 2026 22:52:34 +0000 Subject: [PATCH 6/6] test(vibe): pin teardown when only a trailing comment remains The trailing-comment case always kept a user hook in the file, so no test covered the documented comments-only case. Add one: the comment survives a refresh, and teardown deletes .vibe/hooks.toml. Refs #5079 Assisted-by: Claude Code (model: claude-opus-5-5, autonomous) --- tests/integrations/test_integration_vibe.py | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/tests/integrations/test_integration_vibe.py b/tests/integrations/test_integration_vibe.py index 7a7938657a..61a06cd183 100644 --- a/tests/integrations/test_integration_vibe.py +++ b/tests/integrations/test_integration_vibe.py @@ -497,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."""