Uh oh!
There was an error while loading. Please reload this page.
feat: support plugins as the unit of opt-in (#378) - #394
Merged
Conversation
Conductor loaded a plugin's `skills/` and dropped everything else it shipped. A plugin's parts are written to work together — its `SKILL.md` routinely tells the agent to hand work to `prs:code-reviewer`, or to call an `ado` MCP tool — so the skill loaded, the agent read those instructions, reached for a subagent that was never registered, and said nothing. Add `runtime.plugins` and per-agent `plugins:`, which opt into the whole unit: skills, `agents/*.agent.md` subagents, and declared MCP servers. Entries take a string shorthand or an object with per-component switches (`skills` / `agents` / `mcp`), all defaulting on — defaulting one off would recreate the partial load the feature exists to fix. An entry is an installed plugin name or a path, classified by the same syntactic rule `skills:` uses; an uninstalled name errors naming where it looked, and an ambiguous one errors rather than picking a winner. Also recognise `.github/plugin/plugin.json` alongside `.claude-plugin/plugin.json`, in one shared definition used by both plugin resolution and `resolve_skill_plugin`. Both conventions have always worked at runtime, so recognising only the latter was Conductor's own gap — on an ordinary machine it stranded 12 of 13 installed plugins. Conductor deconstructs a plugin rather than handing its root to the SDK. Both SDKs' whole-plugin surfaces are all-or-nothing, and on Copilot `excluded_tools` hides an MCP tool from the model but does not stop the server subprocess launching with the user's credentials — verified with a startup marker file — so `mcp: false` built that way would be a guarantee that isn't one. Deconstructed, plugin MCP servers also inherit the existing `tools:` filters, `runtime.tool_output` limits, and dashboard tool events. `custom_agents` accepts the qualified `<plugin>:<agent>` name (verified against a live session), so namespacing survives. Supported on `copilot` and `claude-agent-sdk`; `claude`, `hermes` and `aca` reject `plugins:`, since injecting text into a prompt cannot produce a subagent or an MCP server. On `claude-agent-sdk`, reaching a plugin's skills requires registering its root, which also contributes every subagent it ships — so `agents: false` alongside `skills: true` is refused there rather than silently granting more than the YAML declared. Name collisions are refused rather than resolved by precedence, in the provider merge helpers as well as the validator, because `conductor run` never invokes the static validator and a dropped server or skill would be exactly the silent omission this feature removes. BREAKING: remove `plugins` from `skill_discovery.sources`. It scanned a plugin's `skills/` and left the rest behind, which is this bug rather than a feature with a gap; it was also wrong more often than it looked — of 13 installed plugins, 3 loaded instructions without the subagents those instructions dispatch to, and the 3 most plugin-like were never discovered at all. Use `runtime.plugins`, which reproduces on another machine. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Seven-agent review of the plugin implementation. The findings cluster in
one place: components that reached the provider incompletely, or refusals
that existed only in the half of the codebase `conductor run` skips.
Plugin MCP servers bypassed Conductor's resolution pipeline entirely.
A plugin's stdio server was handed a literal `${VAR}` and its http server
attached with no `Authorization` header — verified against a real
installed plugin declaring an `oauth` block. The server loaded and did
not work, which is issue #378's own failure mode one layer down. Both
sources now share `mcp_auth.resolve_mcp_servers`, so they cannot drift.
Four places claimed plugin MCP inherits a `tools:` filter via
`MCPServerDef`; it does not, and they now say what actually happens.
`_translate_mcp_servers` silently dropped every key it did not recognise.
It was written for `MCPServerDef`'s closed field set and is now fed
arbitrary third-party JSON, so an `oauth` block vanished and a
`disabled: true` server would have been launched. It now fails closed,
the same standard the narrowing `tools:` filter is already held to.
A discovered skill could shadow a *named* plugin's skill, inverting the
resolution-versus-discovery distinction the feature rests on — and the
warning called a scanned root "declared". Precedence now follows how much
the author said, and the message names the real source.
Refusals that ran in only one of two places, because `conductor run`
never calls the static validator: the claude-agent-sdk `agents: false`
carve-out, and the MCP name-collision check on both providers. A plugin
component that a name clash would have made unreachable was silently
dropped instead of refused.
Also: `_reject_unsupported_plugins` failed *open* for a provider with no
capability declaration; a provider could declare `plugins=True` while
lacking a skill surface, dropping plugin skills with the run reporting
success; `getattr(..., "skills_require_plugin_root", False)` would have
turned a rename into a silently disabled guard; `agents: false` still
parsed every `*.agent.md`, so the documented opt-out failed over the
files it opted out of; two entries resolving to one root discarded the
second's switches in the permissive direction; and `Path` satisfying
`.name` let duck-typing reclassify a path entry as an installed name and
resolve a different plugin. The three new dataclasses now enforce their
documented invariants, as their `skills/` counterparts already did.
A plugin's `tools:` frontmatter is written in its authoring CLI's
vocabulary, so forwarding it to claude-agent-sdk handed a subagent no
valid identifier; `tools: []` there left registered subagents with no
dispatch tool. Both refused rather than silently under-delivering.
Tests: the engine test exercised the single-provider branch, but
`conductor run` always uses the registry branch — its `workflow_plugins=`
could be deleted with all 5107 tests green. Now parametrized over both,
and mutation-verified. Adds coverage for claude-agent-sdk options
delivery, the Copilot resume path (which also guards the pre-existing
skills feature), the cache key, permission-denied trees, and the
`.github/plugin` convention end-to-end. `capabilities.plugins=True` is
now enforced at import rather than trusted, which gives the previously
unused `uses_native_plugins` a consumer.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>… pass
Final polish pass over the plugin implementation. No behaviour change
outside two verbose-output lines, both of which previously contradicted
what the surrounding code claimed.
`_merge_skills_and_plugin_skills` was called twice in `execute()` — once
to build `skill_directories` and again to count for the summary. It is
not idempotent: the second call re-fired the shadowing warning, so a
plugin skill losing to a declared one was reported twice. The result is
now computed once. The same summary counted by skill *name*, so a plugin
skill that had just been reported as "not enabled" was counted as
forwarded on the next line; counting by directory says what actually
went.
The MCP name-collision refusal was written out character-for-character in
both providers, differing only in line wrapping — the exact drift risk
`describe_dropped_components` was extracted to prevent, one layer over.
Now `providers/base.py::refuse_mcp_server_clashes`, with the "refuse
rather than resolve by precedence" argument stated once.
`plugins/registry.py` already imported `is_path_entry` from
`skills/registry.py` to decide *whether* an entry is a path, then
re-implemented deciding *which* path it is — including a duplicated
"normpath, not resolve()" comment. Both halves now share
`normalize_entry_path`, so a workflow naming one directory under both
`skills:` and `plugins:` cannot reach two different places.
Also collapses four dead empty-dict branches in `_merge_mcp_servers`
(`_stamp_cwd({})` is already `{}`, and no key can be overwritten once
clashes are refused), replaces a `setdefault`-as-clash-check with the
`get`-based form its two sibling functions already use, unpacks a
positional tuple access, and moves a comment onto the line it describes.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>`os.geteuid` does not exist on Windows, and the `skipif` expression is evaluated at import — so it raised `AttributeError` during collection and took the whole module with it, failing the Windows install-script job. Uses the idiom the repo already had in `tests/test_skills/test_path_entries.py` (`hasattr(os, "geteuid") and ...`), plus an explicit `win32` skip: `chmod` does not restrict reads there, so the tests would not be meaningful even once they collect. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Jason Robert (jrob5756)
marked this pull request as ready for review
August 10, 2026 15:11
Uh oh!
There was an error while loading. Please reload this page.
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 freeto 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.
Closes#378.
The problem
Conductor loaded a plugin's
skills/and dropped everything else it shipped. Aplugin's parts are written to work together — its
SKILL.mdroutinely tells theagent to hand work to
prs:code-reviewer, or to call anadoMCP tool. So theskill loaded, the agent read those instructions, reached for a subagent that was
never registered, and said nothing.
The reproduction from the issue, before:
{"available": ["explore", "task", "general-purpose", "code-review", "research"], "prs_found": []}After, on a live run:
{"available": ["explore", "task", "general-purpose", "code-review", "research", "prs:code-reviewer", "prs:code-simplifier", "prs:comment-analyzer", "prs:dead-code-finder", "prs:pr-test-analyzer", "prs:silent-failure-hunter", "prs:type-design-analyzer"], "prs_found": ["prs:code-reviewer", "prs:pr-test-analyzer", "prs:silent-failure-hunter"]}What this adds
runtime.pluginsand per-agentplugins:opt into the whole unit — skills,agents/*.agent.mdsubagents, and declared MCP servers:Entries take a string shorthand or an object with per-component switches, all
defaulting on — defaulting one off would recreate the partial load this
fixes. An entry is an installed plugin name or a path, classified by the same
syntactic rule
skills:uses. Tri-state inheritance matchesskills:exactly.Also recognises
.github/plugin/plugin.jsonalongside.claude-plugin/plugin.json, in one shared definition now used by bothplugin resolution and
resolve_skill_plugin. Both conventions have alwaysworked at runtime, so recognising only the latter was Conductor's own gap — on
an ordinary machine it stranded 12 of 13 installed plugins.
Why deconstruct rather than register the plugin root
Both SDKs have a whole-plugin surface (Copilot's
plugin_directories,ClaudeAgentOptions.plugins) and both are all-or-nothing. Registering a root isroughly one line and forfeits the ability to decline any single component.
Empirically, Copilot's
excluded_toolshides an MCP tool from the model butdoes not stop the server subprocess launching — proved with a startup marker
file: the model reported the tool absent while the marker was on disk. For
ado --authentication azclithe credential use happens at process start, not attool call, so
mcp: falsebuilt that way would be a cosmetic filter sold as aguarantee. Registering the root also inverts the providers against each other:
plugin MCP is unavoidable on Copilot and suppressed on claude-agent-sdk by its
unconditional
strict_mcp_config=True.Deconstructed, each component rides the surface Conductor already uses for it,
so plugin MCP inherits the existing
tools:filter,runtime.tool_outputlimits and dashboard tool events.
Two findings that were open questions on the issue, both now verified against
live SDKs:
custom_agentsaccepts a qualified<plugin>:<agent>name. A sessiongiven
{"name": "myplug:quokka"}listed it among launchable agent types, sonamespacing survives deconstruction and two plugins shipping a
reviewagentdo not collide. This was the one result that could have sunk the approach.
ClaudeAgentOptions.agentsexists, so subagents register inline there.But registering a plugin root is still the only way to reach its skills on
that SDK, and the docs describe that option as providing "custom commands,
agents, skills, and hooks" — a filter for skills, none for the rest.
Provider support
copilotclaude-agent-sdkclaude,hermes,acaplugins:rejected at validate and run timeUnlike skills there is no eager-injection fallback: text in a prompt cannot
become a subagent or an MCP server, so
CAPABILITIES.plugins=Falseis a realrefusal rather than a degraded mode.
The carve-out: on
claude-agent-sdk,agents: falsealongsideskills: trueis refused, because reaching the skills requires registering the root, which
carries every subagent with it. Its
hooks/warning likewise says exposed tothe CLI rather than not loaded, which would be false. Both branch on
AgentProvider.skills_require_plugin_root— a description of the mechanismrather than a provider-name check. The identical config works on
copilot.Silent-failure guards
The whole point of the feature is that a component never goes missing quietly,
so every refusal is enforced twice — in
config/validator.pyand again inAgentExecutor— becauseconductor runnever invokes the static validator.Name collisions (two plugins shipping one skill name; an MCP server name claimed
twice, or claimed by both a plugin and
runtime.mcp_servers) are refusedrather than resolved by precedence, in the provider merge helpers as well as the
validator.
hooks/andcommands/are dropped loudly via a validate warning.conductor validatenow prints what each plugin actually contributes:Breaking change
Removes
pluginsfromskill_discovery.sources. It scanned a plugin'sskills/and left the rest behind, which is this bug rather than a feature witha gap — and it was wrong more often than it looked: of 13 installed plugins, 3
loaded their instructions without the subagents those instructions dispatch to,
and the 3 most plugin-like (MCP + subagent toolkits with no
skills/) werenever discovered at all.
personalandprojectare unchanged. Replace withruntime.plugins, which brings the whole unit and, unlike a scan, reproduces onanother machine.
Layout
New
src/conductor/plugins/(manifest, agent parsing, resolution, errors), plussrc/conductor/frontmatter.py— a shared---splitter extracted soSKILL.mdand
*.agent.mduse one parser rather than two that can drift.tests/test_plugins/builds every plugin tree on disk and takeshomeas afixture, so no test reads the developer's real
~. Its executor- andengine-integration suites are load-bearing: a plugin's subagents and MCP servers
have no fallback delivery path, so a negative assertion could not tell a working
path from a dropped one.
Verification
ruff check/ruff format/ty checkcleanexamples/plugins.yaml— whichships its own
examples/demo-plugin/so it validates anywhere rather thandepending on what the runner happens to have installed
Review
A code-review pass found 6 issues, all fixed and regression-tested — including
three latent silent-drop paths that would have ironically reintroduced the exact
bug class this feature removes.
Follow-up
Git-backed plugin sources (
plugin_sources, lockfile, cache,conductor pluginverbs) are filed separately as #380, deliberately sequenced second: name and
path resolution has to exist regardless, and git should feed it rather than
duplicate it.
Review round
A seven-agent code review ran over this branch. Its findings clustered in one
place — components that reached the provider incompletely, and refusals that
existed only in the half of the codebase
conductor runskips. Fixed in6c75670, polished in369699d.The most serious finding was that plugin MCP servers bypassed Conductor's
resolution pipeline entirely: a plugin's stdio server was handed a literal
${VAR}and its http server attached with noAuthorizationheader. Verifiedagainst a real installed plugin declaring an
oauthblock. The server loadedand did not work — issue #378's own failure mode, one layer down. Both server
sources now share
mcp_auth.resolve_mcp_serversso they cannot drift, and fourdocuments that claimed plugin MCP inherits a
tools:filter viaMCPServerDefnow say what actually happens.
Also fixed:
_translate_mcp_serverssilently dropped every key it did notrecognise (an
oauthblock vanished; adisabled: trueserver would have beenlaunched); a discovered skill could shadow a named plugin's skill, inverting
the resolution-versus-discovery distinction the feature rests on;
_reject_unsupported_pluginsfailed open for a provider with no capabilitydeclaration;
agents: falsestill parsed every*.agent.md, so the documentedopt-out failed over the files it opted out of; and
Pathsatisfying.nameletduck-typing reclassify a path entry as an installed name and resolve a different
plugin on disk.
Two refusals ran in only one of their two required places, which matters because
conductor runnever calls the static validator: the claude-agent-sdkagents: falsecarve-out, and the MCP name-collision check.Test gap worth calling out. The engine test exercised the single-provider
branch, but
conductor runalways uses the registry branch — itsworkflow_plugins=argument could be deleted with all 5107 tests still green.Mutation-verified before and after; it is now parametrized over both modes.
capabilities.plugins=Trueis also enforced at import rather than trusted, so aprovider cannot declare plugin support while dropping a delivery channel.
Every fix is mutation-verified where a test was the point.
Verification
claude-agent-sdkinstalled; 5134 withoutruff/ruff format/ty checkcleancomponents (skills, subagents, MCP) reach the SDK