Skip to content

Commit 6cd047e

Browse files
steipetesteipete
committed
refactor: clean up update and plugin uninstall helpers
1 parent d58ede1 commit 6cd047e

7 files changed

Lines changed: 268 additions & 209 deletions

File tree

‎docs/install/updating.md‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,12 @@ the installer, pass `--install-method git --no-onboard` or
7373
npm i -g openclaw@latest
7474
```
7575

76+
When `openclaw update` manages a global npm install, it first runs the normal
77+
global install command. If that command fails, OpenClaw retries once with
78+
`--omit=optional`. That retry helps hosts where native optional dependencies
79+
cannot compile, while keeping the original failure visible if the fallback also
80+
fails.
81+
7682
```bash
7783
pnpm add -g openclaw@latest
7884
```

‎src/cli/plugins-cli-test-helpers.ts‎

Lines changed: 20 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import type { Mock } from "vitest";
33
import { vi } from "vitest";
44
import type { OpenClawConfig } from "../config/types.openclaw.js";
55
import type { PluginInstallRecord } from "../config/types.plugins.js";
6+
import { createEmptyUninstallActions } from "../plugins/uninstall.js";
67
import { createCliRuntimeCapture } from "./test-runtime-capture.js";
78

89
type UnknownMock = Mock<(...args: unknown[]) => unknown>;
@@ -309,20 +310,24 @@ vi.mock("../plugins/slots.js", async (importOriginal) => {
309310
};
310311
});
311312

312-
vi.mock("../plugins/uninstall.js", () => ({
313-
uninstallPlugin: ((
314-
...args: Parameters<(typeof import("../plugins/uninstall.js"))["uninstallPlugin"]>
315-
) =>
316-
invokeMock<
317-
Parameters<(typeof import("../plugins/uninstall.js"))["uninstallPlugin"]>,
318-
ReturnType<(typeof import("../plugins/uninstall.js"))["uninstallPlugin"]>
319-
>(uninstallPlugin, ...args)) as (typeof import("../plugins/uninstall.js"))["uninstallPlugin"],
320-
resolveUninstallDirectoryTarget: ({
321-
installRecord,
322-
}: {
323-
installRecord?: { installPath?: string; sourcePath?: string };
324-
}) => installRecord?.installPath ?? installRecord?.sourcePath ?? null,
325-
}));
313+
vi.mock("../plugins/uninstall.js", async (importOriginal) => {
314+
const actual = await importOriginal<typeof import("../plugins/uninstall.js")>();
315+
return {
316+
...actual,
317+
uninstallPlugin: ((
318+
...args: Parameters<(typeof import("../plugins/uninstall.js"))["uninstallPlugin"]>
319+
) =>
320+
invokeMock<
321+
Parameters<(typeof import("../plugins/uninstall.js"))["uninstallPlugin"]>,
322+
ReturnType<(typeof import("../plugins/uninstall.js"))["uninstallPlugin"]>
323+
>(uninstallPlugin, ...args)) as (typeof import("../plugins/uninstall.js"))["uninstallPlugin"],
324+
resolveUninstallDirectoryTarget: ({
325+
installRecord,
326+
}: {
327+
installRecord?: { installPath?: string; sourcePath?: string };
328+
}) => installRecord?.installPath ?? installRecord?.sourcePath ?? null,
329+
};
330+
});
326331

327332
vi.mock("../plugins/update.js", () => ({
328333
updateNpmInstalledPlugins: ((
@@ -588,15 +593,7 @@ export function resetPluginsCliTestState() {
588593
ok: true,
589594
config: {} as OpenClawConfig,
590595
warnings: [],
591-
actions: {
592-
entry: false,
593-
install: false,
594-
allowlist: false,
595-
loadPath: false,
596-
memorySlot: false,
597-
contextEngineSlot: false,
598-
directory: false,
599-
},
596+
actions: createEmptyUninstallActions(),
600597
});
601598
updateNpmInstalledPlugins.mockResolvedValue({
602599
outcomes: [],

‎src/cli/plugins-cli.ts‎

Lines changed: 11 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@ import {
1414
} from "../plugins/installed-plugin-index-records.js";
1515
import { listMarketplacePlugins } from "../plugins/marketplace.js";
1616
import { inspectPluginRegistry, refreshPluginRegistry } from "../plugins/plugin-registry.js";
17-
import { defaultSlotIdForKey } from "../plugins/slots.js";
1817
import { formatPluginSourceForTable, resolvePluginSourceRoots } from "../plugins/source-display.js";
1918
import {
2019
buildAllPluginInspectReports,
@@ -26,8 +25,11 @@ import {
2625
} from "../plugins/status.js";
2726
import type { PluginLogger } from "../plugins/types.js";
2827
import {
28+
formatUninstallActionLabels,
29+
formatUninstallSlotResetPreview,
2930
resolveUninstallChannelConfigKeys,
3031
resolveUninstallDirectoryTarget,
32+
UNINSTALL_ACTION_LABELS,
3133
uninstallPlugin,
3234
} from "../plugins/uninstall.js";
3335
import { defaultRuntime } from "../runtime.js";
@@ -616,35 +618,33 @@ export function registerPluginsCli(program: Command) {
616618
const isLinked = install?.source === "path";
617619
const preview: string[] = [];
618620
if (hasEntry) {
619-
preview.push("config entry");
621+
preview.push(UNINSTALL_ACTION_LABELS.entry);
620622
}
621623
if (hasInstall) {
622-
preview.push("install record");
624+
preview.push(UNINSTALL_ACTION_LABELS.install);
623625
}
624626
if (cfg.plugins?.allow?.includes(pluginId)) {
625-
preview.push("allowlist entry");
627+
preview.push(UNINSTALL_ACTION_LABELS.allowlist);
626628
}
627629
if (
628630
isLinked &&
629631
install?.sourcePath &&
630632
cfg.plugins?.load?.paths?.includes(install.sourcePath)
631633
) {
632-
preview.push("load path");
634+
preview.push(UNINSTALL_ACTION_LABELS.loadPath);
633635
}
634636
if (cfg.plugins?.slots?.memory === pluginId) {
635-
preview.push(`memory slot (will reset to "${defaultSlotIdForKey("memory")}")`);
637+
preview.push(formatUninstallSlotResetPreview("memory"));
636638
}
637639
if (cfg.plugins?.slots?.contextEngine === pluginId) {
638-
preview.push(
639-
`context engine slot (will reset to "${defaultSlotIdForKey("contextEngine")}")`,
640-
);
640+
preview.push(formatUninstallSlotResetPreview("contextEngine"));
641641
}
642642
const channelIds = plugin?.status === "loaded" ? plugin.channelIds : undefined;
643643
const channels = cfg.channels as Record<string, unknown> | undefined;
644644
if (hasInstall && channels) {
645645
for (const key of resolveUninstallChannelConfigKeys(pluginId, { channelIds })) {
646646
if (Object.hasOwn(channels, key)) {
647-
preview.push(`channel config (channels.${key})`);
647+
preview.push(`${UNINSTALL_ACTION_LABELS.channelConfig} (channels.${key})`);
648648
}
649649
}
650650
}
@@ -712,31 +712,7 @@ export function registerPluginsCli(program: Command) {
712712
},
713713
});
714714

715-
const removed: string[] = [];
716-
if (result.actions.entry) {
717-
removed.push("config entry");
718-
}
719-
if (result.actions.install) {
720-
removed.push("install record");
721-
}
722-
if (result.actions.allowlist) {
723-
removed.push("allowlist");
724-
}
725-
if (result.actions.loadPath) {
726-
removed.push("load path");
727-
}
728-
if (result.actions.memorySlot) {
729-
removed.push("memory slot");
730-
}
731-
if (result.actions.contextEngineSlot) {
732-
removed.push("context engine slot");
733-
}
734-
if (result.actions.channelConfig) {
735-
removed.push("channel config");
736-
}
737-
if (result.actions.directory) {
738-
removed.push("directory");
739-
}
715+
const removed = formatUninstallActionLabels(result.actions);
740716

741717
defaultRuntime.log(
742718
`Uninstalled plugin "${pluginId}". Removed: ${removed.length > 0 ? removed.join(", ") : "nothing"}.`,

‎src/cli/update-cli/update-command.ts‎

Lines changed: 34 additions & 77 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import { resolveGatewayInstallEntrypoint } from "../../daemon/gateway-entrypoint
1919
import { resolveGatewayRestartLogPath } from "../../daemon/restart-logs.js";
2020
import { resolveGatewayService } from "../../daemon/service.js";
2121
import { createLowDiskSpaceWarning } from "../../infra/disk-space.js";
22+
import { runGlobalPackageUpdateSteps } from "../../infra/package-update-steps.js";
2223
import { nodeVersionSatisfiesEngine } from "../../infra/runtime-guard.js";
2324
import {
2425
channelToNpmTag,
@@ -33,13 +34,10 @@ import {
3334
checkUpdateStatus,
3435
} from "../../infra/update-check.js";
3536
import {
36-
collectInstalledGlobalPackageErrors,
3737
canResolveRegistryVersionForPackageTarget,
3838
createGlobalInstallEnv,
3939
cleanupGlobalRenameDirs,
40-
globalInstallFallbackArgs,
4140
globalInstallArgs,
42-
resolveExpectedInstalledVersionFromSpec,
4341
resolveGlobalInstallTarget,
4442
resolveGlobalInstallSpec,
4543
} from "../../infra/update-global.js";
@@ -399,86 +397,45 @@ async function runPackageInstallUpdate(params: {
399397
}
400398
}
401399

402-
const updateStep = await runUpdateStep({
403-
name: "global update",
404-
argv: globalInstallArgs(installTarget, installSpec),
405-
env: installEnv,
400+
const packageUpdate = await runGlobalPackageUpdateSteps({
401+
installTarget,
402+
installSpec,
403+
packageName,
404+
packageRoot: pkgRoot,
405+
runCommand,
406406
timeoutMs: params.timeoutMs,
407-
progress: params.progress,
408-
});
409-
410-
const steps = [updateStep];
411-
let finalInstallStep = updateStep;
412-
if (updateStep.exitCode !== 0) {
413-
const fallbackArgv = globalInstallFallbackArgs(installTarget, installSpec);
414-
if (fallbackArgv) {
415-
const fallbackStep = await runUpdateStep({
416-
name: "global update (omit optional)",
417-
argv: fallbackArgv,
418-
env: installEnv,
419-
timeoutMs: params.timeoutMs,
420-
progress: params.progress,
421-
});
422-
steps.push(fallbackStep);
423-
finalInstallStep = fallbackStep;
424-
}
425-
}
426-
let afterVersion = beforeVersion;
427-
428-
const verifiedPackageRoot =
429-
(
430-
await resolveGlobalInstallTarget({
431-
manager: installTarget,
432-
runCommand,
433-
timeoutMs: params.timeoutMs,
434-
})
435-
).packageRoot ?? pkgRoot;
436-
if (verifiedPackageRoot) {
437-
afterVersion = await readPackageVersion(verifiedPackageRoot);
438-
const expectedVersion = resolveExpectedInstalledVersionFromSpec(packageName, installSpec);
439-
const verificationErrors = await collectInstalledGlobalPackageErrors({
440-
packageRoot: verifiedPackageRoot,
441-
expectedVersion,
442-
});
443-
if (verificationErrors.length > 0) {
444-
steps.push({
445-
name: "global install verify",
446-
command: `verify ${verifiedPackageRoot}`,
447-
cwd: verifiedPackageRoot,
448-
durationMs: 0,
449-
exitCode: 1,
450-
stderrTail: verificationErrors.join("\n"),
451-
stdoutTail: null,
452-
});
453-
}
454-
const entryPath = await resolveGatewayInstallEntrypoint(verifiedPackageRoot);
455-
if (entryPath) {
456-
const doctorStep = await runUpdateStep({
457-
name: `${CLI_NAME} doctor`,
458-
argv: [resolveNodeRunner(), entryPath, "doctor", "--non-interactive", "--fix"],
459-
env: {
460-
...process.env,
461-
OPENCLAW_UPDATE_IN_PROGRESS: "1",
462-
},
463-
timeoutMs: params.timeoutMs,
407+
...(installEnv === undefined ? {} : { env: installEnv }),
408+
runStep: (stepParams) =>
409+
runUpdateStep({
410+
...stepParams,
464411
progress: params.progress,
465-
});
466-
steps.push(doctorStep);
467-
}
468-
}
412+
}),
413+
postVerifyStep: async (verifiedPackageRoot) => {
414+
const entryPath = await resolveGatewayInstallEntrypoint(verifiedPackageRoot);
415+
if (entryPath) {
416+
return await runUpdateStep({
417+
name: `${CLI_NAME} doctor`,
418+
argv: [resolveNodeRunner(), entryPath, "doctor", "--non-interactive", "--fix"],
419+
env: {
420+
...process.env,
421+
OPENCLAW_UPDATE_IN_PROGRESS: "1",
422+
},
423+
timeoutMs: params.timeoutMs,
424+
progress: params.progress,
425+
});
426+
}
427+
return null;
428+
},
429+
});
469430

470-
const failedStep =
471-
finalInstallStep.exitCode !== 0
472-
? finalInstallStep
473-
: (steps.find((step) => step !== updateStep && step.exitCode !== 0) ?? null);
474431
return {
475-
status: failedStep ? "error" : "ok",
432+
status: packageUpdate.failedStep ? "error" : "ok",
476433
mode: manager,
477-
root: verifiedPackageRoot ?? params.root,
478-
reason: failedStep ? failedStep.name : undefined,
434+
root: packageUpdate.verifiedPackageRoot ?? params.root,
435+
reason: packageUpdate.failedStep ? packageUpdate.failedStep.name : undefined,
479436
before: { version: beforeVersion },
480-
after: { version: afterVersion },
481-
steps,
437+
after: { version: packageUpdate.afterVersion ?? beforeVersion },
438+
steps: packageUpdate.steps,
482439
durationMs: Date.now() - params.startedAt,
483440
};
484441
}

0 commit comments

Comments
 (0)