relaxed the de-dup failure

Author: vijay

src/mcp/client/skills.py

@@ -92,10 +92,12 @@ def _require_extension(self, *, directory_read: bool = False) -> None:
     async def list_skills(self, params: ListSkillsParams | None = None) -> list[Skill]:
         """Call `skills/list`, following `nextCursor` to completion, and validate the result.
 
+        A skill a changing catalog surfaces on more than one page is returned once, in the order
+        first seen.
+
         Raises:
-            ValueError: If the server doesn't advertise the Skills extension, its
-                response is not SEP-2640 conformant, it repeats a pagination
-                cursor, or it lists the same skill URI on two pages.
+            ValueError: If the server doesn't advertise the Skills extension, its response is not
+                SEP-2640 conformant, or it repeats a pagination cursor.
             MCPError: If the server returns an error response.
         """
         self._require_extension()
@@ -113,12 +115,12 @@ async def list_skills(self, params: ListSkillsParams | None = None) -> list[Skil
             # A server stuck repeating its cursor is a loop; catch that before the content checks.
             if page.next_cursor is not None and page.next_cursor in seen_cursors:
                 raise ValueError(f"server repeated skills/list pagination cursor {page.next_cursor!r}")
-            # Per-page validation can't catch a URI repeated *across* pages, so track that here.
+            # A catalog that changes between page fetches can legitimately repeat a skill across
+            # pages; keep the first occurrence rather than treating it as an error.
             for skill in page.skills:
-                if skill.uri in seen_uris:
-                    raise ValueError(f"server listed skill {skill.uri!r} on more than one skills/list page")
-                seen_uris.add(skill.uri)
-            skills.extend(page.skills)
+                if skill.uri not in seen_uris:
+                    seen_uris.add(skill.uri)
+                    skills.append(skill)
             if page.next_cursor is None:
                 return skills
             seen_cursors.add(page.next_cursor)
@@ -162,10 +164,12 @@ async def read_skill_uri(self, uri: str) -> ReadResourceResult:
     async def read_directory(self, uri: str, params: ReadDirectoryParams | None = None) -> list[Resource]:
         """Call `resources/directory/read` for `uri`, following `nextCursor` to completion.
 
+        A child a changing directory surfaces on more than one page is returned once, in the order
+        first seen.
+
         Raises:
-            ValueError: If the server doesn't advertise the `directoryRead`
-                setting, its response is not a valid child listing of `uri`, it
-                repeats a pagination cursor, or it lists the same child on two pages.
+            ValueError: If the server doesn't advertise the `directoryRead` setting, its response
+                is not a valid child listing of `uri`, or it repeats a pagination cursor.
             MCPError: If the server returns an error response.
         """
         self._require_extension(directory_read=True)
@@ -183,12 +187,11 @@ async def read_directory(self, uri: str, params: ReadDirectoryParams | None = No
             if page.next_cursor is not None and page.next_cursor in seen_cursors:
                 raise ValueError(f"server repeated resources/directory/read pagination cursor {page.next_cursor!r}")
             validate_directory_result(uri, page)
-            # `validate_directory_result` dedupes within a page; catch a child repeated across pages.
+            # A changing directory can legitimately repeat a child across pages; keep the first.
             for resource in page.resources:
-                if resource.uri in seen_uris:
-                    raise ValueError(f"server listed child {resource.uri!r} on more than one directory page")
-                seen_uris.add(resource.uri)
-            resources.extend(page.resources)
+                if resource.uri not in seen_uris:
+                    seen_uris.add(resource.uri)
+                    resources.append(resource)
             if page.next_cursor is None:
                 return resources
             seen_cursors.add(page.next_cursor)

src/mcp/server/skills.py

@@ -77,9 +77,8 @@ class Skills(Extension):
 
     Handler error contract: raise `MCPError` to return a specific error to the
     caller (e.g. `INVALID_PARAMS` from `get_skill` for a URI it doesn't serve).
-    Any other failure — a raised exception, or a result that isn't SEP-2640
-    conformant — is treated as a server fault and reported as `INTERNAL_ERROR`,
-    never the caller's bad params.
+    A result that isn't SEP-2640 conformant is caught here and reported as
+    `INTERNAL_ERROR` — a server fault, not the caller's bad params.
     """
 
     identifier = EXTENSION_ID

tests/client/test_skills.py

@@ -160,9 +160,9 @@ async def test_list_skills_raises_on_a_server_that_repeats_its_cursor() -> None:
             await skills.list_skills()
 
 
-async def test_list_skills_rejects_a_skill_repeated_across_pages() -> None:
-    """SEP-2640 pagination: a page's own validation dedupes within that page, so the client also
-    rejects a skill URI the server hands back on a second page rather than returning it twice."""
+async def test_list_skills_collapses_a_skill_repeated_across_pages() -> None:
+    """A catalog that changes between page fetches can surface the same skill on two pages; the
+    client returns it once (first occurrence) rather than failing or duplicating it."""
 
     async def handler(ctx: ServerRequestContext[Any, Any], params: ListSkillsParams) -> ListSkillsResult:
         if params.cursor is None:
@@ -171,8 +171,8 @@ async def handler(ctx: ServerRequestContext[Any, Any], params: ListSkillsParams)
 
     server = MCPServer("catalog", extensions=[Skills(list_skills=handler, get_skill=_get_skill)])
     async with _skills(server) as skills:
-        with pytest.raises(ValueError, match="more than one skills/list page"):
-            await skills.list_skills()
+        result = await skills.list_skills()
+    assert [s.uri for s in result] == [_SKILL_URI]
 
 
 async def test_list_skills_requires_the_extension_to_be_advertised() -> None:
@@ -245,8 +245,8 @@ async def test_read_directory_raises_on_a_server_that_repeats_its_cursor() -> No
             await skills.read_directory("skill://git-workflow/references")
 
 
-async def test_read_directory_rejects_a_child_repeated_across_pages() -> None:
-    """The client dedupes directory children across pages, not just within a single page."""
+async def test_read_directory_collapses_a_child_repeated_across_pages() -> None:
+    """A changing directory can surface the same child on two pages; the client returns it once."""
     child = Resource(uri="skill://git-workflow/references/A.md", name="A.md")
 
     async def handler(ctx: ServerRequestContext[Any, Any], params: ReadDirectoryParams) -> ReadDirectoryResult:
@@ -259,8 +259,8 @@ async def handler(ctx: ServerRequestContext[Any, Any], params: ReadDirectoryPara
         extensions=[Skills(list_skills=_paginated_list_handler(), get_skill=_get_skill, read_directory=handler)],
     )
     async with _skills(server) as skills:
-        with pytest.raises(ValueError, match="more than one directory page"):
-            await skills.read_directory("skill://git-workflow/references")
+        resources = await skills.read_directory("skill://git-workflow/references")
+    assert [r.uri for r in resources] == ["skill://git-workflow/references/A.md"]
 
 
 async def test_get_skill_rejects_a_mismatched_uri_from_a_non_conformant_server() -> None: