fix: encode file paths for DIAL Core requests

src/quickapp/common/state_holder.py

@@ -1,6 +1,7 @@
 import logging
 from hashlib import sha256
 from typing import Any
+from urllib.parse import quote, unquote
 
 from aidial_client.types.metadata import FileMetadata
 from aidial_sdk.chat_completion import Attachment, ToolCall
@@ -54,4 +55,9 @@ def invalidate_file_data(self, url: str) -> None:
 
     @staticmethod
     def _get_file_key_by_url(url: str) -> str:
-        return sha256(url.encode('utf-8')).hexdigest()
+        # Normalize percent-encoding (unquote-then-quote is idempotent) so the
+        # encoded and decoded spellings of one URL share a single cache entry —
+        # the file cache is populated by several services (DialFileService,
+        # FileLoaderService) that may receive either spelling of the same file.
+        normalized = quote(unquote(url), safe="/")
+        return sha256(normalized.encode("utf-8")).hexdigest()

src/quickapp/dial_core_services/dial_file_service.py

@@ -2,6 +2,7 @@
 from collections import deque
 from pathlib import PurePosixPath
 from typing import Literal, NoReturn
+from urllib.parse import quote, unquote
 
 from aidial_client import AsyncDial
 from aidial_client._exception import DialException, ResourceNotFoundError
@@ -17,6 +18,19 @@
 logger = logging.getLogger(__name__)
 
 
+def _encode(url: str) -> str:
+    """Percent-encode a 'files/...' URL path for the DIAL Core API.
+
+    Unquote-then-quote makes this idempotent: a decoded input ('file name.pdf')
+    and an already-encoded one ('file%20name.pdf') both yield 'file%20name.pdf'.
+    Accepted edge case: a stored filename literally containing a valid '%XX'
+    sequence is indistinguishable from its encoded form and gets normalized —
+    this favors the overwhelmingly common case. `safe="/"` keeps path separators
+    (including a trailing '/', which distinguishes folders) intact.
+    """
+    return quote(unquote(url), safe="/")
+
+
 class FolderEntry(BaseModel):
     model_config = ConfigDict(frozen=True)
 
@@ -27,6 +41,13 @@ class FolderEntry(BaseModel):
 
 @inject
 class DialFileService:
