diff --git a/ui/src/i18n/locales/en.ts b/ui/src/i18n/locales/en.ts index c84b8a0f6216..b2986462d8f6 100644 --- a/ui/src/i18n/locales/en.ts +++ b/ui/src/i18n/locales/en.ts @@ -2794,6 +2794,7 @@ export const en: TranslationMap = { unavailable: "Unavailable", install: "Install", installing: "Installing…", + checkingInstallOutcome: "Checking…", installNamed: "Install {name}", acknowledgeRisk: "Acknowledge risk and install", defaultRiskWarning: "Review the ClawHub warning before installing this plugin.", @@ -2806,6 +2807,10 @@ export const en: TranslationMap = { policyReviewSeverityCritical: "Critical", policyReviewTechnicalDetails: "Details", installAnyway: "Install anyway", + installOutcomeChecking: + "The Gateway connection changed before OpenClaw confirmed this install. Checking plugin status…", + installOutcomeFailed: + "OpenClaw couldn’t confirm whether this plugin was installed. Retry the status check before installing again.", connectToChange: "Connect to the gateway to change plugins.", adminRequired: "Browsing only. Plugin changes require operator.admin access.", changesDisabled: "Browsing only. This gateway does not allow plugin changes.", diff --git a/ui/src/pages/plugins/plugins-page.test-support.ts b/ui/src/pages/plugins/plugins-page.test-support.ts index 0b3679d7535d..d13b612c0c7a 100644 --- a/ui/src/pages/plugins/plugins-page.test-support.ts +++ b/ui/src/pages/plugins/plugins-page.test-support.ts @@ -20,6 +20,7 @@ import { } from "../../test-helpers/application-context.ts"; import type { PluginsRouteData } from "./plugins-page.ts"; import type { PluginRowMessage } from "./view.ts"; +import type { InstallOutcomeReconciliation } from "./view.ts"; import "./plugins-page.ts"; type RequestHandler = (method: string, params: unknown) => Promise; @@ -35,11 +36,13 @@ type TestPluginsPage = HTMLElement & { result: PluginListResult | null; loading: boolean; busy: Record; + installOutcomeReconciliations: Record; messages: Record; activeTab: "installed" | "discover"; searchResults: PluginSearchResult[] | null; applyMutationResult: (result: PluginMutationResult) => void; install: (rowKey: string, request: PluginInstallRequest) => Promise; + refreshCatalog: () => Promise; updateEnabled: (pluginId: string, enabled: boolean, key?: string) => Promise; uninstall: (pluginId: string, rowKey: string) => Promise; }; diff --git a/ui/src/pages/plugins/plugins-page.test.ts b/ui/src/pages/plugins/plugins-page.test.ts index 4e480c221df5..4f0bedcfc7c9 100644 --- a/ui/src/pages/plugins/plugins-page.test.ts +++ b/ui/src/pages/plugins/plugins-page.test.ts @@ -201,8 +201,9 @@ describe("PluginsPage", () => { ); }); - it("retires a pending install-policy review across a same-client reconnect", async () => { + it("reconciles a pending install-policy retry before allowing another install", async () => { const retry = deferred(); + const catalogRefresh = deferred(); let installCalls = 0; const { client, request } = createClient(async (method) => { if (method === "plugins.install") { @@ -224,7 +225,7 @@ describe("PluginsPage", () => { return retry.promise; } if (method === "plugins.list") { - return createResult(); + return catalogRefresh.promise; } throw new Error(`Unexpected method ${method}`); }); @@ -252,11 +253,24 @@ describe("PluginsPage", () => { expect(request.mock.calls.filter(([method]) => method === "plugins.install")).toHaveLength(2), ); expect(page.messages[rowKey]?.installPolicyWarning).toBeDefined(); + page.messages["plugin:workboard"] = { kind: "success", text: "Unrelated message." }; harness.emit(client, false); harness.emit(client, true); - await page.updateComplete; + await waitForFast(() => + expect(request.mock.calls.filter(([method]) => method === "plugins.list")).toHaveLength(1), + ); expect(page.messages[rowKey]).toBeUndefined(); + expect(page.messages["plugin:workboard"]?.text).toBe("Unrelated message."); + expect(page.installOutcomeReconciliations[rowKey]).toBe("checking"); + + catalogRefresh.resolve( + createResult( + createPlugin({ id: "lobster", name: "Lobster", installed: true, enabled: true }), + ), + ); + await waitForFast(() => expect(page.installOutcomeReconciliations[rowKey]).toBeUndefined()); + expect(page.result?.plugins[0]?.installed).toBe(true); retry.resolve({ ok: true, @@ -265,6 +279,73 @@ describe("PluginsPage", () => { }); await pendingRetry; expect(page.messages[rowKey]).toBeUndefined(); + expect(page.result?.plugins[0]?.installed).toBe(true); + }); + + it("keeps an unknown install outcome blocked until a failed catalog check is retried", async () => { + const retry = deferred(); + let installCalls = 0; + let listCalls = 0; + const { client } = createClient(async (method) => { + if (method === "plugins.install") { + installCalls += 1; + if (installCalls === 1) { + throw new GatewayRequestError({ + code: "INVALID_REQUEST", + message: "install requires review", + details: { + installPolicyCode: "install_policy_warning_acknowledgement_required", + targetName: "@openclaw/lobster", + targetType: "plugin", + requestMode: "install", + reason: "Review this plugin.", + acknowledgementToken: "approval-token", + }, + }); + } + return retry.promise; + } + if (method === "plugins.list") { + listCalls += 1; + if (listCalls === 1) { + throw new Error("catalog unavailable"); + } + return createResult( + createPlugin({ + id: "lobster", + name: "Lobster", + installed: false, + install: { source: "clawhub", packageName: "@openclaw/lobster" }, + }), + ); + } + throw new Error(`Unexpected method ${method}`); + }); + const harness = createGateway(client); + const { page } = await mountPage( + createContext(harness.gateway), + createPluginsRouteData(harness.gateway), + ); + const rowKey = "plugin:@openclaw/lobster"; + const installRequest = { + source: "clawhub", + packageName: "@openclaw/lobster", + } satisfies PluginInstallRequest; + + await page.install(rowKey, installRequest); + void page.install(rowKey, { + ...installRequest, + installPolicyWarningAcknowledgement: "approval-token", + }); + await waitForFast(() => expect(installCalls).toBe(2)); + + harness.emit(client, false); + harness.emit(client, true); + await waitForFast(() => expect(page.installOutcomeReconciliations[rowKey]).toBe("failed")); + + await page.refreshCatalog(); + expect(page.installOutcomeReconciliations[rowKey]).toBeUndefined(); + expect(page.result?.plugins[0]?.installed).toBe(false); }); it("debounces two-character ClawHub searches and cancels stale input", async () => { diff --git a/ui/src/pages/plugins/plugins-page.ts b/ui/src/pages/plugins/plugins-page.ts index 5415e5a1f9ea..365b2e3abe94 100644 --- a/ui/src/pages/plugins/plugins-page.ts +++ b/ui/src/pages/plugins/plugins-page.ts @@ -61,6 +61,7 @@ import { pluginRowKey, renderPlugins, type InstalledFilter, + type InstallOutcomeReconciliation, type PluginRowMessage, type PluginsTab, } from "./view.ts"; @@ -129,6 +130,7 @@ class PluginsPage extends OpenClawLightDomElement { @state() private installedFilter: InstalledFilter = "all"; @state() private debouncedSearchQuery = ""; @state() private busy: Record = {}; + @state() private installOutcomeReconciliations: Record = {}; @state() private messages: Record = {}; @state() private pendingRemoval: Record = {}; @state() private detailPluginId: string | null = null; @@ -153,6 +155,7 @@ class PluginsPage extends OpenClawLightDomElement { private readonly gateway = new GatewayPageController(this, { getGateway: () => this.context?.gateway, onIdentityChange: () => { + this.captureUnknownInstallOutcomes(); this.result = null; this.error = null; this.messages = {}; @@ -173,9 +176,11 @@ class PluginsPage extends OpenClawLightDomElement { client ? client.request("plugins.list", {}, { signal }) : initialState, onComplete: (result) => { this.replaceResult(result); + this.clearInstallOutcomeReconciliations(); }, onError: (error) => { this.error = formatUiError(error); + this.failInstallOutcomeReconciliations(); }, }); @@ -363,10 +368,41 @@ class PluginsPage extends OpenClawLightDomElement { void this.configTask.run([null, this.context.runtimeConfig]); void this.searchTask.run([null, ""]); this.mutationTokens.clear(); - // A connection-epoch change makes the one-shot approval outcome unknown; - // retire its action so reconnect cannot resend a spent or process-stale token. - this.messages = Object.fromEntries( - Object.entries(this.messages).filter(([, message]) => !message.installPolicyWarning), + this.captureUnknownInstallOutcomes(); + } + + private captureUnknownInstallOutcomes() { + // Once an acknowledged request crosses a connection epoch, only a fresh catalog can + // prove whether it committed. Keep that safety gate separate from dismissible messages. + const nextMessages = { ...this.messages }; + const nextReconciliations = { ...this.installOutcomeReconciliations }; + for (const [key, message] of Object.entries(this.messages)) { + if (!message.installPolicyWarning) { + continue; + } + if (this.busy[key]) { + nextReconciliations[key] = "checking"; + } + delete nextMessages[key]; + } + this.messages = nextMessages; + this.installOutcomeReconciliations = nextReconciliations; + } + + private clearInstallOutcomeReconciliations() { + if (Object.keys(this.installOutcomeReconciliations).length === 0) { + return; + } + this.installOutcomeReconciliations = {}; + } + + private failInstallOutcomeReconciliations() { + const entries = Object.keys(this.installOutcomeReconciliations); + if (entries.length === 0) { + return; + } + this.installOutcomeReconciliations = Object.fromEntries( + entries.map((key) => [key, "failed" as const]), ); } @@ -571,6 +607,11 @@ class PluginsPage extends OpenClawLightDomElement { if (!client || !this.gateway.connected) { return; } + if (Object.keys(this.installOutcomeReconciliations).length > 0) { + this.installOutcomeReconciliations = Object.fromEntries( + Object.keys(this.installOutcomeReconciliations).map((key) => [key, "checking" as const]), + ); + } this.error = null; await this.catalogTask.run([client]); } @@ -1027,6 +1068,7 @@ class PluginsPage extends OpenClawLightDomElement { searchLoading: this.searchLoading, searchError: this.searchError, busy: this.busy, + installOutcomeReconciliations: this.installOutcomeReconciliations, messages: this.messages, pendingRemoval: this.pendingRemoval, detailPluginId: this.detailPluginId, @@ -1052,6 +1094,7 @@ class PluginsPage extends OpenClawLightDomElement { void this.updateEnabled(pluginId, enabled, rowKey), onInstall: (rowKey, request) => void this.install(rowKey, request), onDismissMessage: (rowKey) => this.setMessage(rowKey, null), + onRetryInstallOutcome: () => void this.refreshCatalog(), onRequestUninstall: (rowKey) => this.setPendingRemoval(rowKey, true), onCancelUninstall: (rowKey) => this.setPendingRemoval(rowKey, false), onUninstall: (pluginId, rowKey) => void this.uninstall(pluginId, rowKey), diff --git a/ui/src/pages/plugins/plugins.e2e.test.ts b/ui/src/pages/plugins/plugins.e2e.test.ts index 559ff549e46c..ce291b5cf05e 100644 --- a/ui/src/pages/plugins/plugins.e2e.test.ts +++ b/ui/src/pages/plugins/plugins.e2e.test.ts @@ -813,7 +813,7 @@ describeControlUiE2e("Control UI Plugins mocked Gateway E2E", () => { } }); - it("retires a pending install-policy review after reconnect", async () => { + it("reconciles a pending install-policy review before allowing another install", async () => { const context = await newContext(); const page = await context.newPage(); const gateway = await installMockGateway(page, { @@ -844,6 +844,7 @@ describeControlUiE2e("Control UI Plugins mocked Gateway E2E", () => { await waitForNextRequest(gateway, "plugins.install", installCountBeforeRetry); await review.getByRole("button", { name: "Installing…", exact: true }).waitFor(); + await gateway.deferNext("plugins.list"); const socketsBeforeReconnect = await gateway.getSocketCount(); await gateway.closeLatest(1001, "install-policy reconnect proof"); await expect @@ -851,10 +852,42 @@ describeControlUiE2e("Control UI Plugins mocked Gateway E2E", () => { .toBeGreaterThan(socketsBeforeReconnect); await review.waitFor({ state: "detached" }); - await row.getByRole("button", { name: "Install Lobster", exact: true }).waitFor(); + await row.getByText("Checking plugin status…", { exact: false }).waitFor(); + const blockedInstall = row.getByRole("button", { name: "Install Lobster", exact: true }); + expect(await blockedInstall.isDisabled()).toBe(true); expect(await row.getByRole("button", { name: "Install anyway", exact: true }).count()).toBe( 0, ); + expect((await gateway.getRequests("plugins.install")).length).toBe(2); + + await gateway.rejectDeferred("plugins.list", { + code: "UNAVAILABLE", + message: "catalog unavailable", + }); + await row + .getByText("couldn’t confirm whether this plugin was installed", { + exact: false, + }) + .waitFor(); + expect(await blockedInstall.isDisabled()).toBe(true); + + const listCountBeforeRetry = (await gateway.getRequests("plugins.list")).length; + await gateway.deferNext("plugins.list"); + await row.getByRole("button", { name: "Retry", exact: true }).click(); + await waitForNextRequest(gateway, "plugins.list", listCountBeforeRetry); + await gateway.resolveDeferred( + "plugins.list", + inventory([workboardDisabled, installedLobsterPlugin, remoteIconPlugin]), + ); + await row.getByRole("button", { name: "Disable", exact: true }).waitFor(); + expect(await row.getByText("couldn’t confirm", { exact: false }).count()).toBe(0); + + await gateway.resolveDeferred("plugins.install", { + ok: true, + plugin: installedLobsterPlugin, + restartRequired: true, + } satisfies PluginMutationResult); + await row.getByRole("button", { name: "Disable", exact: true }).waitFor(); } finally { await context.close(); } diff --git a/ui/src/pages/plugins/view.test.ts b/ui/src/pages/plugins/view.test.ts index a1f689fb8a46..3a7f67b47eb6 100644 --- a/ui/src/pages/plugins/view.test.ts +++ b/ui/src/pages/plugins/view.test.ts @@ -46,6 +46,7 @@ function createProps(overrides: Partial = {}): PluginsViewProp searchLoading: false, searchError: null, busy: {}, + installOutcomeReconciliations: {}, messages: {}, pendingRemoval: {}, detailPluginId: null, @@ -66,6 +67,7 @@ function createProps(overrides: Partial = {}): PluginsViewProp onSetEnabled: () => undefined, onInstall: () => undefined, onDismissMessage: () => undefined, + onRetryInstallOutcome: () => undefined, onRequestUninstall: () => undefined, onCancelUninstall: () => undefined, onUninstall: () => undefined, @@ -764,6 +766,37 @@ describe("renderPlugins", () => { }); }); + it("blocks a repeated install while its reconnect outcome is unresolved", () => { + const plugin = createPlugin({ + id: "lobster", + name: "Lobster", + installed: false, + enabled: false, + state: "disabled", + install: { source: "clawhub", packageName: "@openclaw/lobster" }, + }); + const key = pluginRowKey(plugin.id); + const onRetryInstallOutcome = vi.fn(); + const container = mount( + createProps({ + activeTab: "discover", + result: createResult([plugin]), + installOutcomeReconciliations: { [key]: "failed" }, + onRetryInstallOutcome, + }), + ); + + const row = container.querySelector('[data-plugin-id="lobster"]'); + expect(normalizedText(row?.querySelector('[role="alert"]') ?? null)).toContain( + "couldn’t confirm whether this plugin was installed", + ); + const install = actionButton(row ?? container, "Install Lobster"); + expect(install?.disabled).toBe(true); + expect(normalizedText(install)).toBe("Checking…"); + row?.querySelector('[role="alert"] button')?.click(); + expect(onRetryInstallOutcome).toHaveBeenCalledOnce(); + }); + it("keeps the not-installed outcome visible for reason-only policy warnings", () => { const plugin = createPlugin({ id: "reason-only", diff --git a/ui/src/pages/plugins/view.ts b/ui/src/pages/plugins/view.ts index 0597392636be..1887728c486c 100644 --- a/ui/src/pages/plugins/view.ts +++ b/ui/src/pages/plugins/view.ts @@ -55,6 +55,8 @@ export type PluginRowMessage = { }; }; +export type InstallOutcomeReconciliation = "checking" | "failed"; + type PluginInstallPolicyFinding = NonNullable< PluginInstallPolicyWarningDetails["findings"] >[number]; @@ -84,6 +86,7 @@ type PluginsViewProps = { searchLoading: boolean; searchError: string | null; busy: Readonly>; + installOutcomeReconciliations: Readonly>; messages: Readonly>; pendingRemoval: Readonly>; detailPluginId: string | null; @@ -104,6 +107,7 @@ type PluginsViewProps = { onSetEnabled: (pluginId: string, enabled: boolean, rowKey: string) => void; onInstall: (rowKey: string, request: PluginInstallRequest) => void; onDismissMessage: (rowKey: string) => void; + onRetryInstallOutcome: () => void; onRequestUninstall: (rowKey: string) => void; onCancelUninstall: (rowKey: string) => void; onUninstall: (pluginId: string, rowKey: string) => void; @@ -398,6 +402,28 @@ function renderRowMessage( busy: boolean, props: PluginsViewProps, ) { + const installOutcome = props.installOutcomeReconciliations[key]; + if (installOutcome) { + return html` +
+ + ${t( + installOutcome === "failed" + ? "pluginsPage.installOutcomeFailed" + : "pluginsPage.installOutcomeChecking", + )} + + ${installOutcome === "failed" + ? html`` + : nothing} +
+ `; + } if (!message) { return nothing; } @@ -608,7 +634,11 @@ function renderInstallButton( props.onInstall(key, request); }} > - ${busy ? t("pluginsPage.installing") : t("pluginsPage.install")} + ${busy + ? props.installOutcomeReconciliations[key] + ? t("pluginsPage.checkingInstallOutcome") + : t("pluginsPage.installing") + : t("pluginsPage.install")} `; } @@ -734,7 +764,7 @@ function renderPluginRow( includePackageName = false, ): TemplateResult { const key = pluginRowKey(plugin.id); - const busy = props.busy[key] ?? false; + const busy = Boolean(props.busy[key] || props.installOutcomeReconciliations[key]); return html`