fix(coding-agent): normalize hosted plugin git shorthands
Addresses review feedback on #1527.
This commit is contained in:
@@ -26,9 +26,9 @@ const KNOWN_HOSTS: Record<string, (pathname: string, hash: string) => { user: st
|
||||
};
|
||||
|
||||
/**
|
||||
* Namespaced shorthand prefixes recognised by bun's `bun install <spec>`, mapped
|
||||
* to their canonical host. Mirrors npm's `github:`, `gitlab:`, `bitbucket:` etc.
|
||||
* shorthand so callers can pass them through to bun verbatim.
|
||||
* Namespaced shorthand prefixes accepted by `omp plugin install`, mapped to
|
||||
* their canonical host. `PluginManager.install` normalizes non-GitHub prefixes
|
||||
* before invoking bun because bun only treats `github:` as a hosted shorthand.
|
||||
*/
|
||||
const SHORTHAND_PREFIXES: Record<string, string> = {
|
||||
github: "github.com",
|
||||
@@ -274,8 +274,8 @@ function tryNamespacedShorthand(trimmed: string): GitSource | null {
|
||||
*
|
||||
* Rules:
|
||||
* - Namespaced shorthand (`github:user/repo`, `gitlab:`, `bitbucket:`,
|
||||
* `codeberg:`, `sourcehut:`/`srht:`) is accepted directly — these mirror
|
||||
* bun/npm's built-in install shorthands and skip the rest of the pipeline.
|
||||
* `codeberg:`, `sourcehut:`/`srht:`) is accepted directly; installers should
|
||||
* normalize entries that bun does not understand natively.
|
||||
* - With `git:` prefix, accept generic shorthand forms.
|
||||
* - Without `git:` prefix, only accept explicit protocol URLs.
|
||||
*
|
||||
|
||||
@@ -10,7 +10,7 @@ import {
|
||||
isEnoent,
|
||||
logger,
|
||||
} from "@oh-my-pi/pi-utils";
|
||||
import { isGitSpec } from "./git-url";
|
||||
import { parseGitUrl, type GitSource } from "./git-url";
|
||||
import { extractPackageName, parsePluginSpec } from "./parser";
|
||||
import type {
|
||||
DoctorCheck,
|
||||
@@ -64,6 +64,16 @@ function validateGitSpec(spec: string): void {
|
||||
}
|
||||
}
|
||||
|
||||
function gitInstallSpec(original: string, source: GitSource): string {
|
||||
if (/^github:/i.test(original) || !/^[a-z]+:[^/]/i.test(original)) {
|
||||
return original;
|
||||
}
|
||||
if (!source.ref || source.repo.includes("#")) {
|
||||
return source.repo;
|
||||
}
|
||||
return `${source.repo}#${source.ref}`;
|
||||
}
|
||||
|
||||
// =============================================================================
|
||||
// Plugin Manager
|
||||
// =============================================================================
|
||||
@@ -187,7 +197,7 @@ export class PluginManager {
|
||||
*/
|
||||
async install(specString: string, options: InstallOptions = {}): Promise<InstalledPlugin> {
|
||||
const spec = parsePluginSpec(specString);
|
||||
const gitSource = isGitSpec(spec.packageName);
|
||||
const gitSource = parseGitUrl(spec.packageName);
|
||||
if (gitSource) {
|
||||
validateGitSpec(spec.packageName);
|
||||
} else {
|
||||
@@ -208,9 +218,10 @@ export class PluginManager {
|
||||
}
|
||||
const pkgJsonPath = getPluginsPackageJson();
|
||||
const depsBefore = gitSource ? await this.#readDeps(pkgJsonPath) : {};
|
||||
const packageInstallSpec = gitSource ? gitInstallSpec(spec.packageName, gitSource) : spec.packageName;
|
||||
|
||||
// Run npm install
|
||||
const proc = Bun.spawn(["bun", "install", spec.packageName], {
|
||||
const proc = Bun.spawn(["bun", "install", packageInstallSpec], {
|
||||
cwd: getPluginsDir(),
|
||||
stdin: "ignore",
|
||||
stdout: "pipe",
|
||||
@@ -237,9 +248,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 value containing the original spec instead.
|
||||
// Match by the install value for force-reinstalls where no new key is
|
||||
// added (non-GitHub shorthands are normalized before bun sees them).
|
||||
if (!resolved) {
|
||||
const needle = spec.packageName.replace(/^git\+/, "");
|
||||
const needle = packageInstallSpec.replace(/^git\+/i, "");
|
||||
for (const [key, value] of Object.entries(depsAfter)) {
|
||||
if (typeof value === "string" && value.includes(needle)) {
|
||||
resolved = key;
|
||||
|
||||
@@ -111,6 +111,55 @@ describe("PluginManager.install with git sources", () => {
|
||||
expect(result.path).toBe(path.join(pluginsNodeModules, "real-name"));
|
||||
});
|
||||
|
||||
test("normalizes non-GitHub shorthand before invoking bun install", async () => {
|
||||
await Bun.write(
|
||||
pluginsPkgJson,
|
||||
JSON.stringify({ name: "omp-plugins", private: true, dependencies: {} }, null, 2),
|
||||
);
|
||||
|
||||
vi.spyOn(Bun, "spawn").mockImplementation(((cmd: string[]) => {
|
||||
expect(cmd[0]).toBe("bun");
|
||||
expect(cmd[1]).toBe("install");
|
||||
expect(cmd[2]).toBe("https://gitlab.com/group/sub/project#v1.0.0");
|
||||
|
||||
const prepare = (async () => {
|
||||
await Bun.write(
|
||||
pluginsPkgJson,
|
||||
JSON.stringify(
|
||||
{
|
||||
name: "omp-plugins",
|
||||
private: true,
|
||||
dependencies: {
|
||||
"gitlab-plugin": "git+https://gitlab.com/group/sub/project.git#v1.0.0",
|
||||
},
|
||||
},
|
||||
null,
|
||||
2,
|
||||
),
|
||||
);
|
||||
const installedDir = path.join(pluginsNodeModules, "gitlab-plugin");
|
||||
await fs.mkdir(installedDir, { recursive: true });
|
||||
await Bun.write(
|
||||
path.join(installedDir, "package.json"),
|
||||
JSON.stringify({ name: "gitlab-plugin", version: "1.0.0" }, null, 2),
|
||||
);
|
||||
})();
|
||||
|
||||
return {
|
||||
pid: 1,
|
||||
stdout: emptyStream(),
|
||||
stderr: emptyStream(),
|
||||
exited: prepare.then(() => 0),
|
||||
} as Subprocess;
|
||||
}) as typeof Bun.spawn);
|
||||
|
||||
const mgr = new PluginManager(tmpRoot);
|
||||
const result = await mgr.install("gitlab:group/sub/project#v1.0.0");
|
||||
|
||||
expect(result.name).toBe("gitlab-plugin");
|
||||
expect(result.version).toBe("1.0.0");
|
||||
});
|
||||
|
||||
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/);
|
||||
|
||||
Reference in New Issue
Block a user