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 <noreply@paperclip.ing>
This commit is contained in:
@@ -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<AdapterEnvironmentTestResult> => {
|
||||
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<AdapterEnvironmentTestResult> => ({
|
||||
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>): Environment {
|
||||
return {
|
||||
id: "env-1",
|
||||
|
||||
@@ -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<unknown>;
|
||||
runTest: () => Promise<AdapterEnvironmentTestResult>;
|
||||
}) {
|
||||
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<string | null>(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
|
||||
|
||||
Reference in New Issue
Block a user