Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 15 additions & 3 deletions src/specify_cli/events.py
Original file line number Diff line number Diff line change
Expand Up @@ -2129,7 +2129,10 @@ def _merge_toml_fragment(dst: Path, fragment: str) -> bool:
An unreadable or undecodable pre-existing file aborts the merge instead
of discarding the user's bytes, mirroring ``_load_user_json`` (#22).
Returns False when skipped so callers avoid tracking the untouched file
(S5).
(S5) — including when there is no fragment to add and no owned blocks to
remove, so a no-op install doesn't rewrite (and, via text-mode newline
translation, mangle the line endings of) an untouched pre-existing file
(#4563).
"""
_ensure_safe_destination(dst)
existing = ""
Expand All @@ -2144,14 +2147,16 @@ def _merge_toml_fragment(dst: Path, fragment: str) -> bool:
)
logger.debug("Read error detail: %s", exc)
return False
existing = re.sub(
stripped = re.sub(
r'\[\[hooks\.\w+\]\]\n(?:(?!\[\[hooks\.\w+\]\]).)*?speckit_marker = true\n*',
"",
existing,
flags=re.DOTALL,
)
if not fragment and stripped == existing:
return False
dst.parent.mkdir(parents=True, exist_ok=True)
dst.write_text(existing.rstrip() + "\n\n" + fragment + "\n", encoding="utf-8")
dst.write_text(stripped.rstrip() + "\n\n" + fragment + "\n", encoding="utf-8")
return True


Expand Down Expand Up @@ -2195,6 +2200,11 @@ def _remove_toml_entries(dst: Path) -> bool:
"""Remove Specify-marked TOML entries; delete the file if now empty (#14).

Returns True if the file was deleted (no user content remained).

Leaves the file untouched (no write) when there are no Specify-owned
blocks to strip, so a no-op teardown/install doesn't rewrite (and, via
text-mode newline translation, mangle the line endings of) an untouched
pre-existing file (#4563).
"""
if not dst.exists():
return False
Expand Down Expand Up @@ -2230,6 +2240,8 @@ def _remove_toml_entries(dst: Path) -> bool:
if not stripped:
dst.unlink(missing_ok=True)
return True
if cleaned == existing:
return False
dst.write_text(cleaned, encoding="utf-8")
return False

Expand Down
48 changes: 48 additions & 0 deletions tests/integrations/test_events.py
Original file line number Diff line number Diff line change
Expand Up @@ -937,6 +937,54 @@ def test_teardown_skips_unreadable_config_and_preserves_bytes(self, tmp_path):
assert config_path.read_bytes() == user_bytes


class TestTomlNoOpMerge:
"""#4563: installing with no resolved events must leave a pre-existing,
Specify-unowned config.toml byte-for-byte untouched.

This is the real ``specify integration install codex`` repro: a project
with no Codex-specific event hooks configured resolves to ``events={}``
(see ``resolve_events``), which routes through
``install_integration_events``'s empty-map branch into
``_remove_native_event_hooks`` -> ``_remove_toml_entries`` — not through
``_merge_toml_fragment``, which only runs when there is at least one
supported, non-empty event to merge. ``_remove_toml_entries`` computed
``cleaned`` via a regex strip and then unconditionally called
``dst.write_text(cleaned, ...)`` even when ``cleaned == existing`` (no
Specify-marked blocks present), which (through Python's text-mode
newline translation on read/write) silently changed the file's
line-ending convention on Windows — turning a clean install into a
spurious git diff with no semantic change.
"""

def test_no_events_leaves_existing_config_untouched(self, tmp_path):
from specify_cli.integrations.codex import CodexIntegration

integration = CodexIntegration()
manifest = _claude_manifest(tmp_path)
config_path = tmp_path / ".codex" / "config.toml"
config_path.parent.mkdir(parents=True)
original_bytes = b"project_doc_max_bytes = 200000"
config_path.write_bytes(original_bytes)
mtime_before = config_path.stat().st_mtime_ns

# The real no-extensions-installed shape: resolve_events() returns an
# empty map when no built-in defaults, extensions, or overrides
# contribute any handlers.
install_integration_events(
integration, tmp_path, manifest,
{},
)

assert config_path.read_bytes() == original_bytes
# An unconditional rewrite can reproduce identical bytes on Linux
# (text-mode newline translation is a no-op when the platform line
# separator is already "\n"), so byte equality alone doesn't catch
# the defect here; assert the file was never even opened for
# writing, which is what actually mangles line endings on Windows.
assert config_path.stat().st_mtime_ns == mtime_before
manifest.record_existing.assert_not_called()


# -- Opencode TS Plugin merging ---------------------------------------------

class TestOpencodePluginMerging:
Expand Down