mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-10 12:37:55 +01:00
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - The agent configuration UI controls each agent and its allowed secrets. > - The environment variable editor already has a secret selector with search and folder navigation. > - The secret access editor used a basic list and made large secret stores hard to use. > - The secret access controls also occupied the main Configuration tab. > - This pull request reuses the rich selector and moves secret access to a dedicated Secrets tab. > - The benefit is one consistent secret selection workflow with clearer agent configuration navigation. ## Linked Issues or Issue Description **What existing behavior does this improve?** The agent detail configuration view and its secret access editor. **Subsystem affected** `ui/` — React and Vite board UI. **Current behavior** The secret access editor uses a basic select control. It does not provide the search and folder navigation available in the environment variable editor. The editor also appears inside the Configuration tab. **Proposed behavior** The secret access editor uses the shared secret picker. Users can search secrets and browse slash-delimited folders. Agent details provide a dedicated Secrets tab for this editor. **Reason and benefit** Large secret stores are slow to scan in a flat list. Reusing one selector reduces UI differences and makes scoped secret access easier to manage. **Breaking changes** None. The API and saved secret access data do not change. ## What Changed - Reused the environment variable secret picker in the agent secret access editor. - Preserved secret version selection and the create-secret action, including nested-popover focus handling. - Added a route-backed Secrets tab to agent details and removed secret access controls from Configuration. - Guarded unsaved configuration across tab, link, browser-history, and action-triggered navigation. - Rechecked dirty state when navigation-producing agent actions finish, covering edits made while a request is pending. - Added component, page, and Storybook coverage for the workflow. ## Verification - `pnpm --filter @paperclipai/ui exec vitest run src/components/AgentActionButtons.test.tsx src/components/AgentConfigForm.render.test.tsx src/components/AgentSecretAccessEditor.test.tsx src/components/environment-variables-editor/EnvironmentVariablesEditor.test.tsx src/pages/AgentDetail.progress.test.ts` — 82 tests passed. - `pnpm --filter @paperclipai/ui typecheck` - `pnpm check:token-gates` - All GitHub PR checks passed on `5209c5b787`, including build, canary, general and serialized tests, and all three e2e shards. - Greptile completed at 5/5 with zero unresolved review threads. ## Risks - Low risk. The API and persisted binding format are unchanged; this changes agent configuration navigation and secret selection UI. - Dirty-state guards now cover direct navigation, Back/Forward history, and navigation-producing agent actions, including pending-request races. - Tests cover tab separation, secret access updates, search, folder navigation, focus restoration, and navigation rejection. - No documentation change is required because commands, contracts, and setup steps do not change. > For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and discuss it in `#dev` before opening the PR. Feature PRs that overlap with planned core work may need to be redirected — check the roadmap first. See `CONTRIBUTING.md`. ## Model Used OpenAI Codex with GPT-5. This runtime did not expose a more specific model ID or context window. The model used agentic reasoning, repository tools, and code execution. ## 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] 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>
269 lines
9.1 KiB
TypeScript
269 lines
9.1 KiB
TypeScript
// @vitest-environment jsdom
|
|
|
|
import type { ComponentProps, ReactNode } from "react";
|
|
import { flushSync } from "react-dom";
|
|
import { createRoot } from "react-dom/client";
|
|
import { QueryClient, QueryClientProvider } from "@tanstack/react-query";
|
|
import type { Agent } from "@paperclipai/shared";
|
|
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
|
|
import { AgentActionButtons } from "./AgentActionButtons";
|
|
|
|
const mockNavigate = vi.hoisted(() => vi.fn());
|
|
const mockOpenNewIssue = vi.hoisted(() => vi.fn());
|
|
const mockPushToast = vi.hoisted(() => vi.fn());
|
|
const mockAgentsApi = vi.hoisted(() => ({
|
|
invoke: vi.fn(),
|
|
pause: vi.fn(),
|
|
resume: vi.fn(),
|
|
clearError: vi.fn(),
|
|
approve: vi.fn(),
|
|
terminate: vi.fn(),
|
|
resetSession: vi.fn(),
|
|
instructionsBundle: vi.fn(),
|
|
instructionsFile: vi.fn(),
|
|
create: vi.fn(),
|
|
hire: vi.fn(),
|
|
}));
|
|
|
|
vi.mock("@/lib/router", () => ({
|
|
Link: ({ children, to, ...props }: { children: ReactNode; to: string }) => (
|
|
<a href={to} {...props}>{children}</a>
|
|
),
|
|
useNavigate: () => mockNavigate,
|
|
}));
|
|
|
|
vi.mock("../context/DialogContext", () => ({
|
|
useDialogActions: () => ({ openNewIssue: mockOpenNewIssue }),
|
|
}));
|
|
|
|
vi.mock("../context/ToastContext", () => ({
|
|
useToastActions: () => ({ pushToast: mockPushToast }),
|
|
}));
|
|
|
|
vi.mock("../api/agents", () => ({
|
|
agentsApi: mockAgentsApi,
|
|
}));
|
|
|
|
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
|
(globalThis as any).IS_REACT_ACT_ENVIRONMENT = true;
|
|
|
|
async function act(callback: () => void | Promise<void>) {
|
|
let result: void | Promise<void> = undefined;
|
|
flushSync(() => {
|
|
result = callback();
|
|
});
|
|
await result;
|
|
}
|
|
|
|
async function flushReact() {
|
|
await act(async () => {
|
|
await Promise.resolve();
|
|
await new Promise((resolve) => window.setTimeout(resolve, 0));
|
|
});
|
|
}
|
|
|
|
function makeAgent(overrides: Partial<Agent> = {}): Agent {
|
|
return {
|
|
id: "agent-1",
|
|
companyId: "company-1",
|
|
name: "Alpha Agent",
|
|
urlKey: "alpha",
|
|
role: "engineer",
|
|
title: null,
|
|
icon: null,
|
|
status: "active",
|
|
reportsTo: null,
|
|
capabilities: null,
|
|
adapterType: "codex_local",
|
|
adapterConfig: {},
|
|
runtimeConfig: {},
|
|
budgetMonthlyCents: 0,
|
|
spentMonthlyCents: 0,
|
|
pauseReason: null,
|
|
pausedAt: null,
|
|
permissions: { canCreateAgents: false },
|
|
lastHeartbeatAt: null,
|
|
metadata: null,
|
|
createdAt: new Date("2026-01-01T00:00:00Z"),
|
|
updatedAt: new Date("2026-01-01T00:00:00Z"),
|
|
...overrides,
|
|
};
|
|
}
|
|
|
|
describe("AgentActionButtons", () => {
|
|
let container: HTMLDivElement;
|
|
let root: ReturnType<typeof createRoot> | null;
|
|
let queryClient: QueryClient;
|
|
let invalidateQueries: ReturnType<typeof vi.fn>;
|
|
|
|
beforeEach(() => {
|
|
container = document.createElement("div");
|
|
document.body.appendChild(container);
|
|
root = null;
|
|
queryClient = new QueryClient({
|
|
defaultOptions: { queries: { retry: false } },
|
|
});
|
|
invalidateQueries = vi.spyOn(queryClient, "invalidateQueries") as unknown as ReturnType<typeof vi.fn>;
|
|
mockAgentsApi.clearError.mockResolvedValue(makeAgent({ status: "idle" }));
|
|
mockAgentsApi.pause.mockResolvedValue(makeAgent({ status: "paused" }));
|
|
mockAgentsApi.resume.mockResolvedValue(makeAgent({ status: "idle" }));
|
|
mockAgentsApi.terminate.mockResolvedValue(makeAgent({ status: "terminated" }));
|
|
mockAgentsApi.invoke.mockResolvedValue({ id: "run-1" });
|
|
mockAgentsApi.resetSession.mockResolvedValue(undefined);
|
|
});
|
|
|
|
afterEach(async () => {
|
|
const currentRoot = root;
|
|
if (currentRoot) {
|
|
await act(async () => {
|
|
currentRoot.unmount();
|
|
});
|
|
}
|
|
queryClient.clear();
|
|
container.remove();
|
|
document.body.innerHTML = "";
|
|
vi.clearAllMocks();
|
|
});
|
|
|
|
function render(agent: Agent, props: Partial<ComponentProps<typeof AgentActionButtons>> = {}) {
|
|
root ??= createRoot(container);
|
|
root.render(
|
|
<QueryClientProvider client={queryClient}>
|
|
<AgentActionButtons agent={agent} companyId="company-1" runLabel="Run Heartbeat" {...props} />
|
|
</QueryClientProvider>,
|
|
);
|
|
}
|
|
|
|
it("replaces the pause slot with Clear error for error agents", async () => {
|
|
render(makeAgent({ status: "error" }));
|
|
await flushReact();
|
|
|
|
expect(container.textContent).toContain("Clear error");
|
|
expect(container.textContent).not.toContain("Pause");
|
|
expect(container.textContent).toContain("Run Heartbeat");
|
|
|
|
await act(async () => {
|
|
container.querySelector<HTMLButtonElement>('[aria-label="Open actions for Alpha Agent"]')?.click();
|
|
});
|
|
await flushReact();
|
|
|
|
expect(document.body.textContent).toContain("Reset Sessions");
|
|
});
|
|
|
|
it("calls clearError and refreshes agent-related queries", async () => {
|
|
render(makeAgent({ status: "error" }));
|
|
await flushReact();
|
|
|
|
await act(async () => {
|
|
container.querySelector<HTMLButtonElement>('[aria-label="Clear error and return agent to idle"]')?.click();
|
|
});
|
|
await flushReact();
|
|
|
|
expect(mockAgentsApi.clearError).toHaveBeenCalledWith("agent-1", "company-1");
|
|
expect(invalidateQueries).toHaveBeenCalledWith({ queryKey: ["agents", "detail", "agent-1"] });
|
|
expect(invalidateQueries).toHaveBeenCalledWith({ queryKey: ["agents", "detail", "alpha"] });
|
|
expect(invalidateQueries).toHaveBeenCalledWith({ queryKey: ["agents", "runtime-state", "agent-1"] });
|
|
expect(invalidateQueries).toHaveBeenCalledWith({ queryKey: ["agents", "task-sessions", "agent-1"] });
|
|
expect(invalidateQueries).toHaveBeenCalledWith({ queryKey: ["agents", "company-1"] });
|
|
expect(invalidateQueries).toHaveBeenCalledWith({ queryKey: ["live-runs", "company-1"] });
|
|
expect(invalidateQueries).toHaveBeenCalledWith({ queryKey: ["heartbeats", "company-1", "agent-1"] });
|
|
});
|
|
|
|
it("keeps the normal pause action for non-error agents", async () => {
|
|
render(makeAgent({ status: "active" }));
|
|
await flushReact();
|
|
|
|
expect(container.textContent).toContain("Pause");
|
|
expect(container.textContent).not.toContain("Clear error");
|
|
});
|
|
|
|
it("calls the terminate success handler after terminating an agent", async () => {
|
|
const onTerminateSuccess = vi.fn();
|
|
render(makeAgent(), { onTerminateSuccess });
|
|
await flushReact();
|
|
|
|
await act(async () => {
|
|
container.querySelector<HTMLButtonElement>('[aria-label="Open actions for Alpha Agent"]')?.click();
|
|
});
|
|
await flushReact();
|
|
|
|
const terminateButton = Array.from(document.body.querySelectorAll("button"))
|
|
.find((button) => button.textContent?.includes("Terminate"));
|
|
expect(terminateButton).toBeTruthy();
|
|
|
|
await act(async () => {
|
|
terminateButton?.click();
|
|
});
|
|
await flushReact();
|
|
|
|
expect(mockAgentsApi.terminate).toHaveBeenCalledWith("agent-1", "company-1");
|
|
expect(onTerminateSuccess).toHaveBeenCalledWith(expect.objectContaining({
|
|
id: "agent-1",
|
|
status: "terminated",
|
|
}));
|
|
expect(invalidateQueries).toHaveBeenCalledWith({ queryKey: ["agents", "detail", "agent-1"] });
|
|
expect(invalidateQueries).toHaveBeenCalledWith({ queryKey: ["agents", "detail", "alpha"] });
|
|
expect(invalidateQueries).toHaveBeenCalledWith({ queryKey: ["agents", "company-1"] });
|
|
});
|
|
|
|
it("does not terminate when navigation away from a dirty detail page is rejected", async () => {
|
|
const onBeforeNavigate = vi.fn().mockReturnValue(false);
|
|
render(makeAgent(), { onBeforeNavigate, onTerminateSuccess: vi.fn() });
|
|
await flushReact();
|
|
|
|
await act(async () => {
|
|
container.querySelector<HTMLButtonElement>('[aria-label="Open actions for Alpha Agent"]')?.click();
|
|
});
|
|
await flushReact();
|
|
|
|
const terminateButton = Array.from(document.body.querySelectorAll("button"))
|
|
.find((button) => button.textContent?.includes("Terminate"));
|
|
await act(async () => {
|
|
terminateButton?.click();
|
|
});
|
|
await flushReact();
|
|
|
|
expect(onBeforeNavigate).toHaveBeenCalledOnce();
|
|
expect(mockAgentsApi.terminate).not.toHaveBeenCalled();
|
|
});
|
|
|
|
it("rechecks navigation when the form becomes dirty while termination is pending", async () => {
|
|
let resolveTermination!: (agent: Agent) => void;
|
|
mockAgentsApi.terminate.mockReturnValue(new Promise((resolve) => {
|
|
resolveTermination = resolve;
|
|
}));
|
|
const onBeforeNavigate = vi.fn().mockReturnValueOnce(true).mockReturnValue(false);
|
|
const onTerminateSuccess = vi.fn();
|
|
const agent = makeAgent();
|
|
render(agent, {
|
|
hasPendingNavigationChanges: false,
|
|
onBeforeNavigate,
|
|
onTerminateSuccess,
|
|
});
|
|
await flushReact();
|
|
|
|
await act(async () => {
|
|
container.querySelector<HTMLButtonElement>('[aria-label="Open actions for Alpha Agent"]')?.click();
|
|
});
|
|
await flushReact();
|
|
const terminateButton = Array.from(document.body.querySelectorAll("button"))
|
|
.find((button) => button.textContent?.includes("Terminate"));
|
|
await act(async () => {
|
|
terminateButton?.click();
|
|
});
|
|
await flushReact();
|
|
|
|
render(agent, {
|
|
hasPendingNavigationChanges: true,
|
|
onBeforeNavigate,
|
|
onTerminateSuccess,
|
|
});
|
|
await flushReact();
|
|
resolveTermination(makeAgent({ status: "terminated" }));
|
|
await flushReact();
|
|
|
|
expect(onBeforeNavigate).toHaveBeenCalledTimes(2);
|
|
expect(onTerminateSuccess).not.toHaveBeenCalled();
|
|
});
|
|
});
|