fix: scope environments "Test provider" button to clicked row (#8380)
## 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
This commit is contained in:
Devin Foley
@@ -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<void>) {
|
||||
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<string, () => 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(
|
||||
<QueryClientProvider client={queryClient}>
|
||||
<TooltipProvider>
|
||||
<CompanyEnvironments />
|
||||
</TooltipProvider>
|
||||
</QueryClientProvider>,
|
||||
);
|
||||
});
|
||||
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(
|
||||
<QueryClientProvider client={queryClient}>
|
||||
<TooltipProvider>
|
||||
<CompanyEnvironments />
|
||||
</TooltipProvider>
|
||||
</QueryClientProvider>,
|
||||
);
|
||||
});
|
||||
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);
|
||||
});
|
||||
});
|
||||
@@ -166,6 +166,7 @@ export function CompanyEnvironments() {
|
||||
const [editingEnvironmentId, setEditingEnvironmentId] = useState<string | null>(null);
|
||||
const [environmentForm, setEnvironmentForm] = useState<EnvironmentFormState>(createEmptyEnvironmentForm);
|
||||
const [probeResults, setProbeResults] = useState<Record<string, EnvironmentProbeResult | null>>({});
|
||||
const [testingEnvironmentId, setTestingEnvironmentId] = useState<string | null>(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"
|
||||
|
||||
Reference in New Issue
Block a user