diff --git a/tests/tools/test_open_preview_tool.py b/tests/tools/test_open_preview_tool.py index 3708a1aa23..2c852d750e 100644 --- a/tests/tools/test_open_preview_tool.py +++ b/tests/tools/test_open_preview_tool.py @@ -23,3 +23,60 @@ def test_emitter_failure_is_reported(): desktop_ui.set_emitter(_boom) assert "no window" in json.loads(op.open_preview_tool("https://x.example"))["error"] + + +def _capture_emits(): + emitted: list = [] + + def _emit(_sid, event, payload): + emitted.append((event, payload)) + + desktop_ui.set_emitter(_emit) + return emitted + + +def test_existing_directory_is_an_error_not_success(tmp_path): + """#95853: a directory must not report success while opening nothing.""" + emitted = _capture_emits() + folder = tmp_path / "Active" + folder.mkdir() + + result = json.loads(op.open_preview_tool(str(folder))) + + assert "error" in result + assert "director" in result["error"].lower() + assert result.get("success") is not True + assert emitted == [] + + +def test_existing_file_still_opens(tmp_path): + emitted = _capture_emits() + path = tmp_path / "notes.md" + path.write_text("hi", encoding="utf-8") + + result = json.loads(op.open_preview_tool(str(path))) + + assert result["success"] is True + assert result["url"] == str(path) + assert emitted == [("preview.open", {"url": str(path), "label": ""})] + + +def test_https_url_is_not_treated_as_a_directory(): + emitted = _capture_emits() + result = json.loads(op.open_preview_tool("https://example.com/docs")) + + assert result["success"] is True + assert emitted[0][0] == "preview.open" + + +def test_file_uri_directory_is_an_error(tmp_path): + emitted = _capture_emits() + folder = tmp_path / "docs" + folder.mkdir() + uri = folder.resolve().as_uri() + + result = json.loads(op.open_preview_tool(uri)) + + assert "error" in result + assert "director" in result["error"].lower() + assert emitted == [] diff --git a/tools/open_preview_tool.py b/tools/open_preview_tool.py index d40f53a34a..c026a5b128 100644 --- a/tools/open_preview_tool.py +++ b/tools/open_preview_tool.py @@ -6,7 +6,10 @@ the normalizer + open action. Emits ``preview.open`` via ``desktop_ui``: the ren the pane for the window that asked and never steals focus for a background session. """ +import os import re +from pathlib import Path +from urllib.parse import unquote, urlparse from tools import desktop_ui from tools.registry import tool_error @@ -28,6 +31,39 @@ def _normalize_target(raw: str) -> str: return v +def _local_fs_path(target: str) -> Path | None: + """Return a filesystem path for local targets; None for http(s) URLs.""" + raw = (target or "").strip() + if not raw: + return None + if "://" in raw: + parsed = urlparse(raw) + if parsed.scheme.lower() != "file": + return None + path = unquote(parsed.path or "") + if parsed.netloc and parsed.netloc not in {"", "localhost"}: + path = f"//{parsed.netloc}{path}" + elif ( + os.name == "nt" + and len(path) >= 3 + and path[0] == "/" + and path[2] == ":" + ): + path = path[1:] + return Path(path) if path else None + return Path(raw).expanduser() + + +def _is_existing_directory(target: str) -> bool: + path = _local_fs_path(target) + if path is None: + return False + try: + return path.is_dir() + except OSError: + return False + + def open_preview_tool(url: str, label: str = "") -> str: """Ask the desktop GUI to show ``url`` in the preview pane beside the chat.""" target = _normalize_target(url or "") @@ -35,6 +71,12 @@ def open_preview_tool(url: str, label: str = "") -> str: return tool_error( "url is required — a web URL (https://…), a localhost dev server, or a " "file path to show in the preview pane.") + if _is_existing_directory(target): + return tool_error( + "directories are not previewable — pass a file path or a URL. " + f"{target} is a directory." + ) + label = (label or "").strip() return desktop_ui.emit_or_error( "preview.open",