Skip to content

Commit 03b0bd8

Browse files
Report a skill file deleted before reload as a failed read and align read error messages
1 parent af17ea2 commit 03b0bd8

3 files changed

Lines changed: 39 additions & 7 deletions

File tree

‎docs/advanced/skills.md‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -207,8 +207,8 @@ are not served.
207207
The catalog reads and hashes every file when `load()` runs. By default, it reads files from
208208
the provider again on each `resources/read` and checks their bytes against that fingerprint.
209209

210-
If a file changes afterwards, reading it fails with an internal error (`-32603`) until you
211-
[reload the catalog](#reloading-skills) or restart the server. And because `load()`
210+
If a file changes or is deleted afterwards, reading it fails with an internal error (`-32603`)
211+
until you [reload the catalog](#reloading-skills) or restart the server. And because `load()`
212212
reads every file, a large asset directory makes startup slower.
213213

214214
### Reloading skills
@@ -265,8 +265,8 @@ connection runs the timer only while a client is connected.
265265
!!! note
266266
With `auto_refresh`, the catalog keeps the bytes of every served file **in memory**. That's
267267
what lets the previous skills keep serving consistently, even if you edit or delete a file on
268-
disk while a refresh is still running. Without it, a file edited on disk reads as an internal
269-
error until the next `reload()`.
268+
disk between refreshes. Without it, a file edited or deleted on disk reads as an internal
269+
error until you call `reload()`.
270270

271271
!!! warning
272272
Clients don't hear about a reload. They may keep a `skills/list` result for `ttl_ms`, and a

‎src/mcp/server/skills/catalog.py‎

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ class _Fingerprint(BaseModel, frozen=True):
7272
class _SkillFileResource(Resource):
7373
"""One file of a skill, read from the provider on every `resources/read`.
7474
75-
The bytes are checked against the digest taken at load time, so a manifest
75+
The bytes are checked against the digest taken at the last scan, so a manifest
7676
entry never describes content other than what is served.
7777
"""
7878

@@ -91,9 +91,15 @@ def size(self) -> int:
9191
return self._fingerprint.size
9292

9393
async def read(self) -> str | bytes:
94-
data = await self._provider.read_file(self._path)
94+
try:
95+
data = await self._provider.read_file(self._path)
96+
except OSError as exc:
97+
# A file removed or made unreadable between scans is an expected state, not a crash.
98+
raise ResourceError(
99+
f"{self.uri} could not be read; it was deleted or became unreadable after the last scan"
100+
) from exc
95101
if _fingerprint(data) != self._fingerprint:
96-
raise ResourceError(f"{self.uri} changed since the catalog was loaded")
102+
raise ResourceError(f"{self.uri} changed after the last scan")
97103
try:
98104
return data.decode("utf-8")
99105
except UnicodeDecodeError:

‎tests/server/skills/test_catalog.py‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@
5454
pytestmark = pytest.mark.anyio
5555

5656
CATALOG_LOGGER = "mcp.server.skills.catalog"
57+
SERVER_LOGGER = "mcp.server.mcpserver.server"
5758

5859

5960
class _RawResult(BaseModel):
@@ -484,6 +485,31 @@ async def test_reading_a_file_that_changed_after_load_is_an_internal_error() ->
484485
with pytest.raises(MCPError) as exc_info:
485486
await client.read_resource("skill://git-workflow/SKILL.md")
486487
assert exc_info.value.code == INTERNAL_ERROR
488+
assert exc_info.value.message == "skill://git-workflow/SKILL.md changed after the last scan"
489+
490+
491+
async def test_reading_a_file_deleted_after_load_is_an_internal_error_without_a_traceback(
492+
tmp_path: Path, caplog: pytest.LogCaptureFixture
493+
) -> None:
494+
"""SDK-defined: a file removed before the next reload is an expected state, logged as a failed read, not a crash."""
495+
(tmp_path / "git-workflow").mkdir()
496+
(tmp_path / "git-workflow" / "SKILL.md").write_bytes(_skill_md("git-workflow"))
497+
(tmp_path / "git-workflow" / "guide.md").write_bytes(b"# Guide\n")
498+
catalog = await SkillCatalog.load(FilesystemSkillProvider(tmp_path), parse_frontmatter=_parse)
499+
(tmp_path / "git-workflow" / "guide.md").unlink()
500+
501+
with caplog.at_level(logging.INFO, logger=SERVER_LOGGER):
502+
async with Client(MCPServer("skills", extensions=[catalog])) as client:
503+
with pytest.raises(MCPError) as exc_info:
504+
await client.read_resource("skill://git-workflow/guide.md")
505+
506+
assert exc_info.value.code == INTERNAL_ERROR
507+
assert (
508+
exc_info.value.message
509+
== "skill://git-workflow/guide.md could not be read; it was deleted or became unreadable after the last scan"
510+
)
511+
records = [record for record in caplog.records if record.name == SERVER_LOGGER]
512+
assert [(record.levelno, record.exc_info) for record in records] == [(logging.INFO, None)]
487513

488514

489515
async def test_reading_a_uri_outside_the_catalog_is_invalid_params() -> None:

0 commit comments

Comments
 (0)