diff --git a/packages/coding-agent/src/cli/plugin-cli.ts b/packages/coding-agent/src/cli/plugin-cli.ts index 6f555224c0..5564c5edfd 100644 --- a/packages/coding-agent/src/cli/plugin-cli.ts +++ b/packages/coding-agent/src/cli/plugin-cli.ts @@ -19,6 +19,7 @@ import { isGjcPluginSourceShape, listGjcBundles, previewGjcBundleUpdate, + uninstallGjcBundle, } from "../extensibility/gjc-plugins"; import { PluginManager, parseSettingValue, validateSetting } from "../extensibility/plugins"; import { @@ -480,6 +481,31 @@ function describeInstallFailure(error: unknown): string { return error instanceof GjcPluginLoadError ? error.code : "install_failed"; } +function isGjcRegistryShapeFailure(error: unknown): boolean { + return ( + (error instanceof GjcPluginLoadError && error.code === "invalid_manifest") || + (error instanceof TypeError && + /(?:not iterable|localeCompare|reading ['"](?:scope|name|pluginRoot|plugins|map))/.test(error.message)) + ); +} + +async function findGjcBundlesForUninstall( + cwd: string, + name: string, + scope: "user" | "project" | undefined, +): Promise { + const scopes = scope ? [scope] : (["user", "project"] as const); + const matches: GjcBundleSummary[] = []; + for (const candidateScope of scopes) { + try { + const result = await getGjcBundle({ cwd }, bundleIdentity(candidateScope, name)); + if (result.ok) matches.push(result.value); + } catch (error) { + if (!isGjcRegistryShapeFailure(error)) throw error; + } + } + return matches; +} async function handleInstall( manager: PluginManager, packages: string[], @@ -627,21 +653,41 @@ async function handleInstall( async function handleUninstall( manager: PluginManager, packages: string[], - flags: { json?: boolean; scope?: "user" | "project" }, + flags: { json?: boolean; scope?: "user" | "project"; user?: boolean; project?: boolean }, ): Promise { if (packages.length === 0) { console.error(chalk.red(`Usage: ${APP_NAME} plugin uninstall ...`)); process.exit(1); } - // For uninstall, check the installed plugins registry directly. - // This works even if the marketplace entry was later removed from marketplaces.json. + const scope = flags.scope ?? (flags.user ? "user" : flags.project ? "project" : undefined); + const cwd = getProjectDir(); const mktMgr = await makeMarketplaceManager(); const installedPlugins = new Set((await mktMgr.listInstalledPlugins()).map(p => p.id)); for (const name of packages) { + const matches = await findGjcBundlesForUninstall(cwd, name, scope); + if (matches.length > 0) { + if (matches.length > 1) { + console.error(chalk.red(`GJC bundle "${name}" is installed in both scopes; specify --user or --project.`)); + process.exit(1); + } + const identity = matches[0].identity; + const result = await uninstallGjcBundle({ cwd }, identity); + if (!result.ok) { + console.error(chalk.red(`${theme.status.error} ${result.error.message}`)); + if (result.error.recovery) console.error(chalk.dim(` Try: ${result.error.recovery}`)); + process.exit(3); + } + if (flags.json) { + console.log(JSON.stringify({ uninstalled: identity })); + } else { + console.log(chalk.green(`${theme.status.success} Uninstalled ${identity.name} (${identity.scope})`)); + } + continue; + } + if (installedPlugins.has(name)) { - // Exact match against installed marketplace plugin IDs (name@marketplace) try { await mktMgr.uninstallPlugin(name, flags.scope); console.log(chalk.green(`${theme.status.success} Uninstalled ${name}`)); @@ -652,7 +698,6 @@ async function handleUninstall( continue; } - // npm path try { await manager.uninstall(name); if (flags.json) { diff --git a/packages/coding-agent/src/extensibility/gjc-plugins/lifecycle.ts b/packages/coding-agent/src/extensibility/gjc-plugins/lifecycle.ts index a1b70d2dcf..157eb6ff31 100644 --- a/packages/coding-agent/src/extensibility/gjc-plugins/lifecycle.ts +++ b/packages/coding-agent/src/extensibility/gjc-plugins/lifecycle.ts @@ -14,7 +14,13 @@ import { surfaceIdsOf, targetFingerprint, } from "./lifecycle-reconciliation"; -import { readRegistry, sortRegistryEntries, withRegistryLock, writeRegistryUnlocked } from "./registry"; +import { + readRegistry, + registryRootForScope, + sortRegistryEntries, + withRegistryLock, + writeRegistryUnlocked, +} from "./registry"; import type { GjcBundleIdentity, GjcBundleSafeSource, @@ -49,6 +55,9 @@ export interface GjcLifecycleContext { function fail(code: GjcLifecycleError["code"], message: string, recovery?: string): GjcLifecycleError { return recovery ? { code, message, recovery } : { code, message }; } +function isEnoent(error: unknown): boolean { + return (error as NodeJS.ErrnoException)?.code === "ENOENT"; +} const UNSUPPORTED_UPDATE_REASON: Partial> = {}; @@ -224,6 +233,171 @@ export async function getGjcBundle( return { ok: true, value: toBundleSummary(entry) }; } +function safeInstalledRoot(scope: GjcPluginScope, cwd: string, pluginRoot: string): string | null { + const root = path.resolve(pluginRoot); + const scopeRoot = path.resolve(registryRootForScope(scope, cwd)); + const relative = path.relative(scopeRoot, root); + if (!relative || relative.startsWith(`..${path.sep}`) || relative === ".." || path.isAbsolute(relative)) return null; + return root; +} +function isRecord(value: unknown): value is Record { + return typeof value === "object" && value !== null; +} + +function isStringArray(value: unknown): value is string[] { + return Array.isArray(value) && value.every(item => typeof item === "string"); +} + +function isUninstallableEntry(value: unknown, identity: GjcBundleIdentity): value is GjcPluginRegistryEntry { + if (!isRecord(value)) return false; + if ( + value.name !== identity.name || + value.scope !== identity.scope || + typeof value.version !== "string" || + typeof value.enabled !== "boolean" || + typeof value.pluginRoot !== "string" || + typeof value.manifestPath !== "string" || + typeof value.manifestHash !== "string" || + typeof value.installedAt !== "string" || + typeof value.updatedAt !== "string" || + !isStringArray(value.disabledSurfaceIds) || + !Array.isArray(value.copiedFiles) + ) { + return false; + } + const source = value.source; + if ( + !isRecord(source) || + typeof source.kind !== "string" || + typeof source.uri !== "string" || + typeof source.resolvedAt !== "string" + ) { + return false; + } + const surfaces = value.surfaces; + if (!isRecord(surfaces)) return false; + for (const key of ["subskills", "tools", "hooks", "mcps", "systemAppendices", "agentAppendices"]) { + const list = surfaces[key]; + if ( + !Array.isArray(list) || + !list.every(item => isRecord(item) && typeof item.extensionId === "string" && typeof item.name === "string") + ) { + return false; + } + } + if ( + !value.copiedFiles.every( + file => + isRecord(file) && + typeof file.relativePath === "string" && + typeof file.sha256 === "string" && + typeof file.bytes === "number", + ) + ) { + return false; + } + if (value.quarantine !== undefined) { + if ( + !Array.isArray(value.quarantine) || + !value.quarantine.every( + entry => isRecord(entry) && typeof entry.surfaceId === "string" && typeof entry.code === "string", + ) + ) { + return false; + } + } + return true; +} + +function isMalformedRegistryError(error: unknown): boolean { + return ( + (error instanceof GjcPluginLoadError && error.code === "invalid_manifest") || + (error instanceof TypeError && + /(?:not iterable|localeCompare|reading ['"](?:scope|name|pluginRoot|plugins|map))/.test(error.message)) + ); +} + +function uninstallFailure( + identity: GjcBundleIdentity, + kind: "metadata" | "remove" | "write" | "restore", +): GjcLifecycleError { + const detail = + kind === "metadata" + ? "its installed metadata is invalid" + : kind === "remove" + ? "the installed files could not be moved safely" + : kind === "write" + ? "its registry could not be updated" + : "the previous state could not be restored"; + const recovery = + kind === "metadata" + ? `Repair the GJC ${identity.scope} registry, then retry gjc plugin uninstall ${identity.name} --${identity.scope}` + : `Check GJC plugin directory permissions, then retry gjc plugin uninstall ${identity.name} --${identity.scope}`; + return fail("invalid_target", `Could not uninstall GJC bundle "${identity.name}" because ${detail}`, recovery); +} + +export async function uninstallGjcBundle( + ctx: GjcLifecycleContext, + identity: GjcBundleIdentity, +): Promise> { + return withRegistryLock(identity.scope, ctx.cwd, async () => { + let registry: Awaited>; + try { + registry = await readRegistry(identity.scope, ctx.cwd); + } catch (error) { + if (isMalformedRegistryError(error)) return { ok: false, error: uninstallFailure(identity, "metadata") }; + throw error; + } + + const entry = registry.plugins.find(plugin => plugin && plugin.name === identity.name); + if (!entry) return { ok: false, error: notInstalled(identity) }; + if (!isUninstallableEntry(entry, identity)) return { ok: false, error: uninstallFailure(identity, "metadata") }; + + const root = safeInstalledRoot(identity.scope, ctx.cwd, entry.pluginRoot); + if (!root) return { ok: false, error: uninstallFailure(identity, "metadata") }; + + const summary = toBundleSummary(entry); + const nextRegistry = { ...registry, plugins: registry.plugins.filter(plugin => plugin !== entry) }; + const backupRoot = `${root}.uninstalling-${process.pid}-${Date.now()}`; + let moved = false; + + try { + await fs.rename(root, backupRoot); + moved = true; + } catch (error) { + if (!isEnoent(error)) return { ok: false, error: uninstallFailure(identity, "remove") }; + } + + try { + await writeRegistryUnlocked(nextRegistry, ctx.cwd); + } catch { + if (moved) { + try { + await fs.rename(backupRoot, root); + } catch { + return { ok: false, error: uninstallFailure(identity, "restore") }; + } + } + return { ok: false, error: uninstallFailure(identity, "write") }; + } + + if (moved) { + try { + await fs.rm(backupRoot, { recursive: true, force: true }); + } catch { + try { + await writeRegistryUnlocked(registry, ctx.cwd); + await fs.rename(backupRoot, root); + } catch { + return { ok: false, error: uninstallFailure(identity, "restore") }; + } + return { ok: false, error: uninstallFailure(identity, "remove") }; + } + } + return { ok: true, value: { identity, summary } }; + }); +} + function notInstalled(identity: GjcBundleIdentity): GjcLifecycleError { return fail( "not_installed", diff --git a/packages/coding-agent/test/gjc-plugin-lifecycle.test.ts b/packages/coding-agent/test/gjc-plugin-lifecycle.test.ts index 0bd7700c29..983c4a33de 100644 --- a/packages/coding-agent/test/gjc-plugin-lifecycle.test.ts +++ b/packages/coding-agent/test/gjc-plugin-lifecycle.test.ts @@ -1,4 +1,4 @@ -import { afterEach, beforeEach, describe, expect, test } from "bun:test"; +import { afterEach, beforeEach, describe, expect, spyOn, test } from "bun:test"; import * as fs from "node:fs/promises"; import * as os from "node:os"; import * as path from "node:path"; @@ -13,8 +13,10 @@ import { previewGjcBundleUpdate, readRegistry, redactSourceLocator, + registryPathForScope, setGjcBundleEnabled, setGjcBundleSurfaceEnabled, + uninstallGjcBundle, } from "../src/extensibility/gjc-plugins"; const fixturesRoot = path.join(import.meta.dir, "fixtures", "gjc-plugins"); @@ -104,6 +106,112 @@ describe("GJC bundle lifecycle", () => { expect(disabled).toMatchObject({ ok: true, value: { mutated: true, summary: { enabled: false } } }); expect(await summary(cwd, user)).toEqual(userBefore); }); + test("uninstalls a user bundle and removes its installed root", async () => { + const cwd = await mkProjectCwd(); + const identity = await installFixture(cwd, "user"); + const registryBefore = await readRegistry("user", cwd); + const entry = registryBefore.plugins.find(plugin => plugin.name === identity.name); + expect(entry).toBeDefined(); + if (!entry) throw new Error("missing installed entry"); + expect(await fs.stat(entry.pluginRoot)).toBeTruthy(); + + const result = await uninstallGjcBundle({ cwd }, identity); + expect(result).toMatchObject({ ok: true, value: { identity } }); + expect((await readRegistry("user", cwd)).plugins).toHaveLength(0); + await expect(fs.stat(entry.pluginRoot)).rejects.toMatchObject({ code: "ENOENT" }); + }); + test("returns a typed error for a malformed registry entry without removing its root", async () => { + const cwd = await mkProjectCwd(); + const identity = await installFixture(cwd, "user"); + const registryPath = registryPathForScope("user", cwd); + const raw = JSON.parse(await fs.readFile(registryPath, "utf8")) as { + plugins: Array>; + }; + const entry = raw.plugins[0]; + expect(entry).toBeDefined(); + if (!entry) throw new Error("missing installed entry"); + const installedRoot = entry.pluginRoot; + expect(typeof installedRoot).toBe("string"); + if (typeof installedRoot !== "string") throw new Error("missing installed root"); + const surfaces = entry.surfaces as Record; + surfaces.tools = null; + await fs.writeFile(registryPath, JSON.stringify(raw)); + + const result = await uninstallGjcBundle({ cwd }, identity); + + expect(result).toMatchObject({ + ok: false, + error: { code: "invalid_target", recovery: expect.stringContaining("retry") }, + }); + await expect(fs.stat(installedRoot)).resolves.toBeTruthy(); + }); + + test("restores the root and returns a typed error when registry removal fails", async () => { + const cwd = await mkProjectCwd(); + const identity = await installFixture(cwd, "user"); + const registryBefore = await readRegistry("user", cwd); + const entry = registryBefore.plugins.find(plugin => plugin.name === identity.name); + expect(entry).toBeDefined(); + if (!entry) throw new Error("missing installed entry"); + + const realRename = fs.rename; + const renameSpy = spyOn(fs, "rename"); + renameSpy.mockImplementationOnce(realRename); + renameSpy.mockRejectedValueOnce(new Error("registry rename failed")); + renameSpy.mockImplementation(realRename); + + const result = await uninstallGjcBundle({ cwd }, identity); + + renameSpy.mockRestore(); + expect(result).toMatchObject({ + ok: false, + error: { code: "invalid_target", recovery: expect.stringContaining("retry") }, + }); + expect((await readRegistry("user", cwd)).plugins).toHaveLength(1); + await expect(fs.stat(entry.pluginRoot)).resolves.toBeTruthy(); + }); + + // A registry whose `pluginRoot` points outside the scope root is the shape a + // tampered or hand-edited registry takes; uninstall must refuse it instead of + // deleting whatever the path names. + test("refuses to remove a registry root that escapes the scope directory", async () => { + const cwd = await mkProjectCwd(); + const identity = await installFixture(cwd, "user"); + const outside = await mkProjectCwd(); + const sentinel = path.join(outside, "keep-me.txt"); + await fs.writeFile(sentinel, "not yours to delete"); + + const registryPath = registryPathForScope("user", cwd); + const raw = JSON.parse(await fs.readFile(registryPath, "utf8")) as { + plugins: Array>; + }; + const entry = raw.plugins[0]; + expect(entry).toBeDefined(); + if (!entry) throw new Error("missing installed entry"); + entry.pluginRoot = outside; + await fs.writeFile(registryPath, JSON.stringify(raw)); + + const result = await uninstallGjcBundle({ cwd }, identity); + + expect(result).toMatchObject({ ok: false, error: { code: "invalid_target" } }); + await expect(fs.readFile(sentinel, "utf8")).resolves.toBe("not yours to delete"); + expect((await readRegistry("user", cwd)).plugins).toHaveLength(1); + }); + + test("installs the same bundle again after an uninstall", async () => { + const cwd = await mkProjectCwd(); + const identity = await installFixture(cwd, "user"); + expect(await uninstallGjcBundle({ cwd }, identity)).toMatchObject({ ok: true }); + + const reinstalled = await installGjcBundle({ cwd }, "user", sixSurface); + + expect(reinstalled.ok).toBe(true); + if (!reinstalled.ok) throw new Error(reinstalled.error.code); + expect(reinstalled.value.summary.identity).toEqual(identity); + const registry = await readRegistry("user", cwd); + expect(registry.plugins).toHaveLength(1); + await expect(fs.stat(registry.plugins[0].pluginRoot)).resolves.toBeTruthy(); + }); test("previews unchanged source with an identity-bound unchanged token", async () => { const cwd = await mkProjectCwd(); diff --git a/packages/coding-agent/test/plugin-command.test.ts b/packages/coding-agent/test/plugin-command.test.ts index e050903a06..2cf686cdb2 100644 --- a/packages/coding-agent/test/plugin-command.test.ts +++ b/packages/coding-agent/test/plugin-command.test.ts @@ -18,11 +18,12 @@ const agentDirs: string[] = []; async function runPluginCommand( args: string[], cwd: string, + agentDirOverride?: string, ): Promise<{ exitCode: number; stdout: string; stderr: string }> { // Isolate the user scope: without this the child process reads the real // ~/.gjc/agent registry and inherits whatever the developer has installed. - const agentDir = await fs.mkdtemp(path.join(os.tmpdir(), "gjc-plugin-command-agent-")); - agentDirs.push(agentDir); + const agentDir = agentDirOverride ?? (await fs.mkdtemp(path.join(os.tmpdir(), "gjc-plugin-command-agent-"))); + if (!agentDirOverride) agentDirs.push(agentDir); const proc = Bun.spawn({ cmd: [process.execPath, path.join(import.meta.dir, "../src/cli.ts"), "plugin", ...args], cwd, @@ -97,6 +98,109 @@ describe("Plugin command scope parsing", () => { expect(gjcJson).not.toContain(os.homedir()); expect(gjcJson).not.toMatch(/"uri"\s*:/); }); + it("uninstalls a user-scoped GJC bundle instead of invoking npm", async () => { + const agentDir = await fs.mkdtemp(path.join(os.tmpdir(), "gjc-plugin-command-agent-")); + agentDirs.push(agentDir); + const cwd = await makeTempProject(); + const fixture = path.join(import.meta.dir, "fixtures/gjc-plugins/valid-six-surface-bundle"); + + const install = await runPluginCommand(["install", fixture, "--user"], cwd, agentDir); + expect(install.exitCode).toBe(0); + + const uninstall = await runPluginCommand(["uninstall", "valid-six-surface-bundle", "--user"], cwd, agentDir); + expect(uninstall.exitCode).toBe(0); + expect(uninstall.stderr).toBe(""); + expect(uninstall.stdout).toContain("Uninstalled valid-six-surface-bundle (user)"); + + const listed = await runPluginCommand(["list", "--json"], cwd, agentDir); + expect(listed.exitCode).toBe(0); + expect(JSON.parse(listed.stdout)).toMatchObject({ gjc: [] }); + }); + + // An unqualified uninstall of a name present in both scopes must refuse + // rather than guess, and must not remove either copy. + it("refuses an ambiguous uninstall when the bundle is installed in both scopes", async () => { + const agentDir = await fs.mkdtemp(path.join(os.tmpdir(), "gjc-plugin-command-agent-")); + agentDirs.push(agentDir); + const cwd = await makeTempProject(); + const fixture = path.join(import.meta.dir, "fixtures/gjc-plugins/valid-six-surface-bundle"); + + expect((await runPluginCommand(["install", fixture, "--user"], cwd, agentDir)).exitCode).toBe(0); + expect((await runPluginCommand(["install", fixture, "--project"], cwd, agentDir)).exitCode).toBe(0); + + const ambiguous = await runPluginCommand(["uninstall", "valid-six-surface-bundle"], cwd, agentDir); + expect(ambiguous.exitCode).toBe(1); + expect(ambiguous.stderr).toContain("installed in both scopes"); + + const listed = await runPluginCommand(["list", "--json"], cwd, agentDir); + const scopes = (JSON.parse(listed.stdout) as { gjc: Array<{ identity: { scope: string } }> }).gjc.map( + bundle => bundle.identity.scope, + ); + expect(scopes.toSorted()).toEqual(["project", "user"]); + }); + + it("scopes an explicit --project uninstall to the project copy", async () => { + const agentDir = await fs.mkdtemp(path.join(os.tmpdir(), "gjc-plugin-command-agent-")); + agentDirs.push(agentDir); + const cwd = await makeTempProject(); + const fixture = path.join(import.meta.dir, "fixtures/gjc-plugins/valid-six-surface-bundle"); + + expect((await runPluginCommand(["install", fixture, "--user"], cwd, agentDir)).exitCode).toBe(0); + expect((await runPluginCommand(["install", fixture, "--project"], cwd, agentDir)).exitCode).toBe(0); + + const uninstall = await runPluginCommand(["uninstall", "valid-six-surface-bundle", "--project"], cwd, agentDir); + expect(uninstall.exitCode).toBe(0); + expect(uninstall.stdout).toContain("Uninstalled valid-six-surface-bundle (project)"); + + const listed = await runPluginCommand(["list", "--json"], cwd, agentDir); + expect( + (JSON.parse(listed.stdout) as { gjc: Array<{ identity: { scope: string } }> }).gjc.map( + bundle => bundle.identity.scope, + ), + ).toEqual(["user"]); + }); + + // The recovery path the whole uninstall command exists for: a user who + // uninstalled must be able to install the same bundle again without hitting + // `already_installed_use_upgrade` residue. + it("reinstalls the same bundle cleanly after an uninstall", async () => { + const agentDir = await fs.mkdtemp(path.join(os.tmpdir(), "gjc-plugin-command-agent-")); + agentDirs.push(agentDir); + const cwd = await makeTempProject(); + const fixture = path.join(import.meta.dir, "fixtures/gjc-plugins/valid-six-surface-bundle"); + + expect((await runPluginCommand(["install", fixture, "--user"], cwd, agentDir)).exitCode).toBe(0); + expect( + (await runPluginCommand(["uninstall", "valid-six-surface-bundle", "--user"], cwd, agentDir)).exitCode, + ).toBe(0); + + const reinstall = await runPluginCommand(["install", fixture, "--user"], cwd, agentDir); + expect(reinstall.exitCode).toBe(0); + expect(reinstall.stderr).toBe(""); + expect(`${reinstall.stdout}${reinstall.stderr}`).not.toContain("already_installed"); + + const listed = await runPluginCommand(["list", "--json"], cwd, agentDir); + expect(JSON.parse(listed.stdout)).toMatchObject({ + gjc: [ + expect.objectContaining({ + identity: { kind: "gjc-bundle", scope: "user", name: "valid-six-surface-bundle" }, + }), + ], + }); + }); + it("falls back to non-GJC uninstall when the GJC registry is corrupt", async () => { + const agentDir = await fs.mkdtemp(path.join(os.tmpdir(), "gjc-plugin-command-agent-")); + agentDirs.push(agentDir); + const cwd = await makeTempProject(); + const registryRoot = path.join(agentDir, "gjc-plugins"); + await fs.mkdir(registryRoot, { recursive: true }); + await fs.writeFile(path.join(registryRoot, "registry.json"), "{"); + + const result = await runPluginCommand(["uninstall", "not-a-gjc-bundle"], cwd, agentDir); + + expect(`${result.stdout}${result.stderr}`).toMatch(/Uninstalled|Failed to uninstall/); + expect(`${result.stdout}${result.stderr}`).not.toContain("Corrupt GJC plugin registry"); + }); it("GJC install and upgrade failures never echo the source or its cause", async () => { const cwd = await makeTempProject();