Repository navigation
Conversation
SkillFileTemplate._read() passed raw str/bytes through convert_result(), which stamped the template's advertised application/octet-stream onto every supporting file. A .md read returned TextResourceContents with octet-stream and a .json read returned a Blob with octet-stream, while supporting_files="resources" mode returned text/markdown and application/json for the same files. Guess the MIME per file in the template read path and return a ResourceResult carrying it, keeping the existing text/blob selection.
The MIME fix built ResourceContent directly in SkillFileTemplate._read() instead of going through convert_result(), which also dropped the template's component-level meta from the wire content item. Apply public_result_meta(self.meta) to the constructed content so template meta continues to reach resources/read results while the per-file MIME is preserved.
|
This pull request is awaiting maintainer assignment. Linked PRs stay open while maintainers triage the issue; this check remains failing until assignment. Per CONTRIBUTING.md, an external PR must reference an issue that's assigned to its author. To get there:
Once you're assigned and the link is present, this check is re-run automatically. Previously gate-closed PRs also reopen — no further action needed. Maintainers: reopen this PR or remove the |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
marvin-context-protocol
Bot
added
the
bug
|
Closing this one to reduce review volume, per maintainer feedback. The underlying issue remains open for triage; the branch and tests stay on the fork if it becomes useful later. |
Closes #5852
Description
With the default
SkillProvider(supporting_files="template"), supporting-file reads always reportedapplication/octet-stream: the provider guessed the file MIME to choose text vs blob, then returned a barestr/bytes, andconvert_resultstamped the template's staticmime_typeover the guess. The same files insupporting_files="resources"mode returnedtext/markdown/application/json.What changed
SkillFileTemplate._guess_mime_type()centralizes the guess and the octet-stream fallback._read()wraps the raw content inResourceResult([ResourceContent(content, mime_type=<guessed>)])so the per-file MIME survives, instead of routing throughconvert_resultwith the template's advertised MIME.meta=public_result_meta(self.meta), preserving the template-level metadata propagation thatconvert_resultperformed before this change (privatefastmcp._*keys are stripped).read()keeps its existing rawstr/bytescontract and text/blob selection (text iff the MIME starts withtext/), so direct-read and path-safety tests are unchanged. URI routing, listing, and manifest behavior are untouched.Contribution type
Bug fix verification
Test paths:
tests/server/providers/test_skills_provider.py(three MIME parametrizations plus a listing control) and the template-meta propagation test (both.mdand.jsoncontent items).Red at
adf90b66(implementation reverted, final tests applied): 2 failed, 99 passed; failures are the.mdand.jsonMIME assertions (the unknown-extension and listing controls pass as expected); no collection errors.Green at the fix commit: 2425 passed, 1 skipped across
tests/server/providers/ tests/utilities/.Validation
uv run --isolated pytest tests/server/providers/ tests/utilities/ -q-> 2425 passed, 1 skippeduv run --isolated ruff checkandruff format --checkon the changed files -> cleanuv run --isolated ty check-> All checks passedAI assistance
Tool(s) used: Claude Code
How you used it: investigated from a verified reproduction, implemented the MIME preservation and the regression tests.
Checklist
uv run prek run --all-filesand all checks pass (focused checks passed)