Conversation
There was a problem hiding this comment.
Pull request overview
This PR introduces a full MCP server implementation for FastSkills, including core infrastructure (constants, skill management, sandboxed execution, and MCP tool wrappers), plus comprehensive tests and documentation/examples for running and integrating the server.
Changes:
- Add a centralized configuration layer (
fastskills/constants.py,pyproject.toml,.gitignore,.cursor/rules) and package metadata (fastskills/__init__.py). - Implement core functionality for skills management, filesystem operations, and Python code sandboxing, exposed via an MCP server (
fastskills.skill_manager,python_sandbox,mcp_server,mcp_tools.*), with LangChain integration and examples. - Add extensive unit tests under
tests/and expand documentation inREADME.mdandexamples/README.mddescribing server usage, configuration, and integration patterns.
Reviewed changes
Copilot reviewed 27 out of 28 changed files in this pull request and generated 30 comments.
Show a summary per file
| File | Description |
|---|---|
| .gitignore | Adds ignores for virtualenvs, editor metadata, caches, and lock files to keep the repo clean. |
| .cursor/rules/code-organization.mdc | Defines project-wide rules for import ordering, constants placement, and file structure. |
| .cursor/rules/clean-unused-imports.mdc | Establishes a rule to remove unused imports and suggests tooling to enforce it. |
| .cursor/rules/no-try-import.mdc | Documents the policy to avoid wrapping imports in try/except except for well-documented optional dependencies. |
| .cursor/rules/readme-maintenance.mdc | Introduces a rule to prefer a single root README and limit additional READMEs to explicit exceptions. |
| README.md | Expands root documentation with installation (MCP server), configuration, tool descriptions, examples, and updates the roadmap; contains several small inconsistencies with the actual code (defaults, links, and duplication). |
| pyproject.toml | Defines the fastskills package, core dependencies (FastMCP, mcp, pyyaml, etc.) and an examples extra for LangChain integration. |
| fastskills/init.py | Declares the package docstring and sets __version__ = "0.1.0". |
| fastskills/constants.py | Centralizes server and environment constants (ports, env var names, transports, sandbox defaults); note that DEFAULT_TOP_K and DEFAULT_CODE_SANDBOX_MODULES drive behavior and must stay in sync with documentation. |
| fastskills/skill_manager.py | Implements Skill and SkillManager for discovering skills under .claude/skills/CWD, parsing SKILL.md frontmatter, and providing basic search with a relevance score; includes an unused os import that can be removed. |
| fastskills/python_sandbox.py | Provides the PythonSandbox for restricted code execution and a LangChain create_sandbox_tool; has some behavioral issues around error reporting, result extraction for falsy values, and unused API parameters (timeout, allowed_modules in execute_with_files). |
| fastskills/mcp_tools/init.py | Re-exports register_skills_tools, register_filesystem_tools, and register_code_sandbox_tools for convenient MCP tool registration. |
| fastskills/mcp_tools/skills_tools.py | Wraps SkillManager into MCP tools (read_skill, search_skill), enriches read_skill with available Python script paths, and reads FASTSKILLS_TOP_K from the environment; docstring default for FASTSKILLS_TOP_K is out of sync with the actual DEFAULT_TOP_K. |
| fastskills/mcp_tools/filesystem_tools.py | Implements a suite of filesystem MCP tools (default directory management, read/write/update/list/delete/move/copy) with path resolution and optional base-directory restriction. |
| fastskills/mcp_tools/code_sandbox_tools.py | Exposes an execute_python_code MCP tool using PythonSandbox, discovers the project root, and documents allowed modules dynamically in the tool docstring. |
| fastskills/mcp_server.py | Wires up a FastMCP server, reads env-based configuration (FS_BASE_DIRECTORY, CODE_SANDBOX_MODULES), registers all tool groups, and handles CLI args for port/transport; current env handling for CODE_SANDBOX_MODULES can crash if os.getenv returns None. |
| examples/README.md | Documents how to run the MCP server and the LangChain example, lists tools, and details configuration; a few defaults and URLs (FASTSKILLS_TOP_K, CODE_SANDBOX_MODULES, MCP_SERVER_URL, and stdio vs SSE) are inconsistent with the implementation. |
| examples/langchain_mcp_example.py | Provides an async LangChain example that can auto-start or reuse an MCP server, stream tool calls/results, and summarize tool usage; uses SSE transport via MultiServerMCPClient. |
| tests/init.py | Marks the tests directory as a package. |
| tests/fastskills/init.py | Marks tests.fastskills as a package. |
| tests/fastskills/mcp_tools/init.py | Marks tests.fastskills.mcp_tools as a package. |
| tests/README.md | Documents the test layout, how to run tests via unittest/pytest, and current coverage notes. |
| tests/fastskills/test_skill_manager.py | Adds unit tests for Skill and SkillManager covering initialization, discovery, search behavior, and metadata validation. |
| tests/fastskills/test_python_sandbox.py | Provides broad coverage of PythonSandbox and create_sandbox_tool, including module restrictions, builtins, filesystem execution, and the LangChain tool wrapper; surfaces mismatches in error formatting and result extraction for certain cases. |
| tests/fastskills/test_mcp_server.py | Tests MCP server initialization, environment variable usage, argparse defaults/choices, and main-branch behavior under both stdio and SSE transports; also verifies env access for filesystem base directory and code sandbox modules. |
| tests/fastskills/mcp_tools/test_skills_tools.py | Tests registration and behavior of read_skill/search_skill tools, including success, not-found, load errors, TOP_K behavior, and output formatting; includes one assertion treating a real method as a mock. |
| tests/fastskills/mcp_tools/test_filesystem_tools.py | Provides thorough coverage for filesystem tools: default directory handling, path resolution/sanitization, read/write/update/list/delete/create/move/copy operations, and base-directory restrictions. |
| tests/fastskills/mcp_tools/test_code_sandbox_tools.py | Tests registration and behavior of execute_python_code MCP tool, including success/failure cases, output/result/traceback inclusion, and use of default vs custom allowed modules. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 30 out of 31 changed files in this pull request and generated 13 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 29 out of 31 changed files in this pull request and generated 10 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
WuizaKaseiyo
left a comment
There was a problem hiding this comment.
I am trying implmenting them locally
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 29 out of 31 changed files in this pull request and generated 8 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 30 out of 32 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 32 out of 34 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 33 out of 35 changed files in this pull request and generated 7 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Skills should not be considered as a MCP server. |
This pull request introduces several documentation and code organization improvements, as well as new project-wide standards and constants for the FastSkills project. The most significant changes include a major expansion of the
README.mdwith detailed usage instructions, the addition of project-wide coding standards and rules, and the introduction of a centralconstants.pyfor configuration values. Below are the most important changes grouped by theme:Documentation Enhancements:
README.mdto include comprehensive setup, installation, configuration, usage instructions for the MCP server, environment variable documentation, and a detailed list of available tools and examples. Also updated the development roadmap to reflect completed milestones. [1] [2] [3]examples/README.mdproviding step-by-step instructions for running the MCP server, configuring environment variables, integrating with LangChain agents, and example queries.Project Standards and Code Organization:
.cursor/rules/code-organization.mdcto establish standards for import ordering, placement of constants, and file structure, including recommendations for project-level constants in a dedicated file..cursor/rules/clean-unused-imports.mdcto require removal of unused imports and recommend tooling for enforcement..cursor/rules/no-try-import.mdcto prohibit wrapping imports in try-except blocks except for well-documented, truly optional dependencies..cursor/rules/readme-maintenance.mdcto enforce maintaining a single rootREADME.mdand prohibit additional README files unless explicitly requested.Project Initialization and Constants:
fastskills/constants.pyto centralize project-wide constants such as default ports, environment variable names, server configuration, and allowed modules for the code sandbox.__init__.pyfor thefastskillspackage, setting the project version and docstring.