From f249b1c6df5b92111ca7898305729615e04282da Mon Sep 17 00:00:00 2001 From: Vincent Koc Date: Sat, 2 May 2026 23:17:40 -0700 Subject: [PATCH] fix(plugins): clean managed git uninstall roots --- CHANGELOG.md | 1 + scripts/e2e/lib/plugins/assertions.mjs | 70 ++++++++++++++++++++++++++ scripts/e2e/lib/plugins/sweep.sh | 8 +++ src/plugins/uninstall.test.ts | 32 +++++++++++- src/plugins/uninstall.ts | 55 +++++++++++++++----- 5 files changed, 153 insertions(+), 13 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index bc57db3f96c..89d07b1ad38 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -27,6 +27,7 @@ Docs: https://docs.openclaw.ai - Docker/Gateway: pass Docker setup `.env` values into gateway and CLI containers and preserve exec SecretRef `passEnv` keys in managed service plans, so 1Password Connect-backed Discord tokens keep resolving after doctor or plugin repair. Thanks @vincentkoc. - Control UI/WebChat: explain compaction boundaries in chat history and link directly to session checkpoint controls so pre-compaction turns no longer look silently lost after refresh. Fixes #76415. Thanks @BunsDev. - Channels/WhatsApp: attach native outbound mention metadata for group text and media captions by resolving `@+` and `@` tokens against WhatsApp participant data, including LID groups. Fixes #39879; carries forward #56863. Thanks @kengi1437, @joe2643, and @fridayck. +- Plugins/uninstall: remove empty managed git install parent directories after deleting cloned plugin repos and cover npm/git uninstall residue in Docker plugin lifecycle tests. Thanks @vincentkoc. - Plugins/install: resolve bare official external plugin IDs such as `brave` through the official catalog when no bundled source is available, so packaged installs fetch the intended scoped npm package instead of an unrelated unscoped package. Fixes #76373. Thanks @bek91 and @vincentkoc. - Gateway/sessions: keep async `sessions.list` title and preview hydration bounded to transcript head/tail reads so Control UI polling cannot full-scan large session transcripts every refresh. Thanks @vincentkoc. - Gateway/sessions: keep agent runtime metadata on lightweight `sessions.list` rows so model-only session patches do not make Control UI lose runtime identity. Thanks @vincentkoc. diff --git a/scripts/e2e/lib/plugins/assertions.mjs b/scripts/e2e/lib/plugins/assertions.mjs index 9c27245633b..9643233c67f 100644 --- a/scripts/e2e/lib/plugins/assertions.mjs +++ b/scripts/e2e/lib/plugins/assertions.mjs @@ -18,6 +18,34 @@ function getInstallRecords() { : (index.installRecords ?? {}); } +function readOpenClawConfig() { + const configPath = path.join(process.env.HOME, ".openclaw", "openclaw.json"); + return fs.existsSync(configPath) ? readJson(configPath) : {}; +} + +function assertPluginRemoved(params) { + const list = readJson(params.listFile); + if ((list.plugins || []).some((entry) => entry.id === params.pluginId)) { + throw new Error(`${params.pluginId} still listed after uninstall`); + } + + const installRecords = getInstallRecords(); + if (installRecords[params.pluginId]) { + throw new Error(`${params.pluginId} install record still present after uninstall`); + } + + const config = readOpenClawConfig(); + if (config.plugins?.entries?.[params.pluginId]) { + throw new Error(`${params.pluginId} config entry still present after uninstall`); + } + if ((config.plugins?.allow || []).includes(params.pluginId)) { + throw new Error(`${params.pluginId} allowlist entry still present after uninstall`); + } + if ((config.plugins?.deny || []).includes(params.pluginId)) { + throw new Error(`${params.pluginId} denylist entry still present after uninstall`); + } +} + function recordFixturePluginTrust() { const pluginId = process.argv[3]; const pluginRoot = process.argv[4]; @@ -272,6 +300,25 @@ function assertGitPlugin() { throw new Error(`missing git plugin installed dependency: ${dependencyPackagePath}`); } assertRealPathInside(installPath, dependencyPackagePath, "git plugin installed dependency"); + fs.writeFileSync("/tmp/plugins-git-install-path.txt", installPath, "utf8"); + fs.writeFileSync("/tmp/plugins-git-install-parent.txt", path.dirname(installPath), "utf8"); +} + +function assertGitPluginRemoved() { + const installPath = fs.readFileSync("/tmp/plugins-git-install-path.txt", "utf8").trim(); + const installParent = fs.readFileSync("/tmp/plugins-git-install-parent.txt", "utf8").trim(); + assertPluginRemoved({ + pluginId: "demo-plugin-git", + listFile: "/tmp/plugins-git-uninstalled.json", + }); + if (fs.existsSync(installPath)) { + throw new Error(`git managed repo still exists after uninstall: ${installPath}`); + } + if (fs.existsSync(installParent)) { + throw new Error( + `empty git managed install parent still exists after uninstall: ${installParent}`, + ); + } } function assertRealPathInside(parentPath, childPath, label) { @@ -407,6 +454,8 @@ function assertNpmPlugin() { throw new Error(`missing npm plugin installed dependency: ${dependencyPackagePath}`); } assertRealPathInside(npmRoot, dependencyPackagePath, "npm plugin installed dependency"); + fs.writeFileSync("/tmp/plugins-npm-install-path.txt", installPath, "utf8"); + fs.writeFileSync("/tmp/plugins-npm-dependency-path.txt", dependencyPackagePath, "utf8"); } function assertNpmPluginUpdateUnchanged() { @@ -414,6 +463,25 @@ function assertNpmPluginUpdateUnchanged() { assertNpmPlugin(); } +function assertNpmPluginRemoved() { + const installPath = fs.readFileSync("/tmp/plugins-npm-install-path.txt", "utf8").trim(); + const dependencyPackagePath = fs + .readFileSync("/tmp/plugins-npm-dependency-path.txt", "utf8") + .trim(); + assertPluginRemoved({ + pluginId: "demo-plugin-npm", + listFile: "/tmp/plugins-npm-uninstalled.json", + }); + if (fs.existsSync(installPath)) { + throw new Error(`npm managed package still exists after uninstall: ${installPath}`); + } + if (fs.existsSync(dependencyPackagePath)) { + throw new Error( + `npm managed dependency still exists after uninstall: ${dependencyPackagePath}`, + ); + } +} + function assertMarketplaceUpdated() { const data = readJson("/tmp/plugins-marketplace-updated.json"); const inspect = readJson("/tmp/plugins-marketplace-updated-inspect.json"); @@ -646,10 +714,12 @@ const commands = { ), "plugin-npm": assertNpmPlugin, "plugin-npm-update": assertNpmPluginUpdateUnchanged, + "plugin-npm-removed": assertNpmPluginRemoved, "bundle-disabled": assertClaudeBundleDisabled, "bundle-inspect": assertClaudeBundleInspect, "slash-install": assertSlashInstall, "plugin-git": assertGitPlugin, + "plugin-git-removed": assertGitPluginRemoved, "plugin-git-updated": assertGitPluginUpdated, "marketplace-list": assertMarketplaceList, "marketplace-installed": assertMarketplaceInstalled, diff --git a/scripts/e2e/lib/plugins/sweep.sh b/scripts/e2e/lib/plugins/sweep.sh index 3dab93defc7..b4d20636928 100644 --- a/scripts/e2e/lib/plugins/sweep.sh +++ b/scripts/e2e/lib/plugins/sweep.sh @@ -87,6 +87,10 @@ node scripts/e2e/lib/plugins/assertions.mjs plugin-npm node "$OPENCLAW_ENTRY" plugins update demo-plugin-npm >/tmp/plugins-npm-update.log 2>&1 node scripts/e2e/lib/plugins/assertions.mjs plugin-npm-update +run_logged uninstall-npm node "$OPENCLAW_ENTRY" plugins uninstall demo-plugin-npm --force +node "$OPENCLAW_ENTRY" plugins list --json >/tmp/plugins-npm-uninstalled.json +node scripts/e2e/lib/plugins/assertions.mjs plugin-npm-removed + echo "Testing install from git repo and plugin CLI execution..." git_fixture_root="$(mktemp -d "/tmp/openclaw-plugin-git.XXXXXX")" git_repo="$git_fixture_root/repo" @@ -106,6 +110,10 @@ run_logged exec-git-plugin-cli bash -c 'node "$OPENCLAW_ENTRY" demo-git ping >/t node scripts/e2e/lib/plugins/assertions.mjs plugin-git "$git_repo_url" "$git_ref" +run_logged uninstall-git node "$OPENCLAW_ENTRY" plugins uninstall demo-plugin-git --force +node "$OPENCLAW_ENTRY" plugins list --json >/tmp/plugins-git-uninstalled.json +node scripts/e2e/lib/plugins/assertions.mjs plugin-git-removed + echo "Testing git plugin update from moving ref..." git_update_fixture_root="$(mktemp -d "/tmp/openclaw-plugin-git-update.XXXXXX")" git_update_repo="$git_update_fixture_root/repo" diff --git a/src/plugins/uninstall.test.ts b/src/plugins/uninstall.test.ts index 5310ca10ace..a8829a6e127 100644 --- a/src/plugins/uninstall.test.ts +++ b/src/plugins/uninstall.test.ts @@ -1264,7 +1264,8 @@ describe("uninstallPlugin", () => { it("deletes managed git install repos outside the extensions directory", async () => { const stateDir = path.join(tempDir, "state"); const extensionsDir = path.join(stateDir, "extensions"); - const installPath = path.join(stateDir, "git", "git-abc123", "repo"); + const installParent = path.join(stateDir, "git", "git-abc123"); + const installPath = path.join(installParent, "repo"); await fs.mkdir(installPath, { recursive: true }); await fs.writeFile(path.join(installPath, "index.js"), "// git plugin"); @@ -1284,6 +1285,35 @@ describe("uninstallPlugin", () => { directory: true, }); await expect(fs.access(installPath)).rejects.toThrow(); + await expect(fs.access(installParent)).rejects.toThrow(); + }); + + it("keeps non-empty managed git install parents after deleting the repo", async () => { + const stateDir = path.join(tempDir, "state"); + const extensionsDir = path.join(stateDir, "extensions"); + const installParent = path.join(stateDir, "git", "git-abc123"); + const installPath = path.join(installParent, "repo"); + await fs.mkdir(installPath, { recursive: true }); + await fs.writeFile(path.join(installPath, "index.js"), "// git plugin"); + await fs.writeFile(path.join(installParent, "keep.txt"), "keep"); + + const result = await uninstallPlugin({ + config: createPluginConfig({ + entries: createSinglePluginEntries(), + installs: { + "my-plugin": createGitInstallRecord("my-plugin", installPath), + }, + }), + pluginId: "my-plugin", + deleteFiles: true, + extensionsDir, + }); + + expectSuccessfulUninstallActions(result, { + directory: true, + }); + await expect(fs.access(installPath)).rejects.toThrow(); + await expect(fs.access(path.join(installParent, "keep.txt"))).resolves.toBeUndefined(); }); it("does not delete symlinked git install targets that resolve outside the managed git root", async () => { diff --git a/src/plugins/uninstall.ts b/src/plugins/uninstall.ts index a01b0a9430e..9f737592e3e 100644 --- a/src/plugins/uninstall.ts +++ b/src/plugins/uninstall.ts @@ -98,11 +98,16 @@ export type UninstallPluginResult = export type PluginUninstallDirectoryRemoval = { target: string; - cleanup?: { - kind: "npm"; - npmRoot: string; - packageName: string; - }; + cleanup?: + | { + kind: "npm"; + npmRoot: string; + packageName: string; + } + | { + kind: "git"; + parentDir: string; + }; }; export type PluginUninstallPlanResult = @@ -136,12 +141,12 @@ export function resolveUninstallDirectoryTarget(params: { if (npmManagedInstall) { return npmManagedInstall.installPath; } - const gitManagedPath = resolveGitManagedInstallPath({ + const gitManagedInstall = resolveGitManagedInstall({ installRecord: params.installRecord, extensionsDir: params.extensionsDir, }); - if (gitManagedPath) { - return gitManagedPath; + if (gitManagedInstall) { + return gitManagedInstall.installPath; } let defaultPath: string; @@ -226,10 +231,10 @@ function resolveNpmPackageNameFromInstallPath(params: { return segments[0] ?? null; } -function resolveGitManagedInstallPath(params: { +function resolveGitManagedInstall(params: { installRecord?: PluginInstallRecord; extensionsDir?: string; -}): string | null { +}): { installPath: string; parentDir: string } | null { const installPath = params.installRecord?.installPath?.trim(); if (params.installRecord?.source !== "git" || !installPath) { return null; @@ -246,7 +251,7 @@ function resolveGitManagedInstallPath(params: { isPathInsideOrEqual(gitRoot, installPath) && resolveComparablePath(gitRoot) !== resolveComparablePath(installPath) ) { - return installPath; + return { installPath, parentDir: path.dirname(installPath) }; } } return null; @@ -519,6 +524,13 @@ export function planPluginUninstall(params: UninstallPluginParams): PluginUninst extensionsDir, }) : null; + const gitManagedInstall = + deleteFiles && !isLinked + ? resolveGitManagedInstall({ + installRecord, + extensionsDir, + }) + : null; const deleteTarget = deleteFiles && !isLinked @@ -546,7 +558,14 @@ export function planPluginUninstall(params: UninstallPluginParams): PluginUninst packageName: npmManagedInstall.packageName, }, } - : {}), + : gitManagedInstall && deleteTarget === gitManagedInstall.installPath + ? { + cleanup: { + kind: "git", + parentDir: gitManagedInstall.parentDir, + }, + } + : {}), } : null, }; @@ -599,6 +618,18 @@ export async function applyPluginUninstallDirectoryRemoval( } try { await fs.rm(removal.target, { recursive: true, force: true }); + if (removal.cleanup?.kind === "git") { + try { + await fs.rmdir(removal.cleanup.parentDir); + } catch (error) { + const code = (error as NodeJS.ErrnoException).code; + if (code !== "ENOENT" && code !== "ENOTEMPTY") { + warnings.push( + `Failed to remove empty git plugin install parent ${removal.cleanup.parentDir}: ${formatErrorMessage(error)}`, + ); + } + } + } return { directoryRemoved: existed, warnings }; } catch (error) { return {