fix(plugins): show install loading state (#2454)

This commit is contained in:
Carlos Florêncio
2026-08-09 12:06:38 +01:00
committed by GitHub
parent 2832170eb4
commit d38d9a0b86
8 changed files with 345 additions and 7 deletions
+35 -6
View File
@@ -8,6 +8,7 @@ import type { ReactElement } from "react";
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
import { toast } from "sonner";
import { SettingsSaveProvider } from "@/components/settings/settings-save-provider";
import type { InstallResult } from "@/lib/api/domains/plugins-api";
import type { PluginRecord, SyncResult } from "@/lib/types/plugins";
vi.mock("sonner", () => ({ toast: { success: vi.fn(), error: vi.fn(), warning: vi.fn() } }));
@@ -95,6 +96,8 @@ const PLUGIN_ID = "acme-tools";
const NEW_PLUGIN_ID = "new-plugin";
const SYNC_BUTTON_TESTID = "plugins-sync-button";
const INSTALL_TRIGGER_TESTID = "install-plugin-trigger";
const INSTALL_URL_INPUT_TESTID = "install-plugin-url-input";
const INSTALL_URL_SUBMIT_TESTID = "install-plugin-url-submit";
const NEW_PLUGIN_URL = "https://example.test/new-plugin-1.0.0.tar.gz";
const UPLOAD_FILENAME = "new-plugin-1.0.0.tar.gz";
@@ -252,15 +255,41 @@ describe("PluginsSettingsPage", () => {
});
describe("PluginsSettingsPage install dialog", () => {
it("shows a spinner while a URL install is pending", async () => {
let resolveInstall!: (result: InstallResult) => void;
const pendingInstall = new Promise<InstallResult>((resolve) => {
resolveInstall = resolve;
});
installPluginFromUrlSpy.mockReturnValueOnce(pendingInstall);
setStoreState([]);
render(<PluginsSettingsPage />);
fireEvent.click(screen.getByTestId(INSTALL_TRIGGER_TESTID));
fireEvent.change(screen.getByTestId(INSTALL_URL_INPUT_TESTID), {
target: { value: NEW_PLUGIN_URL },
});
fireEvent.click(screen.getByTestId(INSTALL_URL_SUBMIT_TESTID));
await vi.waitFor(() => expect(installPluginFromUrlSpy).toHaveBeenCalledWith(NEW_PLUGIN_URL));
const submit = screen.getByTestId(INSTALL_URL_SUBMIT_TESTID);
expect((submit as HTMLButtonElement).disabled).toBe(true);
expect(submit.getAttribute("aria-busy")).toBe("true");
expect(submit.querySelector(".animate-spin")).not.toBeNull();
expect(submit.textContent).toMatch(/installing/i);
resolveInstall({ plugin: installedPlugin() });
await vi.waitFor(() => expect(screen.queryByTestId("install-plugin-dialog")).toBeNull());
});
it("installs a plugin from a URL: calls the API, loads the bundle, and updates the store", async () => {
setStoreState([]);
render(<PluginsSettingsPage />);
fireEvent.click(screen.getByTestId(INSTALL_TRIGGER_TESTID));
fireEvent.change(screen.getByTestId("install-plugin-url-input"), {
fireEvent.change(screen.getByTestId(INSTALL_URL_INPUT_TESTID), {
target: { value: NEW_PLUGIN_URL },
});
fireEvent.click(screen.getByTestId("install-plugin-url-submit"));
fireEvent.click(screen.getByTestId(INSTALL_URL_SUBMIT_TESTID));
await vi.waitFor(() => expect(installPluginFromUrlSpy).toHaveBeenCalledWith(NEW_PLUGIN_URL));
const upsertPlugin = storeState.upsertPlugin as ReturnType<typeof vi.fn>;
@@ -340,10 +369,10 @@ describe("PluginsSettingsPage install dialog state reset", () => {
render(<PluginsSettingsPage />);
fireEvent.click(screen.getByTestId(INSTALL_TRIGGER_TESTID));
fireEvent.change(screen.getByTestId("install-plugin-url-input"), {
fireEvent.change(screen.getByTestId(INSTALL_URL_INPUT_TESTID), {
target: { value: NEW_PLUGIN_URL },
});
fireEvent.click(screen.getByTestId("install-plugin-url-submit"));
fireEvent.click(screen.getByTestId(INSTALL_URL_SUBMIT_TESTID));
await vi.waitFor(() =>
expect(toast.warning).toHaveBeenCalledWith(
@@ -365,10 +394,10 @@ describe("PluginsSettingsPage install dialog state reset", () => {
render(<PluginsSettingsPage />);
fireEvent.click(screen.getByTestId(INSTALL_TRIGGER_TESTID));
fireEvent.change(screen.getByTestId("install-plugin-url-input"), {
fireEvent.change(screen.getByTestId(INSTALL_URL_INPUT_TESTID), {
target: { value: "https://example.test/bad.tar.gz" },
});
fireEvent.click(screen.getByTestId("install-plugin-url-submit"));
fireEvent.click(screen.getByTestId(INSTALL_URL_SUBMIT_TESTID));
await vi.waitFor(() =>
expect(screen.getByTestId("install-plugin-error").textContent).toMatch(/bad checksum/),
@@ -2,6 +2,7 @@
import { useEffect, useRef, useState } from "react";
import { useTranslation } from "react-i18next";
import { IconLoader2 } from "@tabler/icons-react";
import { Button } from "@kandev/ui/button";
import {
Dialog,
@@ -134,9 +135,11 @@ export function InstallPluginDialog({
data-testid={install.testId}
onClick={install.onClick}
disabled={install.disabled}
aria-busy={busy || undefined}
className="cursor-pointer"
>
{t("plugins:install")}
{busy ? <IconLoader2 className="h-4 w-4 animate-spin" aria-hidden="true" /> : null}
{busy ? t("plugins:installing") : t("plugins:install")}
</Button>
</DialogFooter>
</DialogContent>
+34
View File
@@ -0,0 +1,34 @@
import type { Page } from "@playwright/test";
export async function holdPluginInstallResponse(page: Page) {
let release = () => {};
let requestStarted = false;
let markResponseSettled = () => {};
const responseHeld = new Promise<void>((resolve) => {
release = resolve;
});
const responseSettled = new Promise<void>((resolve) => {
markResponseSettled = resolve;
});
let markRequestSeen = () => {};
const requestSeen = new Promise<void>((resolve) => {
markRequestSeen = resolve;
});
await page.route("**/api/plugins/install", async (route) => {
requestStarted = true;
markRequestSeen();
try {
await responseHeld;
await route.fulfill({
status: 500,
contentType: "application/json",
body: JSON.stringify({ error: "install failed" }),
});
} finally {
markResponseSettled();
}
});
return { requestSeen, responseSettled, release, requestStarted: () => requestStarted };
}
@@ -13,6 +13,7 @@ import path from "node:path";
import type { Browser, BrowserContext, Page } from "@playwright/test";
import { expect, test } from "../../fixtures/test-base";
import { PrAssetCapture } from "../../helpers/pr-asset-capture";
import { holdPluginInstallResponse } from "../../helpers/plugin-install";
const PLUGIN_ID = "kandev-plugin-e2e";
const NAV_ITEM_ID = "e2e-hello";
@@ -57,6 +58,37 @@ test.describe("Mobile plugin navigation", () => {
await apiClient.rawRequest("DELETE", `/api/plugins/${PLUGIN_ID}`).catch(() => undefined);
});
test("shows an install spinner while an upload install is pending on a phone", async ({
testPage,
}) => {
test.setTimeout(120_000);
await testPage.goto("/settings/plugins");
await testPage.getByTestId("install-plugin-trigger").click();
const heldInstall = await holdPluginInstallResponse(testPage);
try {
await testPage.getByTestId("install-plugin-tab-upload").tap();
await testPage.getByTestId("install-plugin-file-input").setInputFiles(PACKAGE_PATH);
await testPage.getByTestId("install-plugin-upload-submit").tap();
await heldInstall.requestSeen;
const installButton = testPage.getByTestId("install-plugin-upload-submit");
await expect(installButton).toBeDisabled();
await expect(installButton).toHaveAttribute("aria-busy", "true");
await expect(installButton.locator(".animate-spin")).toBeVisible();
await expect(installButton).toHaveText(/Installing/);
} finally {
heldInstall.release();
if (heldInstall.requestStarted()) await heldInstall.responseSettled;
await testPage.unroute("**/api/plugins/install");
}
const installButton = testPage.getByTestId("install-plugin-upload-submit");
await expect(installButton).toBeEnabled();
await expect(installButton).toHaveText("Install");
await expect(testPage.getByTestId("install-plugin-error")).toContainText("install failed");
});
test("opens a plugin page from the phone menu sheet", async ({
testPage,
backend,
@@ -48,6 +48,7 @@ import type { Page } from "@playwright/test";
import { expect, test } from "../../fixtures/test-base";
import { SessionPage } from "../../pages/session-page";
import type { ApiClient } from "../../helpers/api-client";
import { holdPluginInstallResponse } from "../../helpers/plugin-install";
const PLUGIN_ID = "kandev-plugin-e2e";
const NAV_ITEM_ID = "e2e-hello";
@@ -133,6 +134,32 @@ test.describe("Plugins — gRPC plugin install/load/live-update/uninstall", () =
await uninstallViaApi(apiClient);
});
test("shows an install spinner while an upload install is pending", async ({ testPage }) => {
test.setTimeout(90_000);
await openInstallDialog(testPage);
const heldInstall = await holdPluginInstallResponse(testPage);
try {
await uploadPackage(testPage, PACKAGE_PATH);
await heldInstall.requestSeen;
const installButton = testPage.getByTestId("install-plugin-upload-submit");
await expect(installButton).toBeDisabled();
await expect(installButton).toHaveAttribute("aria-busy", "true");
await expect(installButton.locator(".animate-spin")).toBeVisible();
await expect(installButton).toHaveText(/Installing/);
} finally {
heldInstall.release();
if (heldInstall.requestStarted()) await heldInstall.responseSettled;
await testPage.unroute("**/api/plugins/install");
}
const installButton = testPage.getByTestId("install-plugin-upload-submit");
await expect(installButton).toBeEnabled();
await expect(installButton).toHaveText("Install");
await expect(testPage.getByTestId("install-plugin-error")).toContainText("install failed");
});
test("installs via upload, loads the UI, live-updates via WS+gRPC, and uninstalls", async ({
testPage,
apiClient,
@@ -0,0 +1,124 @@
---
spec: docs/specs/plugins/spec.md
created: 2026-08-08
status: complete
---
# Implementation Plan: Plugin install loading state
## Overview
The install action already owns a single `installBusy` state for URL and upload
installs, and the dialog already disables its submit button while that state is
true. The regression is presentational: the dialog always renders the idle
Install label and no loading indicator, so a user cannot tell whether the
disabled action is still working. The fix keeps the existing install pipeline
and dialog composition, rendering the shared busy state as an animated spinner,
an installing label, and an accessible busy state.
## Confirmed root cause
`usePluginActions.runInstall` sets `installBusy` before invoking either install
API and clears it in `finally`, but `InstallPluginDialog` only consumes `busy`
for `disabled={...}`. Its primary button always renders `t("plugins:install")`
and has no icon or `aria-busy` state. The existing page test passes 16 tests but
does not hold the install promise pending, so it cannot detect this missing
feedback.
## Frontend
### Install dialog
- `apps/web/components/settings/plugins/install-plugin-dialog.tsx`
- Reuse the existing `busy` prop for both URL and upload tabs.
- Render `IconLoader2` with `animate-spin` while busy and keep it hidden when
idle.
- Use the existing `plugins:installing` translation while busy, otherwise keep
`plugins:install`.
- Expose `aria-busy` while the pipeline is pending and preserve the stable
tab-specific test IDs and disabled behavior.
No backend, API, store, plugin contract, persistence, or marketplace changes are
needed. The scope is the manual install dialog shown in the reported screenshot;
marketplace actions already expose their own in-progress label and are unchanged.
## Mobile parity contract
- **Desktop outcome / mobile entry:** both viewports submit the same URL/upload
install through Settings > Plugins; mobile enters through the existing Plugins
settings route and install dialog.
- **Nearest shipped exemplar:** the existing Radix install dialog and
`apps/web/e2e/tests/plugins/mobile-plugin-nav.spec.ts` plugin-settings flow.
They provide the dialog presentation, footer action, and mobile reachability.
- **Hierarchy and primary action:** the dialog remains the focused surface; the
footer Install action remains the primary action and shows the same spinner and
installing label on phone and desktop.
- **Presentation / scroll / safe area:** no composition changes; keep the current
Dialog, its existing viewport behavior, and existing scroll ownership. No new
fixed control or safe-area handling is introduced.
- **Shared state:** `installBusy`, error handling, and submit callbacks remain
shared; no mobile-specific business logic is added.
- **Mobile proof:** extend the existing mobile plugin install flow with a held
install response assertion. That flow already owns the disposable plugin
fixture and cleanup, so it can prove the same primary action without creating a
new mobile surface.
## Tests
- **What:** While a URL or upload install promise is pending, the dialog’s primary
action is disabled, has `aria-busy="true"`, shows an animated spinner, and uses
the installing label; after rejection the spinner disappears and the existing
error/retry state remains available.
**File:** `apps/web/app/settings/plugins/page.test.tsx`.
**How:** hold a mocked install promise with an explicit resolver, assert the
rendered button state before resolving/rejecting, then assert the settled state.
- **What:** Existing URL and upload success paths still close the dialog, update
the store, and load the installed bundle.
**File:** `apps/web/app/settings/plugins/page.test.tsx`.
**How:** run the existing focused page suite unchanged alongside the new
pending-state regression.
## E2E Tests
- **Scenario:** GIVEN an upload in Settings > Plugins, WHEN the install request
is deliberately held before its response reaches the browser, THEN the
desktop Install button is disabled and visibly spinning; after a controlled
failure is released, the action recovers and the inline error remains visible.
**Files:** `apps/web/e2e/helpers/plugin-install.ts` and
`apps/web/e2e/tests/plugins/plugins.spec.ts`.
- **Scenario:** The same pending upload state remains reachable and visually
understandable on the configured Pixel 5 project.
**File:** `apps/web/e2e/tests/plugins/mobile-plugin-nav.spec.ts`, using the
shared install-response helper.
- **Focused commands:**
- `cd apps && pnpm install --frozen-lockfile` (only if this worktree lacks dependencies)
- `cd apps && pnpm --filter @kandev/web test -- --run app/settings/plugins/page.test.tsx`
- `cd apps/web && pnpm e2e:run --project chromium e2e/tests/plugins/plugins.spec.ts -- --grep "install.*spinner|spinner.*install"`
- `cd apps/web && pnpm e2e:run --project mobile-chrome e2e/tests/plugins/mobile-plugin-nav.spec.ts -- --grep "install.*spinner|spinner.*install"`
- `cd apps/web && pnpm run typecheck`
## Verification Results
- `make build-backend build-web` — passed; Vite production assets rebuilt.
- `cd apps && pnpm --filter @kandev/web test -- --run app/settings/plugins/page.test.tsx` — passed, 17 tests.
- `cd apps/web && pnpm e2e:run --host --no-build e2e/tests/plugins/plugins.spec.ts -- --grep "install.*spinner|spinner.*install"` — passed, 1 Chromium test.
- `cd apps/web && pnpm e2e:run --host --no-build --project mobile-chrome e2e/tests/plugins/mobile-plugin-nav.spec.ts -- --grep "install.*spinner|spinner.*install"` — passed, 1 Pixel 5 test.
- `cd apps/web && pnpm run typecheck` — passed.
- Targeted ESLint for the changed component, page test, and desktop/mobile plugin specs — passed with zero warnings.
- `git diff --check` — passed.
- Interactive dev-browser validation confirmed `disabled`, `aria-busy="true"`, the animated spinner, and `Installing…` before the held response was released.
- Managed screenshot capture passed and generated two fresh, compressed, secret-free assets in `.pr-assets/manifest.json` for desktop and Pixel 5 viewports; the disposable capture spec was removed afterward.
- CodeRabbit's valid duplication finding was fixed by moving the held-install
route helper into `apps/web/e2e/helpers/plugin-install.ts`; both focused E2E
regressions passed again afterward.
## Implementation Waves And Parallel Candidates
Wave 1 (sequential):
- [x] [task-01-install-loading-state](task-01-install-loading-state.md)
The dialog, page regression test, and focused desktop/mobile E2E evidence share
the same install state and should be implemented and verified as one vertical
slice. The marketplace’s separate install/update buttons are not changed by this
fix.
@@ -0,0 +1,78 @@
---
id: "01-install-loading-state"
title: "Show install loading state"
status: done
wave: 1
depends_on: []
plan: "plan.md"
spec: "../../specs/plugins/spec.md"
---
# Task 01: Show install loading state
## Acceptance
- While a URL or upload install is pending, the manual install dialog’s primary
action is disabled, marked busy, shows an animated spinner, and uses the
existing installing translation.
- When the install settles successfully, the existing dialog-close, plugin-store,
and bundle-load behavior is unchanged; when it fails, the spinner stops and the
existing inline error/retry state remains available.
- The same primary-action behavior remains reachable on the configured mobile
plugin settings flow; no mobile layout or touch target regression is introduced.
## Verification
- `cd apps && pnpm install --frozen-lockfile` if workspace dependencies are absent.
- `cd apps && pnpm --filter @kandev/web test -- --run app/settings/plugins/page.test.tsx`
- `cd apps/web && pnpm e2e:run --project chromium e2e/tests/plugins/plugins.spec.ts -- --grep "install.*spinner|spinner.*install"`
- `cd apps/web && pnpm e2e:run --project mobile-chrome e2e/tests/plugins/mobile-plugin-nav.spec.ts -- --grep "install.*spinner|spinner.*install"`
- `cd apps/web && pnpm run typecheck`
## Files likely touched
- `apps/web/components/settings/plugins/install-plugin-dialog.tsx`
- `apps/web/app/settings/plugins/page.test.tsx`
- `apps/web/e2e/tests/plugins/plugins.spec.ts`
- `apps/web/e2e/tests/plugins/mobile-plugin-nav.spec.ts`
## Dependencies
None.
## Parallelism
Sequential. The UI and tests share the same transient install-state contract.
## Inputs
- `docs/specs/plugins/spec.md` install pipeline and install-dialog scenario.
- `docs/plans/plugin-install-loading-state/plan.md` root-cause evidence and
mobile parity contract.
- Existing `installBusy` wiring in
`apps/web/components/settings/plugins/use-plugin-actions.ts`.
- Existing spinner pattern in `apps/web/components/settings/install-agent-card.tsx`.
## Output contract
Summarize the changed files, the pending-state regression coverage, exact test
commands and results, any E2E fixture limitations, and synchronized task/plan
status updates. Keep the plugin API and install pipeline unchanged.
## Results
- Updated `install-plugin-dialog.tsx` to render the existing installing
translation with an animated `IconLoader2` and `aria-busy` while the shared
install pipeline is pending; disabled behavior and test IDs remain unchanged.
- Added the URL pending-state component regression and desktop/mobile held-route
E2E coverage. The shared held-install helper in
`apps/web/e2e/helpers/plugin-install.ts` is used by both E2E specs. The tests
verify recovery to the idle Install action and the existing inline error state
after a controlled failure.
- Preserved the existing successful URL/upload install pipeline, store updates,
bundle loading, dialog close behavior, and marketplace install controls.
- Verification passed: production build, focused page suite (17 tests),
Chromium E2E (1 test), Pixel 5 E2E (1 test), typecheck, targeted ESLint,
`git diff --check`, interactive browser validation, and managed desktop/mobile
screenshot capture. After PR review, the shared-helper fix was rechecked with
the focused Chromium and Pixel 5 E2E tests plus targeted ESLint.
+11
View File
@@ -59,6 +59,9 @@ surfaces. The core stays small; the ecosystem grows independently.
- Plugins are distributed as a signed-or-unsigned release **tarball** and installed
either by **URL** (kandev downloads it) or by **manual upload** (multipart file).
There is no manifest-paste registration step.
- The Settings > Plugins install dialog SHALL show the primary Install action as
busy while an install is in flight, including an animated loading indicator and
an installing label, while keeping the action disabled until the pipeline settles.
- Capability-based access control: a plugin can only call Host RPCs it declared in its
manifest; undeclared capabilities are rejected with a gRPC `PermissionDenied` status.
- **Kandev owns the plugin process lifecycle**: it extracts the package, spawns the
@@ -771,6 +774,14 @@ complete.
kandev runs the same verify → validate → extract → spawn pipeline and the plugin
reaches `active` without any URL ever being contacted.
- **GIVEN** the operator has entered a valid URL or selected a package in the
Settings > Plugins install dialog, **WHEN** the operator submits the install,
**THEN** the primary Install action is disabled, marked busy, and shows an
animated loading indicator with the installing label until the install and
post-install loading pipeline settles. **WHEN** the pipeline succeeds, **THEN**
the dialog closes as usual. **WHEN** it fails, **THEN** the indicator stops, the
action becomes available for retry, and the existing inline error remains visible.
- **GIVEN** an operator who extracted a plugin package directly into
`~/.kandev/plugins/<id>/<version>/` on the host filesystem (no install call), **WHEN**
the operator clicks **Sync** in Settings > Plugins (`POST /api/plugins/sync`),