Explorar el Código

handle permanent file plugin errors (#27344)

Sebastian hace 3 meses
padre
commit
ef82426e28

+ 36 - 16
packages/opencode/src/plugin/loader.ts

@@ -56,6 +56,22 @@ export namespace PluginLoader {
     ) => void
   }
 
+  type AttemptResult<R> = {
+    value?: R
+    retry: boolean
+  }
+
+  function errorMessage(error: unknown) {
+    if (!error || typeof error !== "object") return ""
+    const message = "message" in error && typeof error.message === "string" ? error.message : ""
+    return message
+  }
+
+  function isRetryableResolveError(stage: "install" | "entry" | "compatibility", error: unknown) {
+    if (stage !== "install") return false
+    return errorMessage(error).includes("missing package.json or index file")
+  }
+
   // Normalize a config item into the loader's internal representation.
   function plan(item: ConfigPlugin.Spec): Plan {
     const spec = ConfigPlugin.pluginSpecifier(item)
@@ -136,11 +152,12 @@ export namespace PluginLoader {
     finish: ((load: Loaded, origin: ConfigPlugin.Origin, retry: boolean) => Promise<R | undefined>) | undefined,
     missing: ((value: Missing, origin: ConfigPlugin.Origin, retry: boolean) => Promise<R | undefined>) | undefined,
     report: Report | undefined,
-  ): Promise<R | undefined> {
+  ): Promise<AttemptResult<R>> {
     const plan = candidate.plan
+    const filePlugin = pluginSource(plan.spec) === "file"
 
     // Deprecated plugin packages are silently ignored because they are now built in.
-    if (plan.deprecated) return
+    if (plan.deprecated) return { retry: false }
 
     report?.start?.(candidate, retry)
 
@@ -151,25 +168,26 @@ export namespace PluginLoader {
         // for example to load theme files from a tui plugin package that has no code entrypoint.
         if (missing) {
           const value = await missing(resolved.value, candidate.origin, retry)
-          if (value !== undefined) return value
+          if (value !== undefined) return { value, retry: false }
         }
         report?.missing?.(candidate, retry, resolved.value.message, resolved.value)
-        return
+        return { retry: false }
       }
       report?.error?.(candidate, retry, resolved.stage, resolved.error)
-      return
+      return { retry: filePlugin && isRetryableResolveError(resolved.stage, resolved.error) }
     }
 
     const loaded = await load(resolved.value)
     if (!loaded.ok) {
       report?.error?.(candidate, retry, "load", loaded.error, resolved.value)
-      return
+      return { retry: false }
     }
 
     // The default behavior is to return the successfully loaded plugin as-is, but callers can
     // provide a finisher to adapt the result into a more specific runtime shape.
-    if (!finish) return loaded.value as R
-    return finish(loaded.value, candidate.origin, retry)
+    if (!finish) return { value: loaded.value as R, retry: false }
+    const value = await finish(loaded.value, candidate.origin, retry)
+    return { value, retry: false }
   }
 
   type Input<R> = {
@@ -183,12 +201,12 @@ export namespace PluginLoader {
 
   // Resolve and load all configured plugins in parallel.
   //
-  // If `wait` is provided, file-based plugins that initially failed are retried once after the
-  // caller finishes preparing dependencies. This supports local plugins that depend on an install
-  // step happening elsewhere before their entrypoint becomes loadable.
+  // If `wait` is provided, file-based plugins with retryable pre-import setup failures are retried
+  // once after the caller finishes preparing dependencies. Once dynamic import runs, failures are
+  // treated as permanent for this process because Bun caches failed module resolution.
   export async function loadExternal<R = Loaded>(input: Input<R>): Promise<R[]> {
     const candidates = input.items.map((origin) => ({ origin, plan: plan(origin.spec) }))
-    const list: Array<Promise<R | undefined>> = []
+    const list: Array<Promise<AttemptResult<R>>> = []
     for (const candidate of candidates) {
       list.push(attempt(candidate, input.kind, false, input.finish, input.missing, input.report))
     }
@@ -196,10 +214,12 @@ export namespace PluginLoader {
     if (input.wait) {
       let deps: Promise<void> | undefined
       for (let i = 0; i < candidates.length; i++) {
-        if (out[i] !== undefined) continue
+        const previous = out[i]
+        if (previous?.value !== undefined) continue
+        if (previous?.retry !== true) continue
 
-        // Only local file plugins are retried. npm plugins already attempted installation during
-        // the first pass, while file plugins may need the caller's dependency preparation to finish.
+        // Only pre-import file plugin setup failures are retried. Bun caches failed dynamic imports,
+        // so dependency waiting cannot fix load/build/runtime/shape failures in this process.
         const candidate = candidates[i]
         if (!candidate || pluginSource(candidate.plan.spec) !== "file") continue
         deps ??= input.wait()
@@ -210,7 +230,7 @@ export namespace PluginLoader {
 
     // Drop skipped/failed entries while preserving the successful result order.
     const ready: R[] = []
-    for (const item of out) if (item !== undefined) ready.push(item)
+    for (const item of out) if (item.value !== undefined) ready.push(item.value)
     return ready
   }
 }

+ 169 - 0
packages/opencode/test/cli/tui/plugin-loader.test.ts

@@ -10,12 +10,116 @@ import { createTuiResolvedConfig, mockTuiRuntime } from "../../fixture/tui-runti
 import { Global } from "@opencode-ai/core/global"
 import { TuiConfig } from "../../../src/cli/cmd/tui/config/tui"
 import { Filesystem } from "@/util/filesystem"
+import { PluginLoader } from "../../../src/plugin/loader"
 
 const { allThemes, addTheme } = await import("../../../src/cli/cmd/tui/context/theme")
 const { TuiPluginRuntime } = await import("../../../src/cli/cmd/tui/plugin/runtime")
 
 type Row = Record<string, unknown>
 
+test("does not retry permanent file plugin load errors", async () => {
+  await using tmp = await tmpdir({
+    init: async (dir) => {
+      const file = path.join(dir, "binary-plugin")
+      await Bun.write(file, new Uint8Array([0xcf, 0xfa, 0xed, 0xfe, 0x0c, 0x00, 0x00, 0x01]))
+      return { spec: pathToFileURL(file).href }
+    },
+  })
+
+  let waited = false
+  const calls: Array<["start" | "error", boolean, string?]> = []
+  const plugins = await PluginLoader.loadExternal({
+    items: [{ spec: tmp.extra.spec, scope: "local", source: path.join(tmp.path, "tui.json") }],
+    kind: "tui",
+    wait: async () => {
+      waited = true
+    },
+    report: {
+      start(_candidate, retry) {
+        calls.push(["start", retry])
+      },
+      error(_candidate, retry, stage) {
+        calls.push(["error", retry, stage])
+      },
+    },
+  })
+
+  expect(plugins).toEqual([])
+  expect(waited).toBe(false)
+  expect(calls).toEqual([
+    ["start", false],
+    ["error", false, "load"],
+  ])
+})
+
+test("does not retry file plugin load errors caused by missing modules", async () => {
+  await using tmp = await tmpdir({
+    init: async (dir) => {
+      const file = path.join(dir, "missing-dependency-plugin.ts")
+      const dep = path.join(dir, "dep.ts")
+      await Bun.write(
+        file,
+        `import value from "./dep"
+export default { id: "demo.retry.load", tui: async () => {}, value }
+`,
+      )
+      return { spec: pathToFileURL(file).href, dep }
+    },
+  })
+
+  let waited = false
+  const calls: Array<["start" | "error", boolean, string?]> = []
+  const plugins = await PluginLoader.loadExternal({
+    items: [{ spec: tmp.extra.spec, scope: "local", source: path.join(tmp.path, "tui.json") }],
+    kind: "tui",
+    wait: async () => {
+      waited = true
+      await Bun.write(tmp.extra.dep, `export default "ready"\n`)
+    },
+    finish: async (loaded, _origin, retry) => ({
+      retry,
+      value: (loaded.mod.default as { value: string }).value,
+    }),
+    report: {
+      start(_candidate, retry) {
+        calls.push(["start", retry])
+      },
+      error(_candidate, retry, stage) {
+        calls.push(["error", retry, stage])
+      },
+    },
+  })
+
+  expect(waited).toBe(false)
+  expect(calls).toEqual([
+    ["start", false],
+    ["error", false, "load"],
+  ])
+  expect(plugins).toEqual([])
+})
+
+test("does not retry top-level plugin errors that look like resolver messages", async () => {
+  await using tmp = await tmpdir({
+    init: async (dir) => {
+      const file = path.join(dir, "throwing-plugin.ts")
+      await Bun.write(file, `throw new Error("Cannot find package intentional")\n`)
+      return { spec: pathToFileURL(file).href }
+    },
+  })
+
+  let waited = false
+  const plugins = await PluginLoader.loadExternal({
+    items: [{ spec: tmp.extra.spec, scope: "local", source: path.join(tmp.path, "tui.json") }],
+    kind: "tui",
+    wait: async () => {
+      waited = true
+    },
+  })
+
+  expect(plugins).toEqual([])
+  expect(waited).toBe(false)
+})
+
 type Data = {
   local: Row
   global: Row
@@ -559,6 +663,71 @@ test("continues loading when a plugin is missing config metadata", async () => {
   }
 })
 
+test("does not wait on permanent tui plugin startup failures", async () => {
+  await using tmp = await tmpdir({
+    init: async (dir) => {
+      const binary = path.join(dir, "binary-plugin")
+      const invalidShape = path.join(dir, "invalid-shape-plugin.ts")
+      const missingID = path.join(dir, "missing-id-plugin.ts")
+      const good = path.join(dir, "good-plugin.ts")
+      const marker = path.join(dir, "good-called.txt")
+
+      await Bun.write(binary, new Uint8Array([0xcf, 0xfa, 0xed, 0xfe, 0x0c, 0x00, 0x00, 0x01]))
+      await Bun.write(invalidShape, `export default { id: "demo.invalid.shape" }\n`)
+      await Bun.write(missingID, `export default { tui: async () => {} }\n`)
+      await Bun.write(
+        good,
+        `export default {
+  id: "demo.good.after-bad",
+  tui: async () => {
+    await Bun.write(${JSON.stringify(marker)}, "called")
+  },
+}
+`,
+      )
+
+      return {
+        binarySpec: pathToFileURL(binary).href,
+        invalidShapeSpec: pathToFileURL(invalidShape).href,
+        missingIDSpec: pathToFileURL(missingID).href,
+        goodSpec: pathToFileURL(good).href,
+        marker,
+      }
+    },
+  })
+
+  process.env.OPENCODE_PLUGIN_META_FILE = path.join(tmp.path, "plugin-meta.json")
+  const wait = spyOn(TuiConfig, "waitForDependencies").mockResolvedValue()
+  const cwd = spyOn(process, "cwd").mockImplementation(() => tmp.path)
+
+  try {
+    await TuiPluginRuntime.init({
+      api: createTuiPluginApi(),
+      config: createTuiResolvedConfig({
+        plugin: [tmp.extra.binarySpec, tmp.extra.invalidShapeSpec, tmp.extra.missingIDSpec, tmp.extra.goodSpec],
+        plugin_origins: [
+          { spec: tmp.extra.binarySpec, scope: "local", source: path.join(tmp.path, "tui.json") },
+          { spec: tmp.extra.invalidShapeSpec, scope: "local", source: path.join(tmp.path, "tui.json") },
+          { spec: tmp.extra.missingIDSpec, scope: "local", source: path.join(tmp.path, "tui.json") },
+          { spec: tmp.extra.goodSpec, scope: "local", source: path.join(tmp.path, "tui.json") },
+        ],
+      }),
+    })
+
+    expect(wait).toHaveBeenCalledTimes(0)
+    await expect(fs.readFile(tmp.extra.marker, "utf8")).resolves.toBe("called")
+    expect(TuiPluginRuntime.list().find((item) => item.id === "demo.good.after-bad")?.active).toBe(true)
+    expect(TuiPluginRuntime.list().some((item) => item.spec === tmp.extra.binarySpec)).toBe(false)
+    expect(TuiPluginRuntime.list().some((item) => item.spec === tmp.extra.invalidShapeSpec)).toBe(false)
+    expect(TuiPluginRuntime.list().some((item) => item.spec === tmp.extra.missingIDSpec)).toBe(false)
+  } finally {
+    await TuiPluginRuntime.dispose()
+    cwd.mockRestore()
+    wait.mockRestore()
+    delete process.env.OPENCODE_PLUGIN_META_FILE
+  }
+})
+
 test("initializes external tui plugins in config order", async () => {
   const globalJson = path.join(Global.Path.config, "tui.json")
   const globalJsonc = path.join(Global.Path.config, "tui.jsonc")

+ 50 - 10
packages/opencode/test/plugin/loader-shared.test.ts

@@ -1180,7 +1180,52 @@ export default {
     ),
   )
 
-  it.live("retries file plugins when finish returns undefined", () =>
+  it.live("does not retry permanent file plugin entry errors", () =>
+    withTmp(
+      async (dir) => {
+        const mod = path.join(dir, "bad-entry")
+        const spec = pathToFileURL(mod).href
+        await fs.mkdir(mod, { recursive: true })
+        await Bun.write(
+          path.join(mod, "package.json"),
+          JSON.stringify({ exports: { "./tui": "../outside.js" } }, null, 2),
+        )
+        return { spec }
+      },
+      (tmp) =>
+        Effect.gen(function* () {
+          let wait = 0
+          const errors: Array<[string, boolean]> = []
+
+          const loaded = yield* Effect.promise(() =>
+            PluginLoader.loadExternal({
+              items: [
+                {
+                  spec: tmp.extra.spec,
+                  scope: "local" as const,
+                  source: tmp.path,
+                },
+              ],
+              kind: "tui",
+              wait: async () => {
+                wait += 1
+              },
+              report: {
+                error(_candidate, retry, stage) {
+                  errors.push([stage, retry])
+                },
+              },
+            }),
+          )
+
+          expect(loaded).toEqual([])
+          expect(wait).toBe(0)
+          expect(errors).toEqual([["entry", false]])
+        }),
+    ),
+  )
+
+  it.live("does not retry file plugins when finish returns undefined", () =>
     withTmp(
       async (dir) => {
         const file = path.join(dir, "plugin.ts")
@@ -1206,20 +1251,15 @@ export default {
               wait: async () => {
                 wait += 1
               },
-              finish: async (load, _item, retry) => {
+              finish: async () => {
                 count += 1
-                if (!retry) return
-                return {
-                  retry,
-                  spec: load.spec,
-                }
               },
             }),
           )
 
-          expect(wait).toBe(1)
-          expect(count).toBe(2)
-          expect(loaded).toEqual([{ retry: true, spec: tmp.extra.spec }])
+          expect(wait).toBe(0)
+          expect(count).toBe(1)
+          expect(loaded).toEqual([])
         }),
     ),
   )