From e281095cb4e8985db0c1055b40a2d44aba7f2c63 Mon Sep 17 00:00:00 2001 From: Devin Foley Date: Sat, 20 Jun 2026 16:04:25 -0700 Subject: [PATCH] fix(ui): make adapter test button test only (#8405) ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Agent adapter configuration is where operators edit the runtime settings that control how each agent is invoked. > - The adapter form includes a test action so operators can validate the in-progress configuration before saving it. > - In edit mode, that test action changed into `Save + Test` when the form was dirty, which mixed validation with persistence. > - Operators already have an explicit Save action for committing adapter changes. > - This pull request makes the adapter test action test-only so validation never implicitly saves a dirty draft. > - The benefit is clearer operator control: Test validates the current draft, Save persists it. ## Linked Issues or Issue Description No matching public GitHub issue or in-flight PR was found, so this PR describes the bug inline. ### Pre-submission checklist - I searched existing open and closed issues and did not find a duplicate. - I can reproduce this behavior on current `master` before this branch. - I confirmed this originates in the Paperclip UI, not in a specific local agent CLI or provider configuration. ### What happened? When editing an existing agent adapter configuration, changing an adapter setting made the adapter environment test button change from `Test` to `Save + Test`. Clicking it saved dirty edits before running the environment test. ### Expected behavior The test action should always be a test action. Dirty draft values should be validated without persisting them, and the explicit Save button should be the only save path. ### Steps to reproduce 1. Open an existing agent adapter configuration. 2. Edit an adapter setting so the form becomes dirty. 3. Observe the adapter environment test action label and behavior. ### Paperclip version or commit Current `master` before this branch, `c0743482b`. ### Deployment mode Local dev/private operator UI. ### Installation method Built from source with pnpm. ### Agent adapter(s) involved Not adapter-specific. The behavior lives in the shared agent adapter config form. ### Database mode Not database-related. ### Access context Board operator UI. ### Additional context Public searches performed before opening this PR: - `adapter test save` - `Save + Test` - `AgentConfigForm testEnvironment` - `adapter configuration test button` No logs or local config are needed for this UI-only behavior report. ## What Changed - Removed the helper that converted dirty edit-mode adapter tests into `Save + Test`. - Removed the save-before-test helper path so clicking Test only invokes the adapter environment test mutation. - Kept draft testing behavior by continuing to build the test payload from the current in-progress adapter config. - Dropped unit tests that asserted the old save-and-test behavior. ## Verification - `pnpm vitest run ui/src/components/AgentConfigForm.test.ts` -> 6/6 passed. - `pnpm --filter @paperclipai/ui typecheck` was attempted locally and failed on the existing unrelated `packages/plugins/sdk/src/ui/components.ts` React module-resolution error before reaching this change. - Static review: `Save + Test`, `getAgentConfigTestActionLabel`, and `runAgentConfigEnvironmentTest` no longer appear in `ui/src`. - Before/after visual state for the dirty edit-mode adapter test action: - Before: `Save + Test` - After: `Test` - PR CI passed: policy, commitperclip review, typecheck/release-registry, build, general tests, serialized server suites, e2e, canary dry run, aggregate verify, and security scans. - Greptile passed with 5/5 confidence. The remaining screenshot/documentation thread was answered and resolved because this is a text-only button-label state already captured above. ## Risks Low risk. This is a narrow UI behavior change in the adapter config form. The main behavior change is intentional: testing a dirty adapter draft no longer persists it. If an operator expected Test to save edits, they must now use the explicit Save button. ## Model Used - Anthropic Claude via Paperclip `claude_local` produced the initial implementation commit. - OpenAI GPT-5 Codex via Paperclip `codex_local` reviewed the change, amended public commit metadata, pushed the branch, and prepared this pull request. The adapter did not expose an exact context-window value in the run payload; tool use and local shell execution were used. ## 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 (e.g. `docs/...`, `fix/...`) 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 - [x] If this change affects the UI, I have included before/after screenshots - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [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 Co-authored-by: Paperclip --- ui/src/components/AgentConfigForm.test.ts | 65 ++--------------------- ui/src/components/AgentConfigForm.tsx | 27 ++-------- 2 files changed, 6 insertions(+), 86 deletions(-) diff --git a/ui/src/components/AgentConfigForm.test.ts b/ui/src/components/AgentConfigForm.test.ts index 51e2e107..befe41c3 100644 --- a/ui/src/components/AgentConfigForm.test.ts +++ b/ui/src/components/AgentConfigForm.test.ts @@ -1,10 +1,6 @@ -import { describe, expect, it, vi } from "vitest"; -import type { AdapterEnvironmentTestResult, Environment } from "@paperclipai/shared"; -import { - getAgentConfigTestActionLabel, - runAgentConfigEnvironmentTest, - supportsAdapterModelRefresh, -} from "./AgentConfigForm"; +import { describe, expect, it } from "vitest"; +import type { Environment } from "@paperclipai/shared"; +import { supportsAdapterModelRefresh } from "./AgentConfigForm"; import { resolveForcedKubernetesEnvironment } from "../lib/forced-kubernetes-environment"; describe("supportsAdapterModelRefresh", () => { @@ -20,61 +16,6 @@ describe("supportsAdapterModelRefresh", () => { }); }); -describe("agent config test action", () => { - it("labels dirty edit-mode tests as save-and-test", () => { - expect(getAgentConfigTestActionLabel({ isCreate: false, isDirty: true })).toBe("Save + Test"); - expect(getAgentConfigTestActionLabel({ isCreate: false, isDirty: false })).toBe("Test"); - expect(getAgentConfigTestActionLabel({ isCreate: true, isDirty: true })).toBe("Test"); - }); - - it("saves a dirty edit draft before running the environment test", async () => { - const callOrder: string[] = []; - const saveDraft = vi.fn(async () => { - callOrder.push("save"); - }); - const runTest = vi.fn(async (): Promise => { - callOrder.push("test"); - return { - adapterType: "claude_local", - status: "pass", - checks: [], - testedAt: new Date(0).toISOString(), - }; - }); - - await runAgentConfigEnvironmentTest({ - isCreate: false, - isDirty: true, - saveDraft, - runTest, - }); - - expect(saveDraft).toHaveBeenCalledTimes(1); - expect(runTest).toHaveBeenCalledTimes(1); - expect(callOrder).toEqual(["save", "test"]); - }); - - it("runs create-mode tests without saving first", async () => { - const saveDraft = vi.fn(async () => {}); - const runTest = vi.fn(async (): Promise => ({ - adapterType: "claude_local", - status: "pass", - checks: [], - testedAt: new Date(0).toISOString(), - })); - - await runAgentConfigEnvironmentTest({ - isCreate: true, - isDirty: true, - saveDraft, - runTest, - }); - - expect(saveDraft).not.toHaveBeenCalled(); - expect(runTest).toHaveBeenCalledTimes(1); - }); -}); - function makeEnvironment(overrides: Partial): Environment { return { id: "env-1", diff --git a/ui/src/components/AgentConfigForm.tsx b/ui/src/components/AgentConfigForm.tsx index 5355d2f1..2b2316f2 100644 --- a/ui/src/components/AgentConfigForm.tsx +++ b/ui/src/components/AgentConfigForm.tsx @@ -116,22 +116,6 @@ export function supportsAdapterModelRefresh(adapterType: string): boolean { return adapterType === "claude_local" || adapterType === "codex_local" || adapterType === "acpx_local"; } -export function getAgentConfigTestActionLabel(input: { isCreate: boolean; isDirty: boolean }): string { - return !input.isCreate && input.isDirty ? "Save + Test" : "Test"; -} - -export async function runAgentConfigEnvironmentTest(input: { - isCreate: boolean; - isDirty: boolean; - saveDraft?: () => void | Promise; - runTest: () => Promise; -}) { - if (!input.isCreate && input.isDirty) { - await input.saveDraft?.(); - } - return await input.runTest(); -} - function isOverlayDirty(o: AgentConfigOverlay): boolean { return ( Object.keys(o.identity).length > 0 || @@ -555,7 +539,7 @@ export function AgentConfigForm(props: AgentConfigFormProps) { }); const [testActionPending, setTestActionPending] = useState(false); const [testActionError, setTestActionError] = useState(null); - const testActionLabel = getAgentConfigTestActionLabel({ isCreate, isDirty }); + const testActionLabel = "Test"; const isSavePending = !isCreate && Boolean(props.isSaving); const testEnvironmentDisabled = testActionPending || isSavePending || !selectedCompanyId; const runEnvironmentTest = useCallback(async () => { @@ -566,19 +550,14 @@ export function AgentConfigForm(props: AgentConfigFormProps) { setTestActionError(null); testEnvironment.reset(); try { - return await runAgentConfigEnvironmentTest({ - isCreate, - isDirty, - saveDraft: !isCreate ? handleSave : undefined, - runTest: () => testEnvironment.mutateAsync(), - }); + return await testEnvironment.mutateAsync(); } catch (error) { setTestActionError(error instanceof Error ? error.message : "Environment test failed"); throw error; } finally { setTestActionPending(false); } - }, [selectedCompanyId, isCreate, isDirty, handleSave, testEnvironment]); + }, [selectedCompanyId, testEnvironment]); // `runEnvironmentTest` (and `testEnvironmentDisabled`) change identity on every // render because `useMutation` returns a fresh result object each time. Hold the // latest behavior in a ref so the trigger handed to the parent stays referentially