From 950484d204e8d39af3f79239403af7a67fa7c87b Mon Sep 17 00:00:00 2001 From: Devin Foley Date: Sat, 20 Jun 2026 00:53:28 -0700 Subject: [PATCH] fix: scope environments "Test provider" button to clicked row (#8380) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Agents run inside environments, and the Environments settings page lets users configure each environment and verify it with a "Test provider" / "Test connection" button per row > - When a user clicked one environment's test button, every environment row's button switched to the disabled "Testing..." state at the same time, then all flipped back together > - That happened because all rows read the same shared `environmentProbeMutation.isPending` flag, so a single in-flight probe disabled and relabeled every button > - This is confusing: it looks like every environment is being tested, and it blocks interacting with other rows while one probe runs > - This pull request tracks the specific environment id being probed in dedicated state and scopes the disabled/label logic to that id > - The benefit is that only the button the user actually clicked shows "Testing..." and is disabled, while the other rows stay interactive ## Linked Issues or Issue Description No public GitHub issue exists, so the bug is described inline below following the bug report template. ### What happened? On the Environments settings page, clicking "Test provider" on one environment caused the test button on *every* environment row to change to "Testing..." and become disabled at the same time, then all reverted together when the probe finished. ### Expected behavior Only the button for the environment the user clicked should show "Testing..." and be disabled while its probe runs. Every other row's button should stay enabled and unchanged. ### Steps to reproduce 1. Open instance settings → Environments with two or more configured environments. 2. Click "Test provider" / "Test connection" on a single row. 3. Observe that all rows' buttons enter the "Testing..." disabled state simultaneously instead of just the clicked one. ### Paperclip version or commit `master` at commit a10f17800 (branch `fix/environments-test-provider-button-scope`). ### Deployment mode Local dev (`pnpm dev`). UI-only; reproduces independent of backend. ## What Changed - Added a dedicated `testingEnvironmentId` state in `ui/src/pages/CompanyEnvironments.tsx` to track which environment is currently being probed. - Set it in the probe mutation's `onMutate` and clear it in `onSettled`, and reset it when the selected company changes. - Scoped the test button's `disabled` state and `"Testing..."` label to `testingEnvironmentId === environment.id` instead of the shared `environmentProbeMutation.isPending` flag. - Added `ui/src/pages/CompanyEnvironments.test.tsx` verifying that clicking one environment's test button puts only that row into the "Testing..." disabled state while other rows stay enabled. ## Verification - `pnpm typecheck` (UI) passes clean. - `vitest run src/pages/CompanyEnvironments.test.tsx` passes (new test fails against the old shared-`isPending` behavior, confirming it guards the fix). - Manual: on the Environments page with multiple environments, click one row's test button and confirm only that button shows "Testing..." / is disabled while the others remain enabled, then it reverts on completion. ## Risks - Low risk. Single-file UI change scoped to per-row button state; no API, schema, or behavior changes to the probe itself. The probe mutation still runs identically — only which buttons reflect the pending state changed. ## Model Used - Claude Opus 4.8 (Anthropic), `claude-opus-4-8`, with extended reasoning and tool use, via Claude Code. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [ ] If this change affects the UI, I have included before/after screenshots - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --- ui/src/pages/CompanyEnvironments.test.tsx | 178 ++++++++++++++++++++++ ui/src/pages/CompanyEnvironments.tsx | 12 +- 2 files changed, 188 insertions(+), 2 deletions(-) create mode 100644 ui/src/pages/CompanyEnvironments.test.tsx diff --git a/ui/src/pages/CompanyEnvironments.test.tsx b/ui/src/pages/CompanyEnvironments.test.tsx new file mode 100644 index 00000000..f0d9237c --- /dev/null +++ b/ui/src/pages/CompanyEnvironments.test.tsx @@ -0,0 +1,178 @@ +// @vitest-environment jsdom + +import { createRoot } from "react-dom/client"; +import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { TooltipProvider } from "@/components/ui/tooltip"; +import { CompanyEnvironments } from "./CompanyEnvironments"; + +const mockEnvironmentsApi = vi.hoisted(() => ({ + list: vi.fn(), + capabilities: vi.fn(), + probe: vi.fn(), + probeConfig: vi.fn(), + create: vi.fn(), + update: vi.fn(), + remove: vi.fn(), + setDefault: vi.fn(), +})); +const mockInstanceSettingsApi = vi.hoisted(() => ({ + getExperimental: vi.fn(), +})); +const mockSecretsApi = vi.hoisted(() => ({ + list: vi.fn(), +})); + +vi.mock("@/context/CompanyContext", () => ({ + useCompany: () => ({ + selectedCompanyId: "company-1", + selectedCompany: { id: "company-1", name: "Paperclip" }, + }), +})); + +vi.mock("@/context/BreadcrumbContext", () => ({ + useBreadcrumbs: () => ({ setBreadcrumbs: vi.fn() }), +})); + +vi.mock("@/context/ToastContext", () => ({ + useToast: () => ({ pushToast: vi.fn() }), +})); + +vi.mock("@/api/environments", () => ({ + environmentsApi: mockEnvironmentsApi, +})); + +vi.mock("@/api/instanceSettings", () => ({ + instanceSettingsApi: mockInstanceSettingsApi, +})); + +vi.mock("@/api/secrets", () => ({ + secretsApi: mockSecretsApi, +})); + +// eslint-disable-next-line @typescript-eslint/no-explicit-any +(globalThis as any).IS_REACT_ACT_ENVIRONMENT = true; + +async function act(callback: () => void | Promise) { + await callback(); + await Promise.resolve(); + await new Promise((resolve) => window.setTimeout(resolve, 0)); +} + +async function flushReact() { + for (let i = 0; i < 3; i += 1) { + await Promise.resolve(); + await new Promise((resolve) => window.setTimeout(resolve, 0)); + } +} + +function testProviderButtons(container: HTMLElement): HTMLButtonElement[] { + return Array.from(container.querySelectorAll("button")).filter((button) => { + const label = button.textContent?.trim(); + return label === "Test provider" || label === "Testing..."; + }); +} + +describe("CompanyEnvironments — test provider button", () => { + let container: HTMLDivElement; + let probeResolvers: Map void>; + + beforeEach(() => { + container = document.createElement("div"); + document.body.appendChild(container); + probeResolvers = new Map(); + mockInstanceSettingsApi.getExperimental.mockResolvedValue({ enableEnvironments: true }); + mockEnvironmentsApi.capabilities.mockResolvedValue({ adapters: [], sandboxProviders: {} }); + mockSecretsApi.list.mockResolvedValue([]); + mockEnvironmentsApi.list.mockResolvedValue([ + { id: "env-1", name: "Alpha", driver: "sandbox", description: null, config: { provider: "e2b" } }, + { id: "env-2", name: "Beta", driver: "sandbox", description: null, config: { provider: "e2b" } }, + ]); + // Each probe stays pending until its resolver is called, so the testing + // state remains observable and can be settled per environment. + mockEnvironmentsApi.probe.mockImplementation( + (environmentId: string) => + new Promise<{ ok: boolean; driver: string; summary: string; details: null }>((resolve) => { + probeResolvers.set(environmentId, () => + resolve({ ok: true, driver: "sandbox", summary: "ok", details: null }), + ); + }), + ); + }); + + afterEach(() => { + container.remove(); + document.body.innerHTML = ""; + vi.clearAllMocks(); + }); + + it("shows the testing state only on the clicked environment's button", async () => { + const root = createRoot(container); + const queryClient = new QueryClient({ defaultOptions: { queries: { retry: false } } }); + + await act(async () => { + root.render( + + + + + , + ); + }); + await flushReact(); + + const buttonsBefore = testProviderButtons(container); + expect(buttonsBefore).toHaveLength(2); + expect(buttonsBefore.every((button) => button.textContent?.trim() === "Test provider")).toBe(true); + expect(buttonsBefore.every((button) => !button.disabled)).toBe(true); + + await act(async () => { + buttonsBefore[0].dispatchEvent(new MouseEvent("click", { bubbles: true })); + }); + await flushReact(); + + const buttonsAfter = testProviderButtons(container); + expect(buttonsAfter).toHaveLength(2); + expect(buttonsAfter[0].textContent?.trim()).toBe("Testing..."); + expect(buttonsAfter[0].disabled).toBe(true); + expect(buttonsAfter[1].textContent?.trim()).toBe("Test provider"); + expect(buttonsAfter[1].disabled).toBe(false); + expect(mockEnvironmentsApi.probe).toHaveBeenCalledExactlyOnceWith("env-1"); + }); + + it("keeps the second environment's testing state when an earlier probe settles", async () => { + const root = createRoot(container); + const queryClient = new QueryClient({ defaultOptions: { queries: { retry: false } } }); + + await act(async () => { + root.render( + + + + + , + ); + }); + await flushReact(); + + // Click both rows in quick succession while both probes are still pending. + await act(async () => { + testProviderButtons(container)[0].dispatchEvent(new MouseEvent("click", { bubbles: true })); + }); + await flushReact(); + await act(async () => { + testProviderButtons(container)[1].dispatchEvent(new MouseEvent("click", { bubbles: true })); + }); + await flushReact(); + + // Settle only the first environment's probe. + await act(async () => { + probeResolvers.get("env-1")?.(); + }); + await flushReact(); + + const buttons = testProviderButtons(container); + expect(buttons[1].textContent?.trim()).toBe("Testing..."); + expect(buttons[1].disabled).toBe(true); + }); +}); diff --git a/ui/src/pages/CompanyEnvironments.tsx b/ui/src/pages/CompanyEnvironments.tsx index 0c83302f..a2e42aef 100644 --- a/ui/src/pages/CompanyEnvironments.tsx +++ b/ui/src/pages/CompanyEnvironments.tsx @@ -166,6 +166,7 @@ export function CompanyEnvironments() { const [editingEnvironmentId, setEditingEnvironmentId] = useState(null); const [environmentForm, setEnvironmentForm] = useState(createEmptyEnvironmentForm); const [probeResults, setProbeResults] = useState>({}); + const [testingEnvironmentId, setTestingEnvironmentId] = useState(null); useEffect(() => { setBreadcrumbs([ @@ -232,6 +233,12 @@ export function CompanyEnvironments() { const environmentProbeMutation = useMutation({ mutationFn: async (environmentId: string) => await environmentsApi.probe(environmentId), + onMutate: (environmentId) => { + setTestingEnvironmentId(environmentId); + }, + onSettled: (_probe, _error, environmentId) => { + setTestingEnvironmentId((current) => (current === environmentId ? null : current)); + }, onSuccess: (probe, environmentId) => { setProbeResults((current) => ({ ...current, @@ -287,6 +294,7 @@ export function CompanyEnvironments() { setEditingEnvironmentId(null); setEnvironmentForm(createEmptyEnvironmentForm()); setProbeResults({}); + setTestingEnvironmentId(null); }, [selectedCompanyId]); function handleEditEnvironment(environment: Environment) { @@ -530,9 +538,9 @@ export function CompanyEnvironments() { size="sm" variant="outline" onClick={() => environmentProbeMutation.mutate(environment.id)} - disabled={environmentProbeMutation.isPending} + disabled={testingEnvironmentId === environment.id} > - {environmentProbeMutation.isPending + {testingEnvironmentId === environment.id ? "Testing..." : environment.driver === "ssh" ? "Test connection"