diff --git a/src/specify_cli/events.py b/src/specify_cli/events.py index 17a9d7ffbe..f61995c895 100644 --- a/src/specify_cli/events.py +++ b/src/specify_cli/events.py @@ -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 = "" @@ -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 @@ -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 @@ -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 diff --git a/tests/integrations/test_events.py b/tests/integrations/test_events.py index 0168f4302c..7476ec6531 100644 --- a/tests/integrations/test_events.py +++ b/tests/integrations/test_events.py @@ -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: