From 801bf108faa406b87bfb9a1e9cf3724b47299400 Mon Sep 17 00:00:00 2001 From: Chris Coutinho Date: Thu, 11 Jun 2026 06:21:36 +0200 Subject: [PATCH] test(webdav): pin encode-once contract + nit cleanups (#891 r3) Round-3 review on PR #891 (no blockers): - Add test_encode_dav_path_encodes_exactly_once pinning the documented decoded-input precondition ("already%20encoded.pdf" -> "already%2520..."). - format_exception_group: proper singular/plural ("1 sub-exception" vs "N sub-exceptions") instead of "(s)". - oauth_sync: use `if doc_task is not None:` to match processor_task's guard. Co-Authored-By: Claude Opus 4.8 (1M context) --- nextcloud_mcp_server/vector/_errors.py | 3 ++- nextcloud_mcp_server/vector/oauth_sync.py | 4 ++-- tests/unit/client/test_webdav.py | 9 +++++++++ tests/unit/vector/test_errors.py | 2 +- 4 files changed, 14 insertions(+), 4 deletions(-) diff --git a/nextcloud_mcp_server/vector/_errors.py b/nextcloud_mcp_server/vector/_errors.py index 6e526317..f2cddc79 100644 --- a/nextcloud_mcp_server/vector/_errors.py +++ b/nextcloud_mcp_server/vector/_errors.py @@ -21,7 +21,8 @@ def format_exception_group(exc: BaseException) -> str: if not isinstance(exc, BaseExceptionGroup): return repr(exc) leaves = _flatten(exc) - return f"{len(leaves)} sub-exception(s): " + "; ".join(repr(e) for e in leaves) + noun = "sub-exception" if len(leaves) == 1 else "sub-exceptions" + return f"{len(leaves)} {noun}: " + "; ".join(repr(e) for e in leaves) def _flatten(exc: BaseException) -> list[BaseException]: diff --git a/nextcloud_mcp_server/vector/oauth_sync.py b/nextcloud_mcp_server/vector/oauth_sync.py index 6ce6b956..6cf44564 100644 --- a/nextcloud_mcp_server/vector/oauth_sync.py +++ b/nextcloud_mcp_server/vector/oauth_sync.py @@ -332,7 +332,7 @@ async def multi_user_processor_task( break except NotProvisionedError: - if doc_task: + if doc_task is not None: logger.warning( "[BasicAuth] User %s not provisioned, skipping %s_%s", doc_task.user_id, @@ -342,7 +342,7 @@ async def multi_user_processor_task( continue except Exception as e: - if doc_task: + if doc_task is not None: logger.error( "[BasicAuth] Processor %s error processing %s_%s: %s", worker_id, diff --git a/tests/unit/client/test_webdav.py b/tests/unit/client/test_webdav.py index cf8a3871..ffa7c6f2 100644 --- a/tests/unit/client/test_webdav.py +++ b/tests/unit/client/test_webdav.py @@ -540,6 +540,15 @@ def test_webdav_path_encoding(path, expected): assert client._webdav_path(path) == expected +@pytest.mark.unit +def test_encode_dav_path_encodes_exactly_once(): + """Pins the decoded-input precondition: a literal '%' becomes '%25', so an + already-encoded path passed in error would double-encode (caught here).""" + from nextcloud_mcp_server.client.webdav import _encode_dav_path + + assert _encode_dav_path("already%20encoded.pdf") == "already%2520encoded.pdf" + + @pytest.mark.unit async def test_read_file_encodes_special_chars(mocker): """read_file must percent-encode '#', commas, and spaces in the path (card 309). diff --git a/tests/unit/vector/test_errors.py b/tests/unit/vector/test_errors.py index f3f7f0e4..8c5dfe22 100644 --- a/tests/unit/vector/test_errors.py +++ b/tests/unit/vector/test_errors.py @@ -41,4 +41,4 @@ def test_format_nested_exception_group_flattens_all_leaves(): assert "ValueError" in formatted assert "ConnectError" in formatted assert "RuntimeError" in formatted - assert "3 sub-exception(s)" in formatted + assert "3 sub-exceptions" in formatted