diff --git a/hermes_cli/clipboard.py b/hermes_cli/clipboard.py index fa750d85..893a84d3 100644 --- a/hermes_cli/clipboard.py +++ b/hermes_cli/clipboard.py @@ -285,20 +285,27 @@ def _convert_to_png(path: Path) -> bool: logger.debug("Pillow BMP→PNG conversion failed: %s", e) # Fall back to ImageMagick convert + tmp = path.with_suffix(".bmp") try: - tmp = path.with_suffix(".bmp") path.rename(tmp) r = subprocess.run( ["convert", str(tmp), "png:" + str(path)], capture_output=True, timeout=5, ) - tmp.unlink(missing_ok=True) if r.returncode == 0 and path.exists() and path.stat().st_size > 0: + tmp.unlink(missing_ok=True) return True + else: + # Convert failed — restore the original file + tmp.rename(path) except FileNotFoundError: logger.debug("ImageMagick not installed — cannot convert BMP to PNG") + if not path.exists() and tmp.exists(): + tmp.rename(path) except Exception as e: logger.debug("ImageMagick BMP→PNG conversion failed: %s", e) + if not path.exists() and tmp.exists(): + tmp.rename(path) # Can't convert — BMP is still usable as-is for most APIs return path.exists() and path.stat().st_size > 0 diff --git a/tests/tools/test_clipboard.py b/tests/tools/test_clipboard.py index 1fb1a39e..dc064e6c 100644 --- a/tests/tools/test_clipboard.py +++ b/tests/tools/test_clipboard.py @@ -559,6 +559,51 @@ class TestConvertToPng: # (raw BMP is better than nothing) assert dest.exists() and dest.stat().st_size > 0 + def test_imagemagick_failure_preserves_original(self, tmp_path): + """When ImageMagick convert fails, the original file must not be lost.""" + dest = tmp_path / "img.png" + original_data = FAKE_BMP + dest.write_bytes(original_data) + + def fake_run_fail(cmd, **kw): + # Simulate convert failing without producing output + return MagicMock(returncode=1) + + with patch.dict(sys.modules, {"PIL": None, "PIL.Image": None}): + with patch("hermes_cli.clipboard.subprocess.run", side_effect=fake_run_fail): + _convert_to_png(dest) + + # Original file must still exist with original content + assert dest.exists(), "Original file was lost after failed conversion" + assert dest.read_bytes() == original_data + + def test_imagemagick_not_installed_preserves_original(self, tmp_path): + """When ImageMagick is not installed, the original file must not be lost.""" + dest = tmp_path / "img.png" + original_data = FAKE_BMP + dest.write_bytes(original_data) + + with patch.dict(sys.modules, {"PIL": None, "PIL.Image": None}): + with patch("hermes_cli.clipboard.subprocess.run", side_effect=FileNotFoundError): + _convert_to_png(dest) + + assert dest.exists(), "Original file was lost when ImageMagick not installed" + assert dest.read_bytes() == original_data + + def test_imagemagick_timeout_preserves_original(self, tmp_path): + """When ImageMagick times out, the original file must not be lost.""" + import subprocess + dest = tmp_path / "img.png" + original_data = FAKE_BMP + dest.write_bytes(original_data) + + with patch.dict(sys.modules, {"PIL": None, "PIL.Image": None}): + with patch("hermes_cli.clipboard.subprocess.run", side_effect=subprocess.TimeoutExpired("convert", 5)): + _convert_to_png(dest) + + assert dest.exists(), "Original file was lost after timeout" + assert dest.read_bytes() == original_data + # ── has_clipboard_image dispatch ─────────────────────────────────────────