+    """DIAL Core file API wrapper.
+
+    URL contract: decoded (human-readable) 'files/...' URLs in and out.
+    Percent-encoding for the wire is an internal egress concern (`_encode`);
+    ingress URLs from Core (`list_folder` items) are decoded before being
+    returned, so callers never handle encoded spellings.
+    """
 
     def __init__(
         self,
@@ -63,12 +84,14 @@ async def _get_metadata(self, url: str) -> FileMetadata:
         pass pre-resolved relative 'files/...' URLs, so get_api_path normalization is moot.
         """
         try:
-            return await self.__dial_client.metadata.get("files", url)
+            return await self.__dial_client.metadata.get("files", _encode(url))
         except DialException as e:
             self._reraise_404_as_not_found(e)
 
     async def download_file(self, file_url: str) -> tuple[bytes, FileMetadata | None]:
         logger.debug(f"File url to download url:{file_url}")
+        # StateHolder normalizes percent-encoding in its cache keys, so encoded
+        # and decoded spellings of one file share a single entry.
         file_data = self.__state_holder.get_file_data(url=file_url)
         if file_data is not None:
             return file_data, self.__state_holder.get_file_metadata(file_url)
@@ -80,7 +103,9 @@ async def download_file(self, file_url: str) -> tuple[bytes, FileMetadata | None
                 raise ValueError(
                     f"File size {size} exceeds the limit of {self.__content_size_limit} bytes."
                 )
-            file_data = await (await self.__dial_client.files.download(file_url)).aget_content()
+            file_data = await (
+                await self.__dial_client.files.download(_encode(file_url))
+            ).aget_content()
             self.__state_holder.store_file_data(file_url, file_data, metadata)
         except Exception as e:
             logger.error("Failed to download: %s", file_url, exc_info=True)
@@ -127,18 +152,20 @@ async def _upload_text(
         if_none_match: Literal["*"] | None = None,
         if_match: str | None = None,
     ) -> str:
-        encoded = content.encode("utf-8")
-        filename = url.split("/")[-1]
+        content_bytes = content.encode("utf-8")
+        filename = unquote(url).split("/")[-1]
         metadata = await self.__dial_client.files.upload(
-            url=url,
-            file=(filename, encoded, content_type),
+            url=_encode(url),
+            file=(filename, content_bytes, content_type),
             etag_if_none_match=if_none_match,
             etag_if_match=if_match,
         )
-        return metadata.url
+        # Core echoes the encoded URL back; return the decoded spelling per the
+        # class contract.
+        return unquote(metadata.url)
 
     async def delete(self, file_url: str) -> None:
-        await self.__dial_client.files.delete(file_url)
+        await self.__dial_client.files.delete(_encode(file_url))
         self.invalidate_cache(file_url)
 
     def invalidate_cache(self, file_url: str) -> None:
@@ -160,7 +187,9 @@ async def list_folder(self, folder_url: str, max_depth: int = 1) -> list[FolderE
             items: list[FileItem] = metadata.items or []
             for item in items:
                 is_folder = item.node_type == "FOLDER"
-                item_url = item.url
+                # Core returns percent-encoded item URLs; expose the decoded
+                # spelling per the class contract.
+                item_url = unquote(item.url)
                 if is_folder and not item_url.endswith("/"):
                     item_url = item_url + "/"
                 results.append(
@@ -175,16 +204,18 @@ async def list_folder(self, folder_url: str, max_depth: int = 1) -> list[FolderE
         return results
 
     async def copy(self, source_url: str, destination_url: str, overwrite: bool) -> None:
+        # copy_to/move_to send the URLs in an ops/resource JSON body that the
+        # HTTP client never encodes, so both must be encoded here (#447).
         await self.__dial_client.files.copy_to(
-            source=source_url,
-            destination=destination_url,
+            source=_encode(source_url),
+            destination=_encode(destination_url),
             overwrite=overwrite,
         )
 
     async def move(self, source_url: str, destination_url: str, overwrite: bool) -> None:
         await self.__dial_client.files.move_to(
-            source=source_url,
-            destination=destination_url,
+            source=_encode(source_url),
+            destination=_encode(destination_url),
             overwrite=overwrite,
         )
         self.invalidate_cache(source_url)

src/tests/unit_tests/common/test_state_holder.py

@@ -0,0 +1,31 @@
+from unittest.mock import MagicMock
+
+from quickapp.common.state_holder import StateHolder
+
+
+class TestFileCacheKeyNormalization:
+    def test_encoded_and_decoded_spellings_share_one_entry(self):
+        holder = StateHolder()
+        metadata = MagicMock()
+
+        holder.store_file_data("files/b/file name (1).pdf", b"data", metadata)
+
+        assert holder.get_file_data(url="files/b/file%20name%20%281%29.pdf") == b"data"
+        assert holder.get_file_metadata("files/b/file%20name%20%281%29.pdf") is metadata
+
+    def test_invalidate_accepts_either_spelling(self):
+        holder = StateHolder()
+
+        holder.store_file_data("files/b/file name.pdf", b"data")
+        holder.invalidate_file_data("files/b/file%20name.pdf")
+
+        assert holder.get_file_data(url="files/b/file name.pdf") is None
+
+    def test_distinct_urls_keep_distinct_entries(self):
+        holder = StateHolder()
+
+        holder.store_file_data("files/b/a.txt", b"a")
+        holder.store_file_data("files/b/b.txt", b"b")
+
+        assert holder.get_file_data(url="files/b/a.txt") == b"a"
+        assert holder.get_file_data(url="files/b/b.txt") == b"b"

src/tests/unit_tests/dial_core_services_tests/test_dial_file_service.py

@@ -29,6 +29,9 @@ def _make_mock_dial_client(
     mock_files = MagicMock()
     mock_files.download = AsyncMock(return_value=mock_download_result)
     mock_files.upload = AsyncMock(return_value=mock_metadata)
+    mock_files.delete = AsyncMock(return_value=None)
+    mock_files.copy_to = AsyncMock(return_value=None)
+    mock_files.move_to = AsyncMock(return_value=None)
 
     # The service fetches file/folder metadata via metadata.get (not files.get_metadata).
     mock_metadata_resource = MagicMock()
@@ -233,6 +236,143 @@ async def test_subsequent_download_hits_network_after_invalidation(self):
         assert second == file_bytes_v2
 
 
+def _make_folder_item(
+    url: str, node_type: str = "ITEM", content_length: int | None = 5
+) -> MagicMock:
+    item = MagicMock()
+    item.node_type = node_type
+    item.url = url
+    item.content_length = content_length
+    return item
+
+
+def _make_folder_metadata(items: list[MagicMock]) -> MagicMock:
+    metadata = MagicMock()
+    metadata.node_type = "FOLDER"
+    metadata.items = items
+    return metadata
+
+
+class TestUrlEncoding:
+    @pytest.mark.asyncio
+    async def test_download_encodes_decoded_path_with_spaces_and_parens(self):
+        mock_client = _make_mock_dial_client()
+        svc = _make_service(dial_client=mock_client)
+
+        await svc.download_file("files/b/Uno-Rules-PDF-Official-Rules-unorules.org_ (1).pdf")
+
+        encoded = "files/b/Uno-Rules-PDF-Official-Rules-unorules.org_%20%281%29.pdf"
+        mock_client.metadata.get.assert_awaited_once_with("files", encoded)
+        mock_client.files.download.assert_awaited_once_with(encoded)
+
+    @pytest.mark.asyncio
+    async def test_download_does_not_double_encode_encoded_input(self):
+        mock_client = _make_mock_dial_client()
+        svc = _make_service(dial_client=mock_client)
+
+        await svc.download_file("files/b/file%20name.pdf")
+
+        mock_client.files.download.assert_awaited_once_with("files/b/file%20name.pdf")
+
+    @pytest.mark.asyncio
+    async def test_metadata_lookup_preserves_trailing_slash(self):
+        mock_client = _make_mock_dial_client()
+        mock_client.metadata.get = AsyncMock(return_value=_make_folder_metadata([]))
+        svc = _make_service(dial_client=mock_client)
+
+        await svc.list_folder("files/b/my folder/")
+
+        mock_client.metadata.get.assert_awaited_once_with("files", "files/b/my%20folder/")
+
+    @pytest.mark.asyncio
+    async def test_delete_encodes_url(self):
+        mock_client = _make_mock_dial_client()
+        svc = _make_service(dial_client=mock_client)
+
+        await svc.delete("files/b/report (final).txt")
+
+        mock_client.files.delete.assert_awaited_once_with("files/b/report%20%28final%29.txt")
+
+    @pytest.mark.asyncio
+    async def test_copy_encodes_source_and_destination(self):
+        mock_client = _make_mock_dial_client()
+        svc = _make_service(dial_client=mock_client)
+
+        await svc.copy("files/b/src (1).pdf", "files/b/dst (1).pdf", overwrite=False)
+
+        mock_client.files.copy_to.assert_awaited_once_with(
+            source="files/b/src%20%281%29.pdf",
+            destination="files/b/dst%20%281%29.pdf",
+            overwrite=False,
+        )
+
+    @pytest.mark.asyncio
+    async def test_move_encodes_source_and_destination(self):
+        mock_client = _make_mock_dial_client()
+        svc = _make_service(dial_client=mock_client)
+
+        await svc.move("files/b/src (1).pdf", "files/b/dst (1).pdf", overwrite=True)
+
+        mock_client.files.move_to.assert_awaited_once_with(
+            source="files/b/src%20%281%29.pdf",
+            destination="files/b/dst%20%281%29.pdf",
+            overwrite=True,
+        )
+
+    @pytest.mark.asyncio
+    async def test_upload_encodes_url_and_keeps_decoded_filename(self):
+        mock_client = _make_mock_dial_client(upload_url="files/b/my%20notes.txt")
+        svc = _make_service(dial_client=mock_client)
+
+        result = await svc.write_file(url="files/b/my notes.txt", content="hi", overwrite=True)
+
+        call_kwargs = mock_client.files.upload.call_args.kwargs
+        assert call_kwargs["url"] == "files/b/my%20notes.txt"
+        # Multipart filename stays human-readable.
+        assert call_kwargs["file"][0] == "my notes.txt"
+        # Core echoes the encoded URL; the service returns it decoded.
+        assert result == "files/b/my notes.txt"
+
+    @pytest.mark.asyncio
+    async def test_cache_hit_across_encoded_and_decoded_spellings(self):
+        mock_client = _make_mock_dial_client(file_content=b"once")
+        svc = _make_service(dial_client=mock_client)
+
+        first, _ = await svc.download_file("files/b/file name.pdf")
+        second, _ = await svc.download_file("files/b/file%20name.pdf")
+
+        assert first == second == b"once"
+        mock_client.files.download.assert_awaited_once()
+
+    @pytest.mark.asyncio
+    async def test_invalidate_cache_accepts_either_spelling(self):
+        mock_client = _make_mock_dial_client(file_content=b"v1")
+        svc = _make_service(dial_client=mock_client)
+
+        await svc.download_file("files/b/file name.pdf")
+        svc.invalidate_cache("files/b/file%20name.pdf")
+        await svc.download_file("files/b/file name.pdf")
+
+        assert mock_client.files.download.await_count == 2
+
+    @pytest.mark.asyncio
+    async def test_list_folder_returns_decoded_entry_urls(self):
+        mock_client = _make_mock_dial_client()
+        mock_client.metadata.get = AsyncMock(
+            return_value=_make_folder_metadata(
+                [
+                    _make_folder_item("files/b/My%20Report%20%281%29.pdf"),
+                    _make_folder_item("files/b/sub%20dir", node_type="FOLDER", content_length=None),
+                ]
+            )
+        )
+        svc = _make_service(dial_client=mock_client)
+
+        entries = await svc.list_folder("files/b/")
+
+        assert [e.url for e in entries] == ["files/b/My Report (1).pdf", "files/b/sub dir/"]
+
+
 class TestGrantPermissions:
     @pytest.mark.asyncio
     async def test_grant_permissions_calls_client(self):

src/tests/unit_tests/dial_files_tooling/test_find_files_tool.py

@@ -113,6 +113,19 @@ async def test_non_root_folder_not_found_raises(self):
         assert "folder not found: missing/" in exc.value.message
         assert "appbucket" not in exc.value.message
 
+    @pytest.mark.asyncio
+    async def test_space_bearing_name_matches_human_readable_glob(self):
+        # DialFileService returns decoded entry URLs, so globs match the
+        # human-readable spelling — no percent-encoding leaks into matching.
+        entries = [
+            _file("files/appbucket/Uno-Rules-PDF-Official-Rules-unorules.org_ (1).pdf"),
+            _file("files/appbucket/other.pdf"),
+        ]
+        tool = _make_tool(entries=entries)
+        result = await tool._run_in_stage_async(stage_wrapper=None, pattern="*(1).pdf")
+        assert "Uno-Rules-PDF-Official-Rules-unorules.org_ (1).pdf" in result.content
+        assert "other.pdf" not in result.content
+
     @pytest.mark.asyncio
     async def test_no_matches_returns_no_files(self):
         tool = _make_tool(entries=[_file("files/appbucket/a.txt")])