Skip to content

feat: add filesystem plugin loader APIs#38

Open
tvdavies wants to merge 1 commit into
mainfrom
codex/filesystem-plugin-loader
Open

feat: add filesystem plugin loader APIs#38
tvdavies wants to merge 1 commit into
mainfrom
codex/filesystem-plugin-loader

Conversation

@tvdavies
Copy link
Copy Markdown
Contributor

Summary

  • add filesystem plugin loading APIs: loadPluginFromDirectory, loadPluginsFromDirectories, and createSubagentDefinitionsFromFilesystemAgents
  • support Claude Code-style package layout with plugin.json, optional mcp.json, optional plugin.js, bundled skills/, and agents/*.md
  • add typed interfaces for manifests, entrypoints, agent frontmatter/definitions, options, results, and load errors
  • export new APIs and types from public src/index.ts
  • document the filesystem plugin specification in docs/filesystem-plugins.md
  • add changelog entry under Unreleased

Details

  • plugin discovery supports one- and two-level directory layouts
  • plugin.js loading is opt-in via allowCodeEntrypoint (secure default is off)
  • MCP server maps (mcp.json or plugin.js mcpServers) are converted to synthetic plugin names <plugin>__<server>
  • agents/*.md frontmatter supports description, model, allowedTools, plugins, and streaming
  • agent plugin refs support self, self:mcp, and explicit plugin names

Validation

  • bun run lint
  • bun run type-check
  • bun run test tests/plugins-loader.test.ts
  • pre-push hook also ran:
    • biome check .
    • full vitest suite (2052 passed, 8 skipped)

Copilot AI review requested due to automatic review settings February 23, 2026 23:06
@greptile-apps
Copy link
Copy Markdown

greptile-apps Bot commented Feb 23, 2026

Greptile Summary

Adds filesystem plugin loader APIs (loadPluginFromDirectory, loadPluginsFromDirectories, createSubagentDefinitionsFromFilesystemAgents) for Claude Code-style plugin packages with support for plugin.json manifests, optional code entrypoints, MCP server configs, bundled skills, and agent definitions.

Key changes:

  • New src/plugins/loader.ts (960 lines) implements plugin discovery, manifest parsing, skill/agent loading, and MCP integration
  • Plugin discovery supports 1-2 level directory layouts with namespace support
  • Code entrypoints (plugin.js) are opt-in via allowCodeEntrypoint flag (secure default: disabled)
  • MCP servers from mcp.json or code entrypoints generate synthetic plugin entries (<plugin>__<server>)
  • Agent definitions (agents/*.md) use YAML frontmatter with support for model resolution, tool allowlists, and plugin refs (self, self:mcp)
  • Comprehensive test suite validates all major features
  • Well-documented in docs/filesystem-plugins.md with examples and security notes
  • All exports properly added to public API

Confidence Score: 4/5

  • Safe to merge with minor considerations about regex edge cases
  • Well-architected feature with comprehensive tests, good error handling, secure defaults (code execution opt-in), and clear documentation. Code follows existing patterns from skills loader. One minor regex edge case to verify.
  • No files require special attention

Important Files Changed

Filename Overview
src/plugins/loader.ts Adds comprehensive filesystem plugin loader with validation, MCP support, and agent definitions - well-structured with proper error handling
tests/plugins-loader.test.ts Comprehensive test coverage for plugin loading, code entrypoints, model resolution, and subagent creation
docs/filesystem-plugins.md Clear documentation with examples, directory layout, and security defaults
src/index.ts Exports new plugin loader APIs and types following existing patterns
CHANGELOG.md Properly documents new APIs under Unreleased/Added section

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    Start[loadPluginsFromDirectories] --> Discover[Discover plugin directories]
    Discover --> Sort[Sort discovered directories]
    Sort --> Load[loadPluginFromDirectory]
    
    Load --> Manifest[Read plugin.json manifest]
    Manifest --> Validate{Validate manifest?}
    Validate -->|Yes| ValidateName[Check name format]
    Validate -->|No| Code
    ValidateName --> Code
    
    Code{allowCodeEntrypoint?}
    Code -->|Yes| ImportJS[Import plugin.js entrypoint]
    Code -->|No| Skills
    ImportJS --> Skills
    
    Skills[Load skills from skills directory]
    Skills --> MergeSkills[Merge file skills and code skills]
    MergeSkills --> BasePlugin[Create base AgentPlugin]
    
    BasePlugin --> MCP[Collect MCP servers from mcp.json]
    MCP --> SyntheticPlugins[Generate synthetic plugins for MCP servers]
    
    SyntheticPlugins --> Agents[Load agents from agents directory]
    Agents --> ParseMD[Parse YAML frontmatter]
    ParseMD --> ResolveModel{Model specified?}
    ResolveModel -->|Yes| CallResolver[Call resolveModel callback]
    ResolveModel -->|No| ResolvePlugins
    CallResolver --> ResolvePlugins
    
    ResolvePlugins[Resolve plugin refs]
    ResolvePlugins --> AgentDef[Create FilesystemAgentDefinition]
    
    AgentDef --> CheckParent{getParentAgent provided?}
    CheckParent -->|Yes| CreateSubagents[Create SubagentDefinitions]
    CheckParent -->|No| Return
    CreateSubagents --> Return[Return PluginLoadResult]
Loading

Last reviewed commit: 57dca0a

Copy link
Copy Markdown

@greptile-apps greptile-apps Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

5 files reviewed, 1 comment

Edit Code Review Agent Settings | Greptile

Comment thread src/plugins/loader.ts
}

function parseMarkdownFrontmatter<TFrontmatter>(content: string): ParsedMarkdownFile<TFrontmatter> {
const frontmatterRegex = /^---\n([\s\S]*?)\n---\n([\s\S]*)$/;
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Regex requires newlines after both opening and closing markers. Will fail if file ends immediately after closing marker without trailing newline. Consider making body group optional with non-capturing group.

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds a filesystem-based plugin loader to the SDK, enabling discovery and loading of Claude Code-style local plugin packages (manifest, optional MCP config, optional code entrypoint, bundled skills, and markdown-based agent definitions).

Changes:

  • Introduces new filesystem plugin loader APIs and typed interfaces (loadPluginFromDirectory, loadPluginsFromDirectories, createSubagentDefinitionsFromFilesystemAgents).
  • Exports the new APIs/types from the public entrypoint (src/index.ts).
  • Adds documentation and a test suite for plugin discovery, skills/MCP aggregation, and agent parsing.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
src/plugins/loader.ts Implements plugin discovery, manifest parsing, optional entrypoint import, skills loading, MCP server synthesis, and agent markdown parsing.
src/index.ts Re-exports the new loader APIs and associated types as public SDK surface area.
docs/filesystem-plugins.md Documents the filesystem plugin package layout, supported files, and loader API usage.
tests/plugins-loader.test.ts Adds Vitest coverage for loading plugins with skills/MCP/agents and the allowCodeEntrypoint + resolveModel behaviors.
CHANGELOG.md Records the new filesystem plugin loading APIs under Unreleased.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +71 to +78
## `plugin.js` (optional)

If `allowCodeEntrypoint: true` is passed to the loader, the entrypoint module is imported.

Supported exports:
- `default` object and/or named `plugin` object
- optional fields: `tools`, `skills`, `hooks`, `setup`, `deferred`, `subagent`, `mcpServer`, `mcpServers`

Copy link

Copilot AI Feb 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The plugin.js section implies ESM-style entrypoints, but in Node a standalone plugin.js outside an ESM package scope (no nearby package.json with "type": "module") will be treated as CommonJS, making import ... syntax fail. Consider documenting the required module format (CJS vs ESM) and/or recommending plugin.mjs or providing a package.json in the plugin folder if authors want ESM syntax.

Copilot uses AI. Check for mistakes.
## Security Defaults

- Code entrypoints are disabled by default (`allowCodeEntrypoint: false`).
- Manifest/MCP/agent parse errors are non-fatal and reported in `errors`.
Copy link

Copilot AI Feb 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doc claims “Manifest/MCP/agent parse errors are non-fatal and reported in errors”, but loadPluginFromDirectory() currently throws on invalid/missing plugin.json because readManifestFile() isn’t caught. Either adjust the implementation to surface manifest errors in PluginLoadResult.errors, or update this bullet to clarify that manifest failures are fatal for the single-directory API (and only non-fatal when using loadPluginsFromDirectories() which catches per-package failures).

Suggested change
- Manifest/MCP/agent parse errors are non-fatal and reported in `errors`.
- When using `loadPluginsFromDirectories()`, manifest/MCP/agent parse errors are non-fatal and reported in `errors`; `loadPluginFromDirectory()` will throw on manifest failures.

Copilot uses AI. Check for mistakes.
Comment thread src/plugins/loader.ts
Comment on lines +579 to +586
function validateManifest(manifest: FilesystemPluginManifest, manifestPath: string): void {
if (!manifest.name) {
throw new Error(`Invalid plugin manifest at '${manifestPath}': 'name' is required`);
}

if (!/^[a-z0-9-]+$/.test(manifest.name)) {
throw new Error(
`Invalid plugin manifest at '${manifestPath}': name must contain lowercase letters, numbers, and hyphens`,
Copy link

Copilot AI Feb 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

validateManifest() only validates name, but entrypoint, skillsDir, and agentsDir are later passed into resolve()/join(). Because join()/resolve() will honor absolute paths and .. segments, a plugin manifest can cause the loader to read/import files outside the plugin directory (even when allowCodeEntrypoint is false via skillsDir/agentsDir). Consider validating these fields when validate is true: require relative paths and ensure the resolved target stays within the plugin package directory (e.g., using path.relative() checks) before using them.

Copilot uses AI. Check for mistakes.
Comment thread src/plugins/loader.ts

const pluginDir = resolve(directory);
const manifestPath = join(pluginDir, "plugin.json");
const manifest = await readManifestFile(manifestPath, validate);
Copy link

Copilot AI Feb 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

loadPluginFromDirectory() is documented as returning “non-fatal errors”, but readManifestFile() can throw (missing/invalid plugin.json, validation failures) and this function doesn’t catch it, so callers get a rejected promise rather than a PluginLoadResult.errors entry. Consider wrapping manifest loading/validation in a try/catch and returning { plugins: [], agents: [], subagents: [], errors: [...] } for consistency with the rest of the loader error-handling (and with the docs).

Suggested change
const manifest = await readManifestFile(manifestPath, validate);
let manifest;
try {
manifest = await readManifestFile(manifestPath, validate);
} catch (error) {
errors.push({
message:
error instanceof Error
? error.message
: "Failed to load plugin manifest",
} as PluginLoadError);
return {
plugins: [],
agents: [],
subagents: [],
errors,
};
}

Copilot uses AI. Check for mistakes.
Comment thread src/plugins/loader.ts
Comment on lines +906 to +912
function parseMarkdownFrontmatter<TFrontmatter>(content: string): ParsedMarkdownFile<TFrontmatter> {
const frontmatterRegex = /^---\n([\s\S]*?)\n---\n([\s\S]*)$/;
const match = content.match(frontmatterRegex);

if (!match) {
throw new Error("Missing YAML frontmatter (must start and end with ---)");
}
Copy link

Copilot AI Feb 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

parseMarkdownFrontmatter() only matches \n line endings (/^---\n.../). Markdown files on Windows commonly use \r\n, which will make otherwise valid agent files fail to parse. Consider accepting both newline styles (e.g., \r?\n) or normalizing line endings before applying the regex.

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants