diff --git a/packages/coding-agent/src/extensibility/plugins/manager.ts b/packages/coding-agent/src/extensibility/plugins/manager.ts index af5af87dd..18844a851 100644 --- a/packages/coding-agent/src/extensibility/plugins/manager.ts +++ b/packages/coding-agent/src/extensibility/plugins/manager.ts @@ -77,10 +77,13 @@ function gitInstallSpec(original: string, source: GitSource): string { return `${source.repo}#${source.ref}`; } -function findExistingGitPackageName(packageInstallSpec: string, deps: Record): string | undefined { - const needle = packageInstallSpec.replace(/^git\+/i, ""); +function findGitPackageName(source: GitSource, deps: Record): string | undefined { for (const [key, value] of Object.entries(deps)) { - if (typeof value === "string" && value.includes(needle)) { + if (typeof value !== "string") { + continue; + } + const installedSource = parseGitUrl(value); + if (installedSource && installedSource.host === source.host && installedSource.path === source.path) { return key; } } @@ -331,7 +334,7 @@ export class PluginManager { const depsBefore = await this.#readDeps(pkgJsonPath); const packageInstallSpec = gitSource ? gitInstallSpec(spec.packageName, gitSource) : spec.packageName; const existingActualName = gitSource - ? findExistingGitPackageName(packageInstallSpec, depsBefore) + ? findGitPackageName(gitSource, depsBefore) : extractPackageName(spec.packageName); const packageSnapshot = await this.#snapshotInstalledPackage(existingActualName); @@ -364,10 +367,10 @@ export class PluginManager { } // Fallback: a force-reinstall of an already-present git plugin will not // add a new key, just rewrite the existing one to the new spec value. - // Match by the install value for force-reinstalls where no new key is - // added (non-GitHub shorthands are normalized before bun sees them). + // Match by repository identity, not by ref, so failed upgrades from + // one ref to another still resolve to the original package name. if (!resolved) { - resolved = findExistingGitPackageName(packageInstallSpec, depsAfter); + resolved = findGitPackageName(gitSource, depsAfter); } if (!resolved) { throw new Error( diff --git a/packages/coding-agent/test/plugin-install-validation.test.ts b/packages/coding-agent/test/plugin-install-validation.test.ts index f8c3ea507..b81f8bd2a 100644 --- a/packages/coding-agent/test/plugin-install-validation.test.ts +++ b/packages/coding-agent/test/plugin-install-validation.test.ts @@ -165,6 +165,71 @@ describe("PluginManager.install load validation", () => { expect(lock.plugins["broken-plugin"]).toEqual({ version: "1.0.0", enabledFeatures: null, enabled: true }); }); + test("restores the previous git plugin tree when reinstalling a different ref fails validation", async () => { + await Bun.write( + pluginsPkgJson, + JSON.stringify( + { name: "omp-plugins", private: true, dependencies: { "git-plugin": "github:org/plugin#v1" } }, + null, + 2, + ), + ); + await Bun.write( + path.join(tmpRoot, "omp-plugins.lock.json"), + JSON.stringify( + { plugins: { "git-plugin": { version: "1.0.0", enabledFeatures: null, enabled: true } }, settings: {} }, + null, + 2, + ), + ); + await writePluginPackage(pluginsNodeModules, "git-plugin", { + version: "1.0.0", + source: 'export default function(pi) { pi.registerCommand("git-old-ok", { handler: async () => {} }); }\n', + }); + + vi.spyOn(Bun, "spawn").mockImplementation(((cmd: string[]) => { + expect(cmd).toEqual(["bun", "install", "github:org/plugin#v2"]); + + const prepare = (async () => { + await Bun.write( + pluginsPkgJson, + JSON.stringify( + { name: "omp-plugins", private: true, dependencies: { "git-plugin": "github:org/plugin#v2" } }, + null, + 2, + ), + ); + await writePluginPackage(pluginsNodeModules, "git-plugin", { + version: "2.0.0", + peerDependencies: { "missing-peer": "^1.0.0" }, + source: + 'import { missing } from "missing-peer";\nexport default function(pi) { pi.registerCommand(String(missing), { handler: async () => {} }); }\n', + }); + })(); + + return { + pid: 1, + stdout: emptyStream(), + stderr: emptyStream(), + exited: prepare.then(() => 0), + } as Subprocess; + }) as typeof Bun.spawn); + + await expect(new PluginManager(tmpRoot).install("github:org/plugin#v2")).rejects.toThrow(/missing-peer/); + + const pluginsPackage = await Bun.file(pluginsPkgJson).json(); + expect(pluginsPackage.dependencies).toEqual({ "git-plugin": "github:org/plugin#v1" }); + const restoredPackage = await Bun.file(path.join(pluginsNodeModules, "git-plugin", "package.json")).json(); + expect(restoredPackage.version).toBe("1.0.0"); + const restoredExtension = await Bun.file( + path.join(pluginsNodeModules, "git-plugin", "dist", "extension.ts"), + ).text(); + expect(restoredExtension).toContain("git-old-ok"); + expect(restoredExtension).not.toContain("missing-peer"); + const lock = await Bun.file(path.join(tmpRoot, "omp-plugins.lock.json")).json(); + expect(lock.plugins["git-plugin"]).toEqual({ version: "1.0.0", enabledFeatures: null, enabled: true }); + }); + test("rejects an install whose manifest declares a missing extension entry", async () => { vi.spyOn(Bun, "spawn").mockImplementation(((cmd: string[]) => { expect(cmd).toEqual(["bun", "install", "partial-plugin"]);