Skip to content

Commit 1ddace7

Browse files
author
vijay
committed
removed restriction to allow only 512 files and 16 mb for each skill, it's not mandatory as per recent update
1 parent cde5cff commit 1ddace7

3 files changed

Lines changed: 62 additions & 29 deletions

File tree

‎src/mcp/client/skills.py‎

Lines changed: 26 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -9,9 +9,12 @@
99
for skill in await skills.bind(client).list_skills():
1010
print(skill.uri, skill.frontmatter["description"])
1111
12-
`bind(client)` returns a `BoundSkills`. Each verb validates that the server
13-
advertises the extension before sending; `list_skills` and `read_directory`
14-
follow `nextCursor` to completion, so one call returns every page's results.
12+
`bind(client)` returns a `BoundSkills`. Its catalog verbs — `list_skills`,
13+
`get_skill`, and `read_directory` — check that the server advertises the
14+
extension and validate its response; `list_skills` and `read_directory` follow
15+
`nextCursor` to completion, so one call returns every page's results.
16+
`read_skill_uri` is a thin `resources/read` alias that does neither — verify its
17+
result with `verify_skill_resource`.
1518
"""
1619

1720
from __future__ import annotations
@@ -68,9 +71,13 @@ def bind(self, client: Client) -> BoundSkills:
6871
class BoundSkills:
6972
"""The SEP-2640 verbs bound to one connected session.
7073
71-
Obtain it from `Skills.bind(client)`. `list_skills` and `read_directory`
72-
follow `nextCursor` to completion; every method validates the server's
73-
response against the SEP-2640 conformance rules before returning it.
74+
Obtain it from `Skills.bind(client)`. The catalog verbs — `list_skills`,
75+
`get_skill`, and `read_directory` — check that the server advertises the
76+
extension and validate its response against the SEP-2640 conformance rules
77+
before returning; `list_skills` and `read_directory` also follow
78+
`nextCursor` to completion. `read_skill_uri` is the exception: a thin
79+
`resources/read` alias that neither checks advertisement nor validates —
80+
pair it with `verify_skill_resource`.
7481
"""
7582

7683
def __init__(self, session: ClientSession) -> None:
@@ -88,8 +95,9 @@ async def list_skills(self, params: ListSkillsParams | None = None) -> list[Skil
8895
"""Call `skills/list`, following `nextCursor` to completion, and validate the result.
8996
9097
Raises:
91-
ValueError: If the server doesn't advertise the Skills extension, or
92-
its response is not SEP-2640 conformant.
98+
ValueError: If the server doesn't advertise the Skills extension, its
99+
response is not SEP-2640 conformant, or it repeats a pagination cursor.
100+
MCPError: If the server returns an error response.
93101
"""
94102
self._require_extension()
95103
base = params if params is not None else ListSkillsParams()
@@ -118,6 +126,8 @@ async def get_skill(self, uri: str) -> Skill:
118126
Raises:
119127
ValueError: If the server doesn't advertise the Skills extension, its
120128
response names a different skill, or the skill is not conformant.
129+
MCPError: If the server returns an error response, such as `-32602`
130+
for a URI it does not serve.
121131
"""
122132
self._require_extension()
123133
result = await self._session.send_request(GetSkillRequest(params=GetSkillParams(uri=uri)), GetSkillResult)
@@ -133,6 +143,11 @@ async def read_skill_uri(self, uri: str) -> ReadResourceResult:
133143
file regardless of whether the skill was ever enumerated. Verify the
134144
result against a held `Skill` entry with `verify_skill_resource` before
135145
treating it as trusted content — this call does not verify anything itself.
146+
147+
Raises:
148+
MCPError: If the server returns an error response.
149+
RuntimeError: If the server returns an `InputRequiredResult`; this
150+
alias does not drive the input-required loop.
136151
"""
137152
return await self._session.read_resource(uri)
138153

@@ -141,7 +156,9 @@ async def read_directory(self, uri: str, params: ReadDirectoryParams | None = No
141156
142157
Raises:
143158
ValueError: If the server doesn't advertise the `directoryRead`
144-
setting, or its response is not a valid child listing of `uri`.
159+
setting, its response is not a valid child listing of `uri`, or
160+
it repeats a pagination cursor.
161+
MCPError: If the server returns an error response.
145162
"""
146163
self._require_extension(directory_read=True)
147164
base = params if params is not None else ReadDirectoryParams(uri=uri)

‎src/mcp/shared/skills.py‎

Lines changed: 19 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -24,10 +24,17 @@
2424
METHOD_READ_DIRECTORY = "resources/directory/read"
2525

2626
MAX_RESOURCES_PER_SKILL = 512
27-
"""SEP-2640 per-skill resource-count limit, `SKILL.md` included."""
27+
"""SEP-2640 per-skill resource-count threshold (`SKILL.md` included).
28+
29+
A SHOULD NOT limit, not a hard cap: the spec requires a host to support skills
30+
*up to and including* 512 entries and permits it to support larger ones, so
31+
`validate_skill` does not reject an over-count manifest."""
2832

2933
MAX_TOTAL_SIZE = 16 * 1024 * 1024
30-
"""SEP-2640 per-skill total-byte-size limit (16 MiB), summed over `resources[].size`."""
34+
"""SEP-2640 per-skill total-byte-size threshold (16 MiB), summed over `resources[].size`.
35+
36+
A SHOULD NOT limit, not a hard cap (see `MAX_RESOURCES_PER_SKILL`); an over-size
37+
manifest is not rejected."""
3138

