fix: use full URL for file:// plugin deduplication#8758
fix: use full URL for file:// plugin deduplication#8758coleleavitt wants to merge 6 commits intoanomalyco:devfrom
Conversation
Previously, file:// plugins were deduplicated by filename only, causing issues when multiple plugins used standard entry points like index.js. Example problem: file:///path/to/plugin-a/dist/index.js -> 'index' file:///path/to/plugin-b/dist/index.js -> 'index' Result: Only last plugin loaded (seen as duplicates) This change uses the full URL as the identity for file:// plugins, allowing multiple plugins in the same or different directories with the same filename. Fixes deduplication logic to support: - Multiple plugins in same directory (github-tools.js, db-tools.js) - Multiple plugins with standard entry points (all named index.js) - Same file:// URL appearing twice still deduplicates correctly npm packages continue to deduplicate by package name as expected.
|
Thanks for your contribution! This PR doesn't have a linked issue. All PRs must reference an existing issue. Please:
See CONTRIBUTING.md for details. |
|
The following comment was made by an LLM, it may be inaccurate: No duplicate PRs found |
|
Fixes #8759 |
| if (plugin.startsWith("file://")) { | ||
| return path.parse(new URL(plugin).pathname).name | ||
| return plugin |
There was a problem hiding this comment.
But now someone w/ oh-my-opencode installed that is working in oh-my-opencode repo will have the plugn registered twice right
Fixes plugin deduplication issue where multiple file:// plugins with generic filenames (index.js, main.js) would collide. Now extracts the parent directory name when filename is generic (index, main, plugin, dist, build, out, lib), allowing both proper deduplication of npm vs file:// versions of the same plugin AND multiple generic-named plugins in different directories. Examples: - file:///.../plugin-a/dist/index.js -> 'plugin-a' - file:///.../plugin-b/dist/index.js -> 'plugin-b' - file:///.../oh-my-opencode/dist/index.js -> 'oh-my-opencode' - oh-my-opencode@2.4.3 -> 'oh-my-opencode' (deduplicates correctly) Addresses reviewer concern about oh-my-opencode npm + file:// loading twice during local development.
Primary method now looks for package.json in parent directories (up to 5 levels) and extracts the 'name' field. Falls back to the heuristic approach (filename or parent directory) when package.json is not found. This is more robust because: - Uses canonical package name from package.json - Handles scoped packages correctly (@scope/name) - Self-maintaining (no hardcoded list to update) - Works for real npm package development workflows Added tests for: - Extracting name from package.json - Preferring package.json name over directory name
… lookup - Use fileURLToPath() instead of URL.pathname for proper Windows support - Use path.sep for platform-specific path separators - Remove package.json lookup (overcomplicated per maintainer feedback) - Keep simple heuristic: skip generic names (index, dist, etc.) to find meaningful parent dir
|
@rekram1-node Re: your review comment about duplicate registration when working in oh-my-opencode repo: Good catch. Yes, that's true - if someone has However, this is actually the existing behavior that my PR preserves. The deduplication happens by checking The alternative would be to resolve to an absolute canonical path and dedupe on that, but that adds complexity and might break legitimate use cases (symlinks, mounted volumes, etc.). Should I add canonical path resolution, or is the current behavior acceptable? |
…ames The package.json approach is superior to path heuristics because: 1. Cross-platform: Avoids path.sep issues that break on Windows 2. Canonical: Uses the actual package name, not guessed from paths 3. Deduplication: Fixes duplicate registration when same plugin loaded from different paths (e.g., global config + local repo) 4. Proper scoped package support: Handles @scope/name correctly Windows users report 40+ test failures with file:// plugins due to path.sep usage. Reading package.json solves this fundamental issue. Fallback to path heuristic when package.json not found.
|
Updated based on feedback from Discord user What ChangedRestored the package.json lookup that was previously removed. This is the correct approach. Why package.json is Better
The Windows Problem
The SolutionRead the canonical name from // file:///home/user/.opencode/plugins/oh-my-opencode/dist/index.js
// → reads package.json → "oh-my-opencode"
// file:///home/user/projects/oh-my-opencode/dist/index.js
// → reads package.json → "oh-my-opencode"
// Both dedupe correctly now!Fallback to path heuristic when This addresses both the Windows compatibility issue and the duplicate registration concern raised by @rekram1-node. |
00637c0 to
71e0ba2
Compare
f1ae801 to
08fa7f7
Compare
|
FYI — I've opened #15598 with a similar fix but a different approach: instead of using the full URL as identity, it extracts the npm package name from This preserves the original design intent where a local |
|
Superseded by #16200 which is rebased on latest dev and uses a cleaner implementation (package.json lookup + realpath fallback). |
|
Closing in favor of #16200 |
Summary
Fixes a bug in plugin deduplication logic where multiple file:// plugins with the same filename (e.g.,
index.js) would be incorrectly treated as duplicates and only the last one would load.Problem
The
getPluginName()function extracted only the filename for file:// URLs:This caused issues when using multiple plugins with standard entry points:
{ "plugin": [ "file:///path/to/plugin-a/dist/index.js", // -> "index" "file:///path/to/plugin-b/dist/index.js", // -> "index" (duplicate!) "file:///path/to/plugin-c/dist/index.js" // -> "index" (duplicate!) ] }Result: Only the last plugin loaded because all were seen as duplicates of "index".
Solution
Use the full URL as the identity for file:// plugins:
This allows:
Testing
Verified with real-world setup using four local file:// plugins, all with
index.jsorindex.mjsentry points:custom-toolkit-alpha/dist/index.jscustom-toolkit-beta/dist/index.jscustom-toolkit-gamma/dist/index.jscustom-auth-plugin/index.mjsBefore fix: Only the last plugin loaded.
After fix: All four plugins load correctly.