Skip to content

fix(opencode): prevent plugin deduplication collision for index.js entry points#11161

Open
guazi04 wants to merge 3 commits intoanomalyco:devfrom
guazi04:fix/plugin-deduplication-collision
Open

fix(opencode): prevent plugin deduplication collision for index.js entry points#11161
guazi04 wants to merge 3 commits intoanomalyco:devfrom
guazi04:fix/plugin-deduplication-collision

Conversation

@guazi04
Copy link

@guazi04 guazi04 commented Jan 29, 2026

Issue for this PR

Closes #11159

Type of change

  • Bug fix
  • New feature
  • Refactor / code improvement
  • Documentation

What does this PR do?

getPluginName() returns only the filename for file:// URLs. Plugins with index.js/index.ts entry points all return "index", causing incorrect deduplication — earlier plugins are silently dropped.

Solution: 3-tier name resolution for file:// URLs

  1. package.json lookup — Walk up (max 5 levels) to find the nearest package.json with a name field. Stops at .opencode boundary to avoid picking up the host project's name for .opencode/plugin/* scripts.
  2. Filename fallback — If no package.json found and filename isn't "index", use the filename.
  3. Directory heuristic — For "index" entry points, walk up directories skipping generic names (src/dist/lib/build/out/esm/cjs).

Also uses fileURLToPath() instead of new URL(...).pathname for cross-platform correctness (Windows paths, URL-encoded chars).

How does this compare to PR #8758?

Similar strategy (package.json + heuristic fallback), but:

  • Adds .opencode boundary guard (prevents host project's package.json from breaking local plugin dedup)
  • Simpler skip set (Set vs rolling heuristic)
  • Rebased on latest dev

How did you verify your code works?

  • 14 tests pass (6 new), including 2 filesystem-backed tests:
    • .opencode/plugin/a.js + b.js with host package.json → both resolve to filename, not project name
    • Real plugin with package.json + src/index.ts → resolves to package name
  • Existing deduplication and override tests still pass
  • Oracle-reviewed for correctness, boundary edge cases, and override semantics

Checklist

  • I have tested my changes locally
  • I have not included unrelated changes in this PR

@github-actions
Copy link
Contributor

The following comment was made by an LLM, it may be inaccurate:

Potential Duplicate Found:

Why it's related: This PR also addresses file:// URL plugin deduplication issues. It may be a previous attempt at fixing a similar problem, so check if #8758 was closed/abandoned or if it's a different approach to the same root cause (issue #11159).

…try points

getPluginName() now uses a 3-tier resolution for file:// URLs:
1. Walk up to find package.json name (stops at .opencode boundary)
2. Use filename if not "index"
3. For index entry points, walk up skipping generic dirs (src/dist/lib/build/out/esm/cjs)

Also uses fileURLToPath() for cross-platform correctness.

Fixes anomalyco#11159
@guazi04 guazi04 force-pushed the fix/plugin-deduplication-collision branch from ed97ab6 to 3682053 Compare March 4, 2026 12:52
guazi04 added 2 commits March 4, 2026 21:03
file:// URLs always use forward slashes, but fileURLToPath() converts
them to backslashes on Windows. Use path.posix for URL pathname parsing
(directory walking heuristic) while keeping fileURLToPath() for actual
filesystem operations in findPackageName().
…URL helper

Replace hardcoded Unix file:// URLs with pathToFileURL(path.resolve())
so fileURLToPath() receives valid absolute paths on Windows.
@cyberprophet
Copy link

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 node_modules paths (handling @scoped packages too) while keeping the existing filename fallback for local plugins. CI is passing.

This preserves the original design intent where a local my-plugin.js can override an npm my-plugin@1.0.0 by name, while fixing the index.js collision.

@guazi04
Copy link
Author

guazi04 commented Mar 4, 2026

Thanks for the heads-up @cyberprophet! Nice approach with the node_modules path parsing — clean and lightweight 👍

Our PR takes a slightly broader approach since the index.js collision can also happen with local file:// plugins (not just npm ones resolved through node_modules). For example:

file:///project/plugins/alpha/src/index.ts
file:///project/plugins/beta/dist/index.js

Both would still resolve to index with filename-only fallback. So we added:

  • findPackageName() — walks up to find the nearest package.json name (with a .opencode boundary guard)
  • A directory heuristic that skips generic dirs like src/dist/lib/build to find meaningful names

Definitely some overlap though! Might be worth letting the maintainers decide which trade-off they prefer — minimal and targeted vs. broader coverage. Either way the core node_modules case gets fixed 🙂

@cyberprophet
Copy link

Good point @guazi04 — the local file:// collision case (plugins/alpha/src/index.ts vs plugins/beta/dist/index.js) is a real scenario I hadn't covered.

That said, I intentionally kept #15598 minimal for a couple of reasons:

  1. The node_modules case is the silent killer. Users don't control how import.meta.resolve rewrites npm specifiers to file:// URLs — the collision happens invisibly. Local file:// plugins are authored directly, so the user has more control over filenames.

  2. findPackageName() with fs walks adds runtime I/O to a hot path (config boot). The node_modules path-parsing approach is pure string manipulation — zero fs calls.

  3. Smaller diff = easier review. The broader fix can always land as a follow-up.

But you're right that both approaches fix the core issue. Happy to let @rekram1-node decide which trade-off fits better — or if a combined approach makes sense.

@guazi04
Copy link
Author

guazi04 commented Mar 4, 2026

Haha to be honest, I just noticed this issue had been open for a while with no fix landed, so I figured I'd take a crack at it. No strong attachment to which PR gets merged — as long as the collision bug gets resolved, that's a win for everyone 🎉

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.

Plugin deduplication incorrectly removes plugins with index.js entry points

2 participants