Skip to content

Fix local plugin loading to preserve OAuth assets #287

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
shuv1337 merged 3 commits into from
Jan 10, 2026
Merged

Fix local plugin loading to preserve OAuth assets #287

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
48605a5
fix: load local plugins without bundling
shuv1337 Jan 10, 2026
ca170c4
fix: address typecheck issues
shuv1337 Jan 10, 2026
b89bc70
fix: guard undefined auth methods
shuv1337 Jan 10, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion packages/opencode/src/command/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import { Plugin } from "../plugin"
import PROMPT_INITIALIZE from "./template/initialize.txt"
import PROMPT_REVIEW from "./template/review.txt"
import { MCP } from "../mcp"
import type { Hooks } from "@opencode-ai/plugin"

export namespace Command {
export const Event = {
Expand Down Expand Up @@ -129,7 +130,7 @@ export namespace Command {
// Plugin commands
const plugins = await Plugin.list()
for (const plugin of plugins) {
const commands = plugin["plugin.command"]
const commands: NonNullable<Hooks["plugin.command"]> | undefined = plugin["plugin.command"]
if (!commands) continue
for (const [name, cmd] of Object.entries(commands)) {
if (result[name]) continue // Don't override existing commands
Expand Down
36 changes: 27 additions & 9 deletions packages/opencode/src/plugin/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,14 +83,18 @@ export namespace Plugin {
}
}

function isModuleResolutionError(err: Error): boolean {
return err.message?.includes("Cannot find module") || err.message?.includes("Cannot find package")
}
Comment on lines +86 to +88
Copy link

@greptile-apps greptile-apps bot Jan 10, 2026

Choose a reason for hiding this comment

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

The isModuleResolutionError function uses string matching to detect module resolution errors. While this works for common cases, it could potentially match false positives where a plugin's error message contains these strings but isn't actually a module resolution error.

Consider also checking the error type or code if available:

Suggested change
function isModuleResolutionError(err: Error): boolean {
return err.message?.includes("Cannot find module") || err.message?.includes("Cannot find package")
}
function isModuleResolutionError(err: Error): boolean {
return (
err.message?.includes("Cannot find module") ||
err.message?.includes("Cannot find package") ||
(err as any).code === "MODULE_NOT_FOUND"
)
}

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Prompt To Fix With AI 
This is a comment left during a code review.
Path: packages/opencode/src/plugin/index.ts
Line: 86:88

Comment:
The `isModuleResolutionError` function uses string matching to detect module resolution errors. While this works for common cases, it could potentially match false positives where a plugin's error message contains these strings but isn't actually a module resolution error.

Consider also checking the error type or code if available:

```suggestion
  function isModuleResolutionError(err: Error): boolean {
    return (
      err.message?.includes("Cannot find module") || 
      err.message?.includes("Cannot find package") ||
      (err as any).code === "MODULE_NOT_FOUND"
    )
  }
```


<sub>Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!</sub>

How can I resolve this? If you propose a fix, please make it concise.


const state = Instance.state(async () => {
const client = createOpencodeClient({
baseUrl: "http://localhost:4096",
// @ts-ignore - fetch type incompatibility
fetch: async (...args) => Server.App().fetch(...args),
})
const config = await Config.get()
const hooks = []
const hooks: Hooks[] = []
const input: PluginInput = {
client,
project: Instance.project,
Expand All @@ -106,6 +110,7 @@ export namespace Plugin {
for (let plugin of plugins) {
log.info("loading plugin", { path: plugin })
let pluginUrl: string
let localPluginPath: string | undefined
if (!plugin.startsWith("file://")) {
const lastAtIndex = plugin.lastIndexOf("@")
const pkg = lastAtIndex > 0 ? plugin.substring(0, lastAtIndex) : plugin
Expand All @@ -122,14 +127,14 @@ export namespace Plugin {
// Resolve relative file:// paths against the working directory
const filePath = plugin.substring("file://".length)
const absolutePath = path.isAbsolute(filePath) ? filePath : path.resolve(Instance.directory, filePath)
// Bundle local plugins with their dependencies for compiled binary compatibility
const bundledPath = await bundleLocalPlugin(absolutePath)
pluginUrl = pathToFileURL(bundledPath).href
localPluginPath = absolutePath
pluginUrl = pathToFileURL(absolutePath).href
}
try {

const loadPluginModule = async (url: string) => {
// Use dynamic import() with absolute file:// URLs for ES module compatibility
// pathToFileURL ensures proper URL encoding regardless of import.meta.url context
const mod = await import(pluginUrl)
const mod = await import(url)
// Prevent duplicate initialization when plugins export the same function
// as both a named export and default export (e.g., `export const X` and `export default X`).
// Object.entries(mod) would return both entries pointing to the same function reference.
Expand All @@ -141,10 +146,23 @@ export namespace Plugin {
const init = await fn(input)
hooks.push(init)
}
}

try {
await loadPluginModule(pluginUrl)
} catch (e) {
const err = e as Error
if (localPluginPath && isModuleResolutionError(err)) {
log.warn("failed to load local plugin directly, bundling fallback", {
plugin,
error: err.message,
})
const bundledPath = await bundleLocalPlugin(localPluginPath)
await loadPluginModule(pathToFileURL(bundledPath).href)
continue
Comment on lines +155 to +162
Copy link

@greptile-apps greptile-apps bot Jan 10, 2026

Choose a reason for hiding this comment

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

The second loadPluginModule call on line 161 is not wrapped in a try-catch block. If the bundled plugin fails to load (for any reason - not just module resolution errors), the error will not be caught, and it will propagate uncaught, bypassing the error logging and hint system below.

This means:

  1. The error won't be logged with the helpful "Make sure all plugin dependencies are installed" hint
  2. The entire plugin loading will fail instead of just this plugin
  3. Debugging will be harder because the error won't go through the standardized error handling
Suggested change
if (localPluginPath && isModuleResolutionError(err)) {
log.warn("failed to load local plugin directly, bundling fallback", {
plugin,
error: err.message,
})
const bundledPath = await bundleLocalPlugin(localPluginPath)
await loadPluginModule(pathToFileURL(bundledPath).href)
continue
if (localPluginPath && isModuleResolutionError(err)) {
log.warn("failed to load local plugin directly, bundling fallback", {
plugin,
error: err.message,
})
try {
const bundledPath = await bundleLocalPlugin(localPluginPath)
await loadPluginModule(pathToFileURL(bundledPath).href)
continue
} catch (bundleErr) {
const bErr = bundleErr as Error
log.error("failed to load bundled plugin", {
plugin,
error: bErr.message,
})
throw bErr
}
}
Prompt To Fix With AI 
This is a comment left during a code review.
Path: packages/opencode/src/plugin/index.ts
Line: 155:162

Comment:
The second `loadPluginModule` call on line 161 is not wrapped in a try-catch block. If the bundled plugin fails to load (for any reason - not just module resolution errors), the error will not be caught, and it will propagate uncaught, bypassing the error logging and hint system below.

This means:
1. The error won't be logged with the helpful "Make sure all plugin dependencies are installed" hint
2. The entire plugin loading will fail instead of just this plugin
3. Debugging will be harder because the error won't go through the standardized error handling

```suggestion
        if (localPluginPath && isModuleResolutionError(err)) {
          log.warn("failed to load local plugin directly, bundling fallback", {
            plugin,
            error: err.message,
          })
          try {
            const bundledPath = await bundleLocalPlugin(localPluginPath)
            await loadPluginModule(pathToFileURL(bundledPath).href)
            continue
          } catch (bundleErr) {
            const bErr = bundleErr as Error
            log.error("failed to load bundled plugin", {
              plugin,
              error: bErr.message,
            })
            throw bErr
          }
        }
```


How can I resolve this? If you propose a fix, please make it concise.

}
// Check for module resolution issues
if (err.message?.includes("Cannot find module") || err.message?.includes("Cannot find package")) {
if (isModuleResolutionError(err)) {
log.error("failed to load plugin", {
plugin,
error: err.message,
Expand Down Expand Up @@ -175,7 +193,7 @@ export namespace Plugin {
for (const hook of await state().then((x) => x.hooks)) {
const fn = hook[name]
if (!fn) continue
// @ts-expect-error if you feel adventurous, please fix the typing, make sure to bump the try-counter if you
// @ts-ignore if you feel adventurous, please fix the typing, make sure to bump the try-counter if you
// give up.
// try-counter: 2
await fn(input, output)
Comment on lines +196 to 199
Copy link

@greptile-apps greptile-apps bot Jan 10, 2026

Choose a reason for hiding this comment

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

Changing from @ts-expect-error to @ts-ignore makes the type checking less strict. @ts-expect-error will warn if there's no type error, helping catch when the typing issue is fixed. @ts-ignore will silently suppress errors even when the typing is correct.

While both work, @ts-expect-error is generally preferred because it provides better type safety feedback. Was there a specific reason for this change, or should we keep the stricter @ts-expect-error?

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Prompt To Fix With AI 
This is a comment left during a code review.
Path: packages/opencode/src/plugin/index.ts
Line: 196:199

Comment:
Changing from `@ts-expect-error` to `@ts-ignore` makes the type checking less strict. `@ts-expect-error` will warn if there's no type error, helping catch when the typing issue is fixed. `@ts-ignore` will silently suppress errors even when the typing is correct.

While both work, `@ts-expect-error` is generally preferred because it provides better type safety feedback. Was there a specific reason for this change, or should we keep the stricter `@ts-expect-error`?

<sub>Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!</sub>

How can I resolve this? If you propose a fix, please make it concise.

Expand All @@ -195,7 +213,7 @@ export namespace Plugin {
const hooks = await state().then((x) => x.hooks)
const config = await Config.get()
for (const hook of hooks) {
// @ts-expect-error this is because we haven't moved plugin to sdk v2
// @ts-ignore this is because we haven't moved plugin to sdk v2
Copy link

@greptile-apps greptile-apps bot Jan 10, 2026

Choose a reason for hiding this comment

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

Same concern as the previous @ts-ignore comment - @ts-expect-error would be stricter and provide better feedback when the typing is eventually fixed.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Prompt To Fix With AI 
This is a comment left during a code review.
Path: packages/opencode/src/plugin/index.ts
Line: 216:216

Comment:
Same concern as the previous `@ts-ignore` comment - `@ts-expect-error` would be stricter and provide better feedback when the typing is eventually fixed.

<sub>Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!</sub>

How can I resolve this? If you propose a fix, please make it concise.

await hook.config?.(config)
}
Bus.subscribeAll(async (input) => {
Expand Down
15 changes: 8 additions & 7 deletions packages/opencode/src/provider/auth.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,14 +30,15 @@ export namespace ProviderAuth {

export async function methods() {
const s = await state().then((x) => x.methods)
return mapValues(s, (x) =>
x.methods.map(
(y): Method => ({
type: y.type,
label: y.label,
return mapValues(s, (x: Hooks["auth"] | undefined) => {
if (!x) return []
return x.methods.map(
(method): Method => ({
type: method.type,
label: method.label,
}),
),
)
)
})
}

export const Authorization = z
Expand Down
4 changes: 3 additions & 1 deletion packages/opencode/src/tool/registry.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,9 @@ export namespace ToolRegistry {

const plugins = await Plugin.list()
for (const plugin of plugins) {
for (const [id, def] of Object.entries(plugin.tool ?? {})) {
const tools = plugin.tool as Record<string, ToolDefinition> | undefined
if (!tools) continue
for (const [id, def] of Object.entries(tools)) {
custom.push(fromPlugin(id, def))
}
}
Expand Down
Loading