fix(coding-agent): refreshed github plugin lockfile pin on re-install
bun install <spec> respects the existing bun.lock pin when the spec is unchanged and never re-resolves the remote ref, so re-running `omp plugin install github:owner/repo` on an already-installed plugin reported success while silently keeping the user on the original resolved commit (1ms no-op, no network). PluginManager.install now follows a git re-install with `bun update <name>` to force re-resolution of the ref against the upstream. First-time installs (no prior dep entry) skip the update — the initial bun install already fetches HEAD. bun update failures trigger the same rollback path as validation failures. Fixes #3063
This commit is contained in:
@@ -2,6 +2,10 @@
|
||||
|
||||
## [Unreleased]
|
||||
|
||||
### Fixed
|
||||
|
||||
- Fixed `omp plugin install github:owner/repo` silently keeping the user on a stale commit when re-run on an already-installed GitHub plugin. `bun install <spec>` respects the existing `bun.lock` pin when the spec is unchanged and never re-resolves the remote ref, so the manager now follows a git re-install with `bun update <name>` to refresh the lockfile pin against the upstream. First-time installs are unaffected. ([#3063](https://github.com/can1357/oh-my-pi/issues/3063))
|
||||
|
||||
## [16.1.3] - 2026-06-19
|
||||
|
||||
### Changed
|
||||
|
||||
@@ -351,19 +351,18 @@ export class PluginManager {
|
||||
const packageSnapshot = await this.#snapshotInstalledPackage(existingActualName);
|
||||
|
||||
try {
|
||||
// Run npm install
|
||||
const proc = Bun.spawn(["bun", "install", packageInstallSpec], {
|
||||
// Step 1: write the spec into plugins/package.json + node_modules.
|
||||
const installProc = Bun.spawn(["bun", "install", packageInstallSpec], {
|
||||
cwd: getPluginsDir(),
|
||||
stdin: "ignore",
|
||||
stdout: "pipe",
|
||||
stderr: "pipe",
|
||||
windowsHide: true,
|
||||
});
|
||||
|
||||
const exitCode = await proc.exited;
|
||||
if (exitCode !== 0) {
|
||||
const stderr = await new Response(proc.stderr).text();
|
||||
throw new Error(`npm install failed: ${stderr}`);
|
||||
const installExit = await installProc.exited;
|
||||
if (installExit !== 0) {
|
||||
const stderr = await new Response(installProc.stderr).text();
|
||||
throw new Error(`bun install failed: ${stderr}`);
|
||||
}
|
||||
// Resolve actual package name. npm specs encode the name (strip version);
|
||||
// git specs do not, so diff plugins/package.json deps to find the new entry.
|
||||
@@ -393,6 +392,31 @@ export class PluginManager {
|
||||
} else {
|
||||
actualName = extractPackageName(spec.packageName);
|
||||
}
|
||||
|
||||
// Step 2: refresh the git lockfile pin when re-installing an existing
|
||||
// git plugin. `bun install <spec>` is a no-op when the spec matches the
|
||||
// lockfile entry — it never re-resolves the remote ref — so re-running
|
||||
// `omp plugin install github:owner/repo` would silently keep the user on
|
||||
// the original resolved commit even after upstream moved (#3063).
|
||||
// `bun update <name>` re-resolves the ref against the remote and
|
||||
// rewrites the pin; SHA-pinned refs stay put because the commit can't
|
||||
// move. First-time installs skip this — the initial `bun install` already
|
||||
// fetched HEAD.
|
||||
if (gitSource && existingActualName) {
|
||||
const updateProc = Bun.spawn(["bun", "update", actualName], {
|
||||
cwd: getPluginsDir(),
|
||||
stdin: "ignore",
|
||||
stdout: "pipe",
|
||||
stderr: "pipe",
|
||||
windowsHide: true,
|
||||
});
|
||||
const updateExit = await updateProc.exited;
|
||||
if (updateExit !== 0) {
|
||||
const stderr = await new Response(updateProc.stderr).text();
|
||||
await this.#rollbackFailedInstall(actualName, packageJsonBefore, packageSnapshot);
|
||||
throw new Error(`bun update ${actualName} failed: ${stderr}`);
|
||||
}
|
||||
}
|
||||
const pkgPath = path.join(getPluginsNodeModules(), actualName, "package.json");
|
||||
|
||||
let pkg: { name: string; version: string; omp?: PluginManifest; pi?: PluginManifest };
|
||||
|
||||
@@ -160,6 +160,117 @@ describe("PluginManager.install with git sources", () => {
|
||||
expect(result.version).toBe("1.0.0");
|
||||
});
|
||||
|
||||
test("re-installing a github plugin runs `bun update` to refresh the stale lockfile pin (#3063)", async () => {
|
||||
// Seed plugins/package.json + node_modules with a previously-installed
|
||||
// github plugin. `findGitPackageName` matches the new install spec to
|
||||
// this existing dep (by repository identity), which is the signal the
|
||||
// manager uses to trigger the lockfile-refresh follow-up.
|
||||
await Bun.write(
|
||||
pluginsPkgJson,
|
||||
JSON.stringify(
|
||||
{
|
||||
name: "omp-plugins",
|
||||
private: true,
|
||||
dependencies: { "stale-plugin": "github:foo/bar" },
|
||||
},
|
||||
null,
|
||||
2,
|
||||
),
|
||||
);
|
||||
const seedDir = path.join(pluginsNodeModules, "stale-plugin");
|
||||
await fs.mkdir(seedDir, { recursive: true });
|
||||
await Bun.write(
|
||||
path.join(seedDir, "package.json"),
|
||||
JSON.stringify({ name: "stale-plugin", version: "0.1.0" }, null, 2),
|
||||
);
|
||||
|
||||
const spawnedCommands: string[][] = [];
|
||||
vi.spyOn(Bun, "spawn").mockImplementation(((cmd: string[]) => {
|
||||
spawnedCommands.push([...cmd]);
|
||||
if (cmd[1] === "install") {
|
||||
// `bun install <same spec>` is a no-op on the lockfile pin —
|
||||
// the manager must NOT rely on this call to refresh the commit.
|
||||
// Leave package.json and node_modules untouched so the test
|
||||
// fails loudly if the manager skips the follow-up `bun update`.
|
||||
return {
|
||||
pid: 1,
|
||||
stdout: emptyStream(),
|
||||
stderr: emptyStream(),
|
||||
exited: Promise.resolve(0),
|
||||
} as Subprocess;
|
||||
}
|
||||
// The follow-up call: simulate bun resolving the upstream HEAD to a
|
||||
// newer commit and bumping the on-disk version. The manager should
|
||||
// read the new version from package.json after this step returns.
|
||||
expect(cmd).toEqual(["bun", "update", "stale-plugin"]);
|
||||
const prepare = (async () => {
|
||||
await Bun.write(
|
||||
path.join(seedDir, "package.json"),
|
||||
JSON.stringify({ name: "stale-plugin", version: "0.1.6" }, null, 2),
|
||||
);
|
||||
})();
|
||||
return {
|
||||
pid: 2,
|
||||
stdout: emptyStream(),
|
||||
stderr: emptyStream(),
|
||||
exited: prepare.then(() => 0),
|
||||
} as Subprocess;
|
||||
}) as typeof Bun.spawn);
|
||||
|
||||
const mgr = new PluginManager(tmpRoot);
|
||||
const result = await mgr.install("github:foo/bar");
|
||||
|
||||
expect(result.version).toBe("0.1.6");
|
||||
expect(spawnedCommands).toEqual([
|
||||
["bun", "install", "github:foo/bar"],
|
||||
["bun", "update", "stale-plugin"],
|
||||
]);
|
||||
});
|
||||
|
||||
test("first-time github install does NOT run `bun update` (no existing pin to refresh)", async () => {
|
||||
await Bun.write(
|
||||
pluginsPkgJson,
|
||||
JSON.stringify({ name: "omp-plugins", private: true, dependencies: {} }, null, 2),
|
||||
);
|
||||
|
||||
const spawnedCommands: string[][] = [];
|
||||
vi.spyOn(Bun, "spawn").mockImplementation(((cmd: string[]) => {
|
||||
spawnedCommands.push([...cmd]);
|
||||
expect(cmd[1]).toBe("install");
|
||||
const prepare = (async () => {
|
||||
await Bun.write(
|
||||
pluginsPkgJson,
|
||||
JSON.stringify(
|
||||
{
|
||||
name: "omp-plugins",
|
||||
private: true,
|
||||
dependencies: { "fresh-plugin": "github:foo/bar" },
|
||||
},
|
||||
null,
|
||||
2,
|
||||
),
|
||||
);
|
||||
const installedDir = path.join(pluginsNodeModules, "fresh-plugin");
|
||||
await fs.mkdir(installedDir, { recursive: true });
|
||||
await Bun.write(
|
||||
path.join(installedDir, "package.json"),
|
||||
JSON.stringify({ name: "fresh-plugin", version: "0.1.6" }, null, 2),
|
||||
);
|
||||
})();
|
||||
return {
|
||||
pid: 1,
|
||||
stdout: emptyStream(),
|
||||
stderr: emptyStream(),
|
||||
exited: prepare.then(() => 0),
|
||||
} as Subprocess;
|
||||
}) as typeof Bun.spawn);
|
||||
|
||||
const mgr = new PluginManager(tmpRoot);
|
||||
await mgr.install("github:foo/bar");
|
||||
|
||||
expect(spawnedCommands).toEqual([["bun", "install", "github:foo/bar"]]);
|
||||
});
|
||||
|
||||
test("rejects git specs containing shell metacharacters", async () => {
|
||||
const mgr = new PluginManager(tmpRoot);
|
||||
await expect(mgr.install("github:foo/bar; rm -rf /")).rejects.toThrow(/Invalid characters in plugin source/);
|
||||
|
||||
@@ -188,30 +188,42 @@ describe("PluginManager.install load validation", () => {
|
||||
});
|
||||
|
||||
vi.spyOn(Bun, "spawn").mockImplementation(((cmd: string[]) => {
|
||||
expect(cmd).toEqual(["bun", "install", "github:org/plugin#v2"]);
|
||||
if (cmd[1] === "install") {
|
||||
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',
|
||||
});
|
||||
})();
|
||||
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;
|
||||
}
|
||||
// The manager follows a git re-install with `bun update <name>` to refresh
|
||||
// the lockfile pin (#3063). The mock treats it as a no-op exit-0 — the
|
||||
// on-disk state already reflects the v2 install above.
|
||||
expect(cmd).toEqual(["bun", "update", "git-plugin"]);
|
||||
return {
|
||||
pid: 1,
|
||||
pid: 2,
|
||||
stdout: emptyStream(),
|
||||
stderr: emptyStream(),
|
||||
exited: prepare.then(() => 0),
|
||||
exited: Promise.resolve(0),
|
||||
} as Subprocess;
|
||||
}) as typeof Bun.spawn);
|
||||
|
||||
|
||||
Reference in New Issue
Block a user