From 63adfeece558586be519a79a71f9a92013b2a242 Mon Sep 17 00:00:00 2001 From: roboomp Date: Thu, 9 Jul 2026 18:35:14 +0000 Subject: [PATCH] fix(plugin): handled pinned git source replacements - Removed the stale pinned dependency edge before invoking Bun for same-repository git source replacements so Bun does not construct a dependency loop. - Added a regression test for the pinned-to-unpinned GitHub plugin replacement path. Fixes #4960 --- packages/coding-agent/CHANGELOG.md | 4 + .../src/extensibility/plugins/manager.ts | 21 ++++++ .../src/prompts/system/workflow-notice.md | 10 +-- .../test/plugin-install-git.test.ts | 73 +++++++++++++++++++ 4 files changed, 103 insertions(+), 5 deletions(-) diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index d1ad895e4..0ebe4a0c5 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -2,6 +2,10 @@ ## [Unreleased] +### Fixed + +- Fixed `omp plugin install github:owner/repo --force` failing with Bun `DependencyLoop` when replacing an existing pinned git plugin source for the same repository; the installer now removes the stale pinned dependency edge before invoking Bun and restores it on rollback. ([#4960](https://github.com/can1357/oh-my-pi/issues/4960)) + ## [16.3.13] - 2026-07-09 ### Fixed diff --git a/packages/coding-agent/src/extensibility/plugins/manager.ts b/packages/coding-agent/src/extensibility/plugins/manager.ts index f1e7801c3..c6ddc30bc 100644 --- a/packages/coding-agent/src/extensibility/plugins/manager.ts +++ b/packages/coding-agent/src/extensibility/plugins/manager.ts @@ -211,6 +211,16 @@ export class PluginManager { } } + async #removeDependencyEntry(pkgJsonPath: string, name: string): Promise { + const pkgJson: { dependencies?: Record; [key: string]: unknown } = + await Bun.file(pkgJsonPath).json(); + if (!pkgJson.dependencies || !(name in pkgJson.dependencies)) { + return; + } + delete pkgJson.dependencies[name]; + await Bun.write(pkgJsonPath, JSON.stringify(pkgJson, null, 2)); + } + #collectInstalledNames(deps: Record, config: PluginRuntimeConfig): Set { const installedNames = new Set(); for (const name of Object.keys(deps)) { @@ -450,6 +460,17 @@ export class PluginManager { // validation throws. let actualName: string | undefined; try { + // Bun treats a dependency replacement from `repo#old-ref` to the same + // package at `repo`/`repo#new-ref` as a self-edge and bails with + // DependencyLoop. Remove only the stale manifest edge; rollback restores + // the original package.json and node_modules snapshot on failure. + if (gitSource && existingActualName) { + const installedSource = parseGitUrl(depsBefore[existingActualName] ?? ""); + if (installedSource && installedSource.ref !== gitSource.ref) { + await this.#removeDependencyEntry(pkgJsonPath, existingActualName); + } + } + // Step 1: write the spec into plugins/package.json + node_modules. const installProc = Bun.spawn(["bun", "install", packageInstallSpec], { cwd: getPluginsDir(), diff --git a/packages/coding-agent/src/prompts/system/workflow-notice.md b/packages/coding-agent/src/prompts/system/workflow-notice.md index 74eddee21..3e62ca5b0 100644 --- a/packages/coding-agent/src/prompts/system/workflow-notice.md +++ b/packages/coding-agent/src/prompts/system/workflow-notice.md @@ -51,20 +51,20 @@ Decompose first, then {{#if taskBatch}}batch the independent leaves{{else}}issue {{#if taskBatch}} task( - context: "# Goal\nReview the auth diff...\n# Constraints\nRead-only...\n# Contract\nReturn findings as severity/file/line/fix...", + context: "# Goal\nReview the auth diff…\n# Constraints\nRead-only…\n# Contract\nReturn findings as severity/file/line/fix…", tasks: [ - { id: "AuthOwner", role: "Auth Storage Reviewer", assignment: "# Target\npackages/ai/src/auth-storage.ts\n# Change\nTrace credential selection...\n# Acceptance\nReturn confirmed findings only..." }, - { id: "PromptOwner", role: "Prompt Contract Reviewer", assignment: "# Target\npackages/coding-agent/src/prompts/**\n# Change\nCheck active-tool guidance...\n# Acceptance\nReturn mismatches and exact prompt lines..." }, + { id: "AuthOwner", role: "Auth Storage Reviewer", assignment: "# Target\npackages/ai/src/auth-storage.ts\n# Change\nTrace credential selection…\n# Acceptance\nReturn confirmed findings only…" }, + { id: "PromptOwner", role: "Prompt Contract Reviewer", assignment: "# Target\npackages/coding-agent/src/prompts/**\n# Change\nCheck active-tool guidance…\n# Acceptance\nReturn mismatches and exact prompt lines…" }, ] ) {{else}} task( role: "Auth Storage Reviewer", - assignment: "# Target\npackages/ai/src/auth-storage.ts\n# Change\nReview the auth diff. Shared contract: read-only; return findings as severity/file/line/fix.\n# Acceptance\nReturn confirmed findings only..." + assignment: "# Target\npackages/ai/src/auth-storage.ts\n# Change\nReview the auth diff. Shared contract: read-only; return findings as severity/file/line/fix.\n# Acceptance\nReturn confirmed findings only…" ) task( role: "Prompt Contract Reviewer", - assignment: "# Target\npackages/coding-agent/src/prompts/**\n# Change\nCheck active-tool guidance. Shared contract: read-only; return mismatches and exact prompt lines.\n# Acceptance\nReturn confirmed findings only..." + assignment: "# Target\npackages/coding-agent/src/prompts/**\n# Change\nCheck active-tool guidance. Shared contract: read-only; return mismatches and exact prompt lines.\n# Acceptance\nReturn confirmed findings only…" ) {{/if}} diff --git a/packages/coding-agent/test/plugin-install-git.test.ts b/packages/coding-agent/test/plugin-install-git.test.ts index 347c23b3e..8efcddb57 100644 --- a/packages/coding-agent/test/plugin-install-git.test.ts +++ b/packages/coding-agent/test/plugin-install-git.test.ts @@ -228,6 +228,79 @@ describe("PluginManager.install with git sources", () => { ]); }); + test("removes an existing pinned git dependency before installing the unpinned source (#4960)", async () => { + await Bun.write( + pluginsPkgJson, + JSON.stringify( + { + name: "omp-plugins", + private: true, + dependencies: { "replaced-plugin": "github:foo/bar#v1.0.0" }, + }, + null, + 2, + ), + ); + const seedDir = path.join(pluginsNodeModules, "replaced-plugin"); + await fs.mkdir(seedDir, { recursive: true }); + await Bun.write( + path.join(seedDir, "package.json"), + JSON.stringify({ name: "replaced-plugin", version: "1.0.0" }, null, 2), + ); + + const spawnedCommands: string[][] = []; + vi.spyOn(Bun, "spawn").mockImplementation(((cmd: string[]) => { + spawnedCommands.push([...cmd]); + if (cmd[1] === "install") { + expect(cmd).toEqual(["bun", "install", "github:foo/bar"]); + const prepare = (async () => { + const packageJson = await Bun.file(pluginsPkgJson).json(); + expect(packageJson.dependencies?.["replaced-plugin"]).toBeUndefined(); + + await Bun.write( + pluginsPkgJson, + JSON.stringify( + { + name: "omp-plugins", + private: true, + dependencies: { "replaced-plugin": "github:foo/bar" }, + }, + null, + 2, + ), + ); + await Bun.write( + path.join(seedDir, "package.json"), + JSON.stringify({ name: "replaced-plugin", version: "1.1.0" }, null, 2), + ); + })(); + return { + pid: 1, + stdout: emptyStream(), + stderr: emptyStream(), + exited: prepare.then(() => 0), + } as Subprocess; + } + + expect(cmd).toEqual(["bun", "update", "replaced-plugin"]); + return { + pid: 2, + stdout: emptyStream(), + stderr: emptyStream(), + exited: Promise.resolve(0), + } as Subprocess; + }) as typeof Bun.spawn); + + const mgr = new PluginManager(tmpRoot); + const result = await mgr.install("github:foo/bar"); + + expect(result.name).toBe("replaced-plugin"); + expect(result.version).toBe("1.1.0"); + expect(spawnedCommands[0]).toEqual(["bun", "install", "github:foo/bar"]); + const packageJson = await Bun.file(pluginsPkgJson).json(); + expect(packageJson.dependencies).toEqual({ "replaced-plugin": "github:foo/bar" }); + }); + test("first-time github install does NOT run `bun update` (no existing pin to refresh)", async () => { await Bun.write( pluginsPkgJson,