Repository navigation
fix(plugins): skip invalid plugin components instead of failing - #309
Merged
Merged
Conversation
Plugin diagnostics and per-server MCP preparation errors now log a warning and skip the affected skill or server. Duplicate plugin MCP server names keep the first plugin in alias order.
Contributor
There was a problem hiding this comment.
Plugin component isolation currently loses user-visible skip diagnostics, converts transient disk failures into server removals, and allows failed declarations to transfer duplicate-name ownership. The documented rejection behavior also needs updating.
Took 15 min
Unavailable plugin files or data directories still fail preparation instead of silently removing a server. A plugin MCP name is reserved before preparation so an invalid first declaration cannot transfer ownership to a later plugin. Docs describe component-level skipping.
A plugin command or cwd whose ancestor is a regular file is a package defect and skips only that server.
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
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.
Summary
Invalid plugin components no longer abort session startup. Plugin diagnostics and per-server MCP preparation errors are logged as warnings and only the affected skill or MCP server is skipped.
Motivation
A single non-conforming
SKILL.mdor a duplicate MCP server name across two marketplace plugins made the agent exit before the session opened:agentkit-pluginsreports these as non-fatal diagnostics scoped to the narrowest component; kit was escalating them to hard errors.Impact
mcp.json, MCP server entry, orextensionsfield still load their valid parts. Each skipped component is logged viatracing::warn!, which the CLI does not currently surface; feat: model-accessible diagnostics search for skill and MCP loading issues #310 tracks a model-accessible diagnostics search.mcp.jsonedit during hot-reload now publishes a generation without that plugin's MCP servers, instead of rejecting the reload and retaining the previous generation. Fatal load errors (e.g. invalidplugin.json) still retain the last valid generation.Technical details
Duplicate plugin MCP server names
The first plugin in alias order keeps the server name; later declarations are skipped with a warning. The name is reserved before preparation, so an invalid first declaration leaves the server unavailable rather than handing it to a later plugin. This mirrors the existing precedence where user-config MCP servers silently override plugin servers.
Per-server MCP preparation
Server preparation moved into
prepare_plugin_server, so an unsupported SSE transport, invalidcwd, escaping command path, or invalid header skips only that server. Preparation errors are split intoInvalid(skipped) andUnavailable(propagated). Disk-availability and filesystem failures stay fatal so a transient failure cannot publish a configuration that unregisters a working server. Non-UTF-8 plugin root or data paths also remain fatal.Revalidation passes
The diagnostic checks in the staging revalidation and immutable skill snapshot passes were removed. The existing semantic-key and skill-set comparisons still reject plugins that change mid-staging.