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
This commit is contained in:
roboomp
2026-07-09 18:35:14 +00:00
parent dd67447a03
commit 63adfeece5
4 changed files with 103 additions and 5 deletions
+4
View File
@@ -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
@@ -211,6 +211,16 @@ export class PluginManager {
}
}
async #removeDependencyEntry(pkgJsonPath: string, name: string): Promise<void> {
const pkgJson: { dependencies?: Record<string, string>; [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<string, string>, config: PluginRuntimeConfig): Set<string> {
const installedNames = new Set<string>();
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(),
@@ -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}}
@@ -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,