3239
_NAME_RE = re.compile(r"^[a-z0-9]+(?:-[a-z0-9]+)*$")
3340
_DIGEST_RE = re.compile(r"^sha256:[0-9a-f]{64}$")
@@ -180,9 +187,11 @@ def validate_skill(skill: Skill) -> None:
180187
"""Validate `skill` against the SEP-2640 and Agent Skills conformance rules.
181188
182189
Checks the frontmatter's `name`/`description` fields, that `resources` (when
183-
not `"dynamic"`) is complete and within the `MAX_RESOURCES_PER_SKILL`/
184-
`MAX_TOTAL_SIZE` limits, and that every resource entry names a file within
185-
the skill's own directory with a well-formed digest.
190+
not `"dynamic"`) is complete — every entry names a file within the skill's
191+
own directory, has a well-formed digest, and `SKILL.md` is present. The
192+
512-entry/16-MiB limits are SEP-2640 SHOULD NOT thresholds, not MUST NOT, so
193+
an over-limit manifest is accepted (a conforming host must support up to the
194+
limits and may support larger).
186195
187196
Raises:
188197
ValueError: If `skill` violates any of the above.
@@ -200,10 +209,7 @@ def validate_skill(skill: Skill) -> None:
200209
if skill.resources == "dynamic":
201210
return
202211
resources = skill.resources
203-
if len(resources) > MAX_RESOURCES_PER_SKILL:
204-
raise ValueError(f"skill {skill.uri!r} has {len(resources)} resources, exceeding {MAX_RESOURCES_PER_SKILL}")
205212
seen: set[str] = set()
206-
total_size = 0
207213
for resource in resources:
208214
_validate_resource_uri_in_skill(skill.uri, resource.uri)
209215
if resource.uri in seen:
@@ -213,11 +219,8 @@ def validate_skill(skill: Skill) -> None:
213219
raise ValueError(f"skill {skill.uri!r} resource {resource.uri!r} has an invalid SHA-256 digest")
214220
if resource.size < 0:
215221
raise ValueError(f"skill {skill.uri!r} resource {resource.uri!r} has a negative size")
216-
total_size += resource.size
217222
if skill.uri not in seen:
218223
raise ValueError(f"skill {skill.uri!r} resources does not include its own SKILL.md")
219-
if total_size > MAX_TOTAL_SIZE:
220-
raise ValueError(f"skill {skill.uri!r} has {total_size} bytes, exceeding {MAX_TOTAL_SIZE}")
221224

222225

223226
def validate_list_result(result: ListSkillsResult) -> None:
@@ -250,7 +253,11 @@ def parse_directory_uri(uri: str) -> tuple[str, str, str]:
250253

251254

252255
def validate_directory_result(uri: str, result: ReadDirectoryResult) -> None:
253-
"""Validate that `result.resources` are exactly the direct children of `uri`.
256+
"""Validate that each entry in `result.resources` is a unique direct child of `uri`.
257+
258+
Checks containment and shape only — that every listed resource is a direct
259+
child of `uri` with a unique `uri` and `name`. It cannot confirm the listing
260+
is exhaustive, since it has no independent view of the directory's contents.
254261
255262
Raises:
256263
ValueError: If `uri` is malformed, or any entry is not a direct child,

‎tests/shared/test_skills.py‎

Lines changed: 17 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -249,25 +249,34 @@ def test_validate_skill_rejects_malformed_digest_formats(digest: str) -> None:
249249
validate_skill(skill)
250250

251251

252-
def test_validate_skill_rejects_more_than_512_resources() -> None:
253-
"""SEP-2640 Limits: 512 entries per skill, `SKILL.md` included."""
252+
def test_validate_skill_accepts_exactly_512_resources() -> None:
253+
"""SEP-2640 Limits: a host MUST support skills up to and including 512 entries
254+
(`SKILL.md` counted), so a 512-entry manifest validates."""
255+
root = "skill://git-workflow/SKILL.md"
256+
resources = [_resource(root)] + [_resource(f"skill://git-workflow/f{i}.md") for i in range(511)]
257+
skill = Skill(uri=root, frontmatter={"name": "git-workflow", "description": "d"}, resources=resources)
258+
validate_skill(skill)
259+
260+
261+
def test_validate_skill_accepts_more_than_512_resources() -> None:
262+
"""SEP-2640 Limits: 512 is a SHOULD NOT threshold, not a hard cap — a host MAY support
263+
larger skills, so an over-count manifest is not rejected."""
254264
root = "skill://git-workflow/SKILL.md"
255265
resources = [_resource(root)] + [_resource(f"skill://git-workflow/f{i}.md") for i in range(512)]
256266
skill = Skill(uri=root, frontmatter={"name": "git-workflow", "description": "d"}, resources=resources)
257-
with pytest.raises(ValueError, match="exceeding 512"):
258-
validate_skill(skill)
267+
validate_skill(skill)
259268

260269

261-
def test_validate_skill_rejects_total_size_over_16mib() -> None:
262-
"""SEP-2640 Limits: 16 MiB total per skill, summed over `resources[].size`."""
270+
def test_validate_skill_accepts_total_size_over_16mib() -> None:
271+
"""SEP-2640 Limits: 16 MiB total is a SHOULD NOT threshold, not a hard cap — an over-size
272+
manifest is not rejected."""
263273
root = "skill://git-workflow/SKILL.md"
264274
skill = Skill(
265275
uri=root,
266276
frontmatter={"name": "git-workflow", "description": "d"},
267277
resources=[_resource(root, size=16 * 1024 * 1024 + 1)],
268278
)
269-
with pytest.raises(ValueError, match="exceeding"):
270-
validate_skill(skill)
279+
validate_skill(skill)
271280

272281

273282
def test_validate_skill_accepts_dynamic_resources_without_further_checks() -> None:

0 commit comments

Comments
 (0)