fix(plugin): parse package specifiers with npm-package-arg and sanitize win32 cache paths (#21135)

Author: Hona

bun.lock

@@ -54,6 +54,7 @@
     "@types/bun": "catalog:",
     "@types/cross-spawn": "catalog:",
     "@types/mime-types": "3.0.1",
+    "@types/npm-package-arg": "6.1.4",
     "@types/npmcli__arborist": "6.3.3",
     "@types/semver": "^7.5.8",
     "@types/turndown": "5.0.5",
@@ -135,6 +136,7 @@
     "jsonc-parser": "3.3.1",
     "mime-types": "3.0.2",
     "minimatch": "10.0.3",
+    "npm-package-arg": "13.0.2",
     "open": "10.1.2",
     "opencode-gitlab-auth": "2.0.1",
     "opencode-poe-auth": "0.0.1",

packages/opencode/package.json

@@ -11,6 +11,7 @@ import { Arborist } from "@npmcli/arborist"
 
 export namespace Npm {
   const log = Log.create({ service: "npm" })
+  const illegal = process.platform === "win32" ? new Set(["<", ">", ":", '"', "|", "?", "*"]) : undefined
 
   export const InstallFailedError = NamedError.create(
     "NpmInstallFailedError",
@@ -19,8 +20,13 @@ export namespace Npm {
     }),
   )
 
+  export function sanitize(pkg: string) {
+    if (!illegal) return pkg
+    return Array.from(pkg, (char) => (illegal.has(char) || char.charCodeAt(0) < 32 ? "_" : char)).join("")
+  }
+
   function directory(pkg: string) {
-    return path.join(Global.Path.cache, "packages", pkg)
+    return path.join(Global.Path.cache, "packages", sanitize(pkg))
   }
 
   function resolveEntryPoint(name: string, dir: string) {

packages/opencode/src/npm/index.ts

@@ -1,5 +1,6 @@
 import path from "path"
 import { fileURLToPath, pathToFileURL } from "url"
+import npa from "npm-package-arg"
 import semver from "semver"
 import { Npm } from "@/npm"
 import { Filesystem } from "@/util/filesystem"
@@ -12,11 +13,24 @@ export function isDeprecatedPlugin(spec: string) {
   return DEPRECATED_PLUGIN_PACKAGES.some((pkg) => spec.includes(pkg))
 }
 
+function parse(spec: string) {
+  try {
+    return npa(spec)
+  } catch {}
+}
+
 export function parsePluginSpecifier(spec: string) {
-  const lastAt = spec.lastIndexOf("@")
-  const pkg = lastAt > 0 ? spec.substring(0, lastAt) : spec
-  const version = lastAt > 0 ? spec.substring(lastAt + 1) : "latest"
-  return { pkg, version }
+  const hit = parse(spec)
+  if (hit?.type === "alias" && !hit.name) {
+    const sub = (hit as npa.AliasResult).subSpec
+    if (sub?.name) {
+      const version = !sub.rawSpec || sub.rawSpec === "*" ? "latest" : sub.rawSpec
+      return { pkg: sub.name, version }
+    }
+  }
+  if (!hit?.name) return { pkg: spec, version: "" }
+  if (hit.raw === hit.name) return { pkg: hit.name, version: "latest" }
+  return { pkg: hit.name, version: hit.rawSpec }
 }
 
 export type PluginSource = "file" | "npm"
@@ -190,9 +204,11 @@ export async function checkPluginCompatibility(target: string, opencodeVersion:
   }
 }
 
-export async function resolvePluginTarget(spec: string, parsed = parsePluginSpecifier(spec)) {
+export async function resolvePluginTarget(spec: string) {
   if (isPathPluginSpec(spec)) return resolvePathPluginTarget(spec)
-  const result = await Npm.add(parsed.pkg + "@" + parsed.version)
+  const hit = parse(spec)
+  const pkg = hit?.name && hit.raw === hit.name ? `${hit.name}@latest` : spec
+  const result = await Npm.add(pkg)
   return result.directory
 }
 

packages/opencode/src/plugin/shared.ts

@@ -0,0 +1,18 @@
+import { describe, expect, test } from "bun:test"
+import { Npm } from "../src/npm"
+
+const win = process.platform === "win32"
+
+describe("Npm.sanitize", () => {
+  test("keeps normal scoped package specs unchanged", () => {
+    expect(Npm.sanitize("@opencode/acme")).toBe("@opencode/acme")
+    expect(Npm.sanitize("@opencode/acme@1.0.0")).toBe("@opencode/acme@1.0.0")
+    expect(Npm.sanitize("prettier")).toBe("prettier")
+  })
+
+  test("handles git https specs", () => {
+    const spec = "acme@git+https://github.com/opencode/acme.git"
+    const expected = win ? "acme@git+https_//github.com/opencode/acme.git" : spec
+    expect(Npm.sanitize(spec)).toBe(expected)
+  })
+})

packages/opencode/test/npm.test.ts

@@ -0,0 +1,88 @@
+import { describe, expect, test } from "bun:test"
+import { parsePluginSpecifier } from "../../src/plugin/shared"
+
+describe("parsePluginSpecifier", () => {
+  test("parses standard npm package without version", () => {
+    expect(parsePluginSpecifier("acme")).toEqual({
+      pkg: "acme",
+      version: "latest",
+    })
+  })
+
+  test("parses standard npm package with version", () => {
+    expect(parsePluginSpecifier("acme@1.0.0")).toEqual({
+      pkg: "acme",
+      version: "1.0.0",
+    })
+  })
+
+  test("parses scoped npm package without version", () => {
+    expect(parsePluginSpecifier("@opencode/acme")).toEqual({
+      pkg: "@opencode/acme",
+      version: "latest",
+    })
+  })
+
+  test("parses scoped npm package with version", () => {
+    expect(parsePluginSpecifier("@opencode/acme@1.0.0")).toEqual({
+      pkg: "@opencode/acme",
+      version: "1.0.0",
+    })
+  })
+
+  test("parses package with git+https url", () => {
+    expect(parsePluginSpecifier("acme@git+https://github.com/opencode/acme.git")).toEqual({
+      pkg: "acme",
+      version: "git+https://github.com/opencode/acme.git",
+    })
+  })
+
+  test("parses scoped package with git+https url", () => {
+    expect(parsePluginSpecifier("@opencode/acme@git+https://github.com/opencode/acme.git")).toEqual({
+      pkg: "@opencode/acme",
+      version: "git+https://github.com/opencode/acme.git",
+    })
+  })
+
+  test("parses package with git+ssh url containing another @", () => {
+    expect(parsePluginSpecifier("acme@git+ssh://git@github.com/opencode/acme.git")).toEqual({
+      pkg: "acme",
+      version: "git+ssh://git@github.com/opencode/acme.git",
+    })
+  })
+
+  test("parses scoped package with git+ssh url containing another @", () => {
+    expect(parsePluginSpecifier("@opencode/acme@git+ssh://git@github.com/opencode/acme.git")).toEqual({
+      pkg: "@opencode/acme",
+      version: "git+ssh://git@github.com/opencode/acme.git",
+    })
+  })
+
+  test("parses unaliased git+ssh url", () => {
+    expect(parsePluginSpecifier("git+ssh://git@github.com/opencode/acme.git")).toEqual({
+      pkg: "git+ssh://git@github.com/opencode/acme.git",
+      version: "",
+    })
+  })
+
+  test("parses npm alias using the alias name", () => {
+    expect(parsePluginSpecifier("acme@npm:@opencode/acme@1.0.0")).toEqual({
+      pkg: "acme",
+      version: "npm:@opencode/acme@1.0.0",
+    })
+  })
+
+  test("parses bare npm protocol specifier using the target package", () => {
+    expect(parsePluginSpecifier("npm:@opencode/acme@1.0.0")).toEqual({
+      pkg: "@opencode/acme",
+      version: "1.0.0",
+    })
+  })
+
+  test("parses unversioned npm protocol specifier", () => {
+    expect(parsePluginSpecifier("npm:@opencode/acme")).toEqual({
+      pkg: "@opencode/acme",
+      version: "latest",
+    })
+  })
+})