feat: plugin system — auto-discover and execute custom tools#16
Merged
kienbui1995 merged 1 commit intomainfrom Apr 11, 2026
Merged
feat: plugin system — auto-discover and execute custom tools#16kienbui1995 merged 1 commit intomainfrom
kienbui1995 merged 1 commit intomainfrom
Conversation
Plugins are scripts in .magic-code/tools/ (sh, py, js). Auto-discovered on startup, registered as plugin_<name> tools. LLM can call them like any other tool. - Added JS support (node) alongside existing sh/py - Plugin specs included in tool_specs sent to LLM - Scripts receive input via PLUGIN_INPUT env var - First comment line used as tool description
📝 WalkthroughWalkthroughThe plugin system now supports JavaScript scripts ( Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
mc/crates/mc-tools/src/plugin.rs (1)
83-89:⚠️ Potential issue | 🔴 CriticalBlock path traversal in plugin script resolution.
nameis interpolated directly into a filesystem path. A crafted tool name likeplugin_../../tmp/pwncan escape.magic-code/toolsand execute unintended scripts.🔒 Proposed hardening
fn find_plugin_script(workspace: &Path, name: &str) -> Option<PathBuf> { let dir = workspace.join(".magic-code/tools"); + if name.contains('/') || name.contains('\\') || name.contains("..") { + return None; + } + let dir_canon = dir.canonicalize().ok()?; for ext in ["sh", "py", "js"] { let path = dir.join(format!("{name}.{ext}")); - if path.exists() { - return Some(path); + if let Ok(path_canon) = path.canonicalize() { + if path_canon.starts_with(&dir_canon) && path_canon.is_file() { + return Some(path_canon); + } } } None }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@mc/crates/mc-tools/src/plugin.rs` around lines 83 - 89, The plugin lookup currently interpolates an unvalidated tool name into a filesystem path in find_plugin_script, allowing path traversal; validate/sanitize the name before constructing dir.join by rejecting any names that contain path separators, dots, or ".." (or better, require a strict whitelist like /^[A-Za-z0-9_-]+$/) and return None on invalid input so only safe filenames are checked in the ext loop (the validation should occur at the start of find_plugin_script, referencing the name parameter and the same dir/.magic-code/tools resolution).
🧹 Nitpick comments (1)
mc/crates/mc-tools/src/registry.rs (1)
109-114: Consider guarding tool-spec growth from plugins.Because
all_specs()feeds prompt/tool-schema construction (mc/crates/mc-core/src/runtime.rs:180-220), many plugins or long descriptions can consume context budget quickly. Consider a cap or truncation strategy for plugin descriptions/spec count.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@mc/crates/mc-tools/src/registry.rs` around lines 109 - 114, The all_specs() accumulator (cached_specs.get_or_init -> all_tool_specs, mcp_tool_specs, plugin_specs) can grow unbounded and inflate prompt/schema context; modify all_specs (or the cached_specs initializer) to enforce a configurable cap and/or truncate plugin_specs descriptions: limit the total number of ToolSpec entries merged (e.g., MAX_TOOL_SPECS) and for entries from plugin_specs trim long description fields to a max length (e.g., MAX_DESCRIPTION_CHARS) or replace excess text with an ellipsis, and make both limits configurable constants so downstream code in runtime.rs receives a bounded spec list.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@mc/crates/mc-tools/src/registry.rs`:
- Around line 54-59: with_workspace_root updates self.plugin_specs but does not
clear the cached aggregate used by all_specs(), causing newly discovered plugins
to remain invisible; after assigning self.plugin_specs in with_workspace_root
(and before returning self), invalidate or clear the cache (e.g. set
self.cached_specs to None or an empty value) so all_specs() will rebuild with
the new plugin_specs on next access.
---
Outside diff comments:
In `@mc/crates/mc-tools/src/plugin.rs`:
- Around line 83-89: The plugin lookup currently interpolates an unvalidated
tool name into a filesystem path in find_plugin_script, allowing path traversal;
validate/sanitize the name before constructing dir.join by rejecting any names
that contain path separators, dots, or ".." (or better, require a strict
whitelist like /^[A-Za-z0-9_-]+$/) and return None on invalid input so only safe
filenames are checked in the ext loop (the validation should occur at the start
of find_plugin_script, referencing the name parameter and the same
dir/.magic-code/tools resolution).
---
Nitpick comments:
In `@mc/crates/mc-tools/src/registry.rs`:
- Around line 109-114: The all_specs() accumulator (cached_specs.get_or_init ->
all_tool_specs, mcp_tool_specs, plugin_specs) can grow unbounded and inflate
prompt/schema context; modify all_specs (or the cached_specs initializer) to
enforce a configurable cap and/or truncate plugin_specs descriptions: limit the
total number of ToolSpec entries merged (e.g., MAX_TOOL_SPECS) and for entries
from plugin_specs trim long description fields to a max length (e.g.,
MAX_DESCRIPTION_CHARS) or replace excess text with an ellipsis, and make both
limits configurable constants so downstream code in runtime.rs receives a
bounded spec list.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7f182ec2-b5dd-47cb-94b0-63da78ba3f12
📒 Files selected for processing (2)
mc/crates/mc-tools/src/plugin.rsmc/crates/mc-tools/src/registry.rs
Comment on lines
+54
to
59
| self.plugin_specs = crate::plugin::discover_plugins(&root); | ||
| if !self.plugin_specs.is_empty() { | ||
| tracing::info!(count = self.plugin_specs.len(), "plugins discovered"); | ||
| } | ||
| self.sandbox = Some(Sandbox::new(root)); | ||
| self |
There was a problem hiding this comment.
Invalidate cached_specs after plugin discovery.
with_workspace_root mutates plugin_specs but does not reset cached_specs. If all_specs() was initialized earlier, plugin tools never become visible from this registry instance.
🩹 Proposed fix
pub fn with_workspace_root(mut self, root: PathBuf) -> Self {
self.plugin_specs = crate::plugin::discover_plugins(&root);
if !self.plugin_specs.is_empty() {
tracing::info!(count = self.plugin_specs.len(), "plugins discovered");
}
+ self.cached_specs = std::sync::OnceLock::new(); // invalidate cache
self.sandbox = Some(Sandbox::new(root));
self
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| self.plugin_specs = crate::plugin::discover_plugins(&root); | |
| if !self.plugin_specs.is_empty() { | |
| tracing::info!(count = self.plugin_specs.len(), "plugins discovered"); | |
| } | |
| self.sandbox = Some(Sandbox::new(root)); | |
| self | |
| self.plugin_specs = crate::plugin::discover_plugins(&root); | |
| if !self.plugin_specs.is_empty() { | |
| tracing::info!(count = self.plugin_specs.len(), "plugins discovered"); | |
| } | |
| self.cached_specs = std::sync::OnceLock::new(); // invalidate cache | |
| self.sandbox = Some(Sandbox::new(root)); | |
| self |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@mc/crates/mc-tools/src/registry.rs` around lines 54 - 59, with_workspace_root
updates self.plugin_specs but does not clear the cached aggregate used by
all_specs(), causing newly discovered plugins to remain invisible; after
assigning self.plugin_specs in with_workspace_root (and before returning self),
invalidate or clear the cache (e.g. set self.cached_specs to None or an empty
value) so all_specs() will rebuild with the new plugin_specs on next access.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Plugin system that auto-discovers scripts in
.magic-code/tools/and registers them as LLM-callable tools.Supported languages
.sh).py).js) — newHow it works
.magic-code/tools/plugin_<filename>toolsPLUGIN_INPUTenv varTested
plugin_hello(bash): ✅plugin_timestamp(python): ✅plugin_upper(js): ✅152 tests pass.
Summary by CodeRabbit