mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-16 05:17:41 +02: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>
268 lines
8.9 KiB
TypeScript
268 lines
8.9 KiB
TypeScript
// @vitest-environment jsdom
|
|
|
|
import { flushSync } from "react-dom";
|
|
import { createRoot, type Root } from "react-dom/client";
|
|
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
|
|
import type { CompanySecret, EnvSecretRefBinding } from "@paperclipai/shared";
|
|
|
|
const mockSecretPickerRender = vi.hoisted(() => vi.fn());
|
|
|
|
// Keep this component test focused on access-row behavior while asserting that
|
|
// the editor routes selection through the shared, folder-aware env picker.
|
|
vi.mock("./environment-variables-editor/SecretPicker", () => ({
|
|
SecretPicker: (props: {
|
|
secretId: string;
|
|
secrets: readonly CompanySecret[];
|
|
onSelect: (secretId: string) => void;
|
|
onCreateNew?: (query: string) => void;
|
|
}) => {
|
|
mockSecretPickerRender(props);
|
|
return <>
|
|
<button type="button" data-testid="pick-secret" onClick={() => props.onSelect("s1")}>
|
|
pick
|
|
</button>
|
|
{props.onCreateNew ? (
|
|
<button type="button" data-testid="create-secret" onClick={() => props.onCreateNew?.("new_secret")}>
|
|
create
|
|
</button>
|
|
) : null}
|
|
</>;
|
|
},
|
|
}));
|
|
|
|
import {
|
|
AgentSecretAccessEditor,
|
|
parseAccessGrants,
|
|
parseEnvSecretRefs,
|
|
rowsToAccessMap,
|
|
summarizeAgentBindings,
|
|
} from "./AgentSecretAccessEditor";
|
|
|
|
function makeSecret(id: string, name: string): CompanySecret {
|
|
return {
|
|
id,
|
|
companyId: "co",
|
|
scope: "company",
|
|
ownerUserId: null,
|
|
userSecretDefinitionId: null,
|
|
key: id,
|
|
name,
|
|
provider: "local_encrypted",
|
|
status: "active",
|
|
managedMode: "paperclip_managed",
|
|
externalRef: null,
|
|
providerConfigId: null,
|
|
providerMetadata: null,
|
|
latestVersion: 1,
|
|
description: null,
|
|
lastResolvedAt: null,
|
|
lastRotatedAt: null,
|
|
deletedAt: null,
|
|
createdByAgentId: null,
|
|
createdByUserId: null,
|
|
createdAt: new Date(0),
|
|
updatedAt: new Date(0),
|
|
};
|
|
}
|
|
|
|
describe("AgentSecretAccessEditor model", () => {
|
|
const config = {
|
|
env: {
|
|
GH_TOKEN: { type: "secret_ref", secretId: "s1", version: 2 },
|
|
PLAIN: { type: "plain", value: "hi" },
|
|
},
|
|
"access.STRIPE": { type: "secret_ref", secretId: "s1" },
|
|
"access.BROKEN": { type: "plain", value: "nope" },
|
|
model: "claude",
|
|
};
|
|
|
|
it("parses env secret refs, ignoring plain values", () => {
|
|
expect(parseEnvSecretRefs(config)).toEqual([{ name: "GH_TOKEN", secretId: "s1", version: 2 }]);
|
|
});
|
|
|
|
it("parses only well-formed top-level access.* secret refs", () => {
|
|
expect(parseAccessGrants(config)).toEqual([{ name: "STRIPE", secretId: "s1", version: "latest" }]);
|
|
});
|
|
|
|
it("summarizes bindings per secret with both delivery modes", () => {
|
|
const summary = summarizeAgentBindings(parseEnvSecretRefs(config), parseAccessGrants(config));
|
|
expect(summary).toEqual([{ secretId: "s1", envKeys: ["GH_TOKEN"], apiAliases: ["STRIPE"] }]);
|
|
});
|
|
|
|
it("drops incomplete, invalid-alias, and unselected rows from the emitted access map", () => {
|
|
expect(
|
|
rowsToAccessMap([
|
|
{ id: "1", alias: "OK", secretId: "s1", version: "latest" },
|
|
{ id: "2", alias: "", secretId: "s1", version: "latest" }, // no alias
|
|
{ id: "3", alias: "1BAD", secretId: "s1", version: "latest" }, // invalid alias
|
|
{ id: "4", alias: "NOSECRET", secretId: "", version: "latest" }, // no secret
|
|
]),
|
|
).toEqual({ OK: { type: "secret_ref", secretId: "s1", version: "latest" } });
|
|
});
|
|
});
|
|
|
|
describe("AgentSecretAccessEditor component", () => {
|
|
let container: HTMLDivElement;
|
|
let root: Root | null = null;
|
|
|
|
beforeEach(() => {
|
|
container = document.createElement("div");
|
|
document.body.appendChild(container);
|
|
});
|
|
|
|
afterEach(() => {
|
|
flushSync(() => root?.unmount());
|
|
root = null;
|
|
container.remove();
|
|
vi.clearAllMocks();
|
|
});
|
|
|
|
function render(node: React.ReactNode) {
|
|
root = createRoot(container);
|
|
flushSync(() => root!.render(node));
|
|
}
|
|
|
|
const secrets = [makeSecret("s1", "STRIPE_KEY")];
|
|
|
|
it("shows the delivery-mode overview for existing bindings", () => {
|
|
render(
|
|
<AgentSecretAccessEditor
|
|
config={{
|
|
env: { GH_TOKEN: { type: "secret_ref", secretId: "s1" } },
|
|
"access.STRIPE": { type: "secret_ref", secretId: "s1" },
|
|
}}
|
|
secrets={secrets}
|
|
onChange={() => {}}
|
|
/>,
|
|
);
|
|
expect(container.textContent).toContain("STRIPE_KEY");
|
|
expect(container.textContent).toContain("Env var");
|
|
expect(container.textContent).toContain("API access");
|
|
expect(container.textContent).toContain("env.GH_TOKEN");
|
|
expect(container.textContent).toContain("access.STRIPE");
|
|
});
|
|
|
|
it("adds an API-access grant, emitting an access.<ALIAS> secret_ref", () => {
|
|
const emitted: Array<Record<string, EnvSecretRefBinding>> = [];
|
|
render(
|
|
<AgentSecretAccessEditor
|
|
config={{}}
|
|
secrets={secrets}
|
|
onChange={(next) => emitted.push(next)}
|
|
/>,
|
|
);
|
|
|
|
// "Add API access" appends an editable row.
|
|
const addButton = [...container.querySelectorAll("button")].find(
|
|
(button) => button.textContent?.includes("Add API access"),
|
|
)!;
|
|
flushSync(() => addButton.click());
|
|
|
|
// Type an alias.
|
|
const aliasInput = container.querySelector<HTMLInputElement>('input[aria-label="Access alias"]')!;
|
|
const setter = Object.getOwnPropertyDescriptor(window.HTMLInputElement.prototype, "value")!.set!;
|
|
setter.call(aliasInput, "STRIPE");
|
|
flushSync(() => aliasInput.dispatchEvent(new Event("input", { bubbles: true })));
|
|
|
|
// Bind a secret via the stubbed picker.
|
|
const pick = container.querySelector<HTMLButtonElement>('[data-testid="pick-secret"]')!;
|
|
expect(mockSecretPickerRender).toHaveBeenLastCalledWith(
|
|
expect.objectContaining({ secretId: "", secrets }),
|
|
);
|
|
flushSync(() => pick.click());
|
|
|
|
const last = emitted.at(-1)!;
|
|
expect(last).toEqual({ STRIPE: { type: "secret_ref", secretId: "s1", version: "latest" } });
|
|
});
|
|
|
|
it("keeps the create form open when focus returns to the shared picker anchor", async () => {
|
|
vi.useFakeTimers();
|
|
try {
|
|
render(
|
|
<AgentSecretAccessEditor
|
|
config={{ "access.STRIPE": { type: "secret_ref", secretId: "s1" } }}
|
|
secrets={secrets}
|
|
onChange={() => {}}
|
|
onCreateSecret={async () => secrets[0]!}
|
|
/>,
|
|
);
|
|
|
|
const createButton = container.querySelector<HTMLButtonElement>('[data-testid="create-secret"]')!;
|
|
flushSync(() => createButton.click());
|
|
flushSync(() => vi.runAllTimers());
|
|
expect(document.body.textContent).toContain("Create secret");
|
|
|
|
const pickerButton = container.querySelector<HTMLButtonElement>('[data-testid="pick-secret"]')!;
|
|
pickerButton.focus();
|
|
flushSync(() => {});
|
|
|
|
expect(document.body.textContent).toContain("Create secret");
|
|
expect(document.querySelector('input[aria-label="Secret name"]')).toBeTruthy();
|
|
} finally {
|
|
vi.useRealTimers();
|
|
}
|
|
});
|
|
|
|
it("renders pending binding proposals as Proposed rows with approve/reject", () => {
|
|
const approved: string[] = [];
|
|
const rejected: string[] = [];
|
|
const proposal = {
|
|
id: "prop-1",
|
|
companyId: "co",
|
|
kind: "binding" as const,
|
|
status: "pending" as const,
|
|
justification: "Bind Stripe for the billing agent.",
|
|
proposedName: null,
|
|
proposedKey: null,
|
|
proposedDescription: null,
|
|
valueFingerprintSha256: null,
|
|
valueLength: null,
|
|
secretId: "s1",
|
|
secretName: "STRIPE_KEY",
|
|
secretProposalId: null,
|
|
secretProposalName: null,
|
|
targetType: "agent" as const,
|
|
target: { id: "agent-1", name: "BillingBot", icon: null },
|
|
configPath: "access.STRIPE",
|
|
proposedBy: { id: "agent-2", name: "ClaudeCoder", icon: null },
|
|
originIssue: null,
|
|
originRunId: "run-1",
|
|
expiresAt: new Date(Date.now() + 86_400_000).toISOString(),
|
|
createdAt: new Date(0).toISOString(),
|
|
resolvedByUserId: null,
|
|
resolvedAt: null,
|
|
resolutionReason: null,
|
|
createdSecretId: null,
|
|
appliedBindingConfigPath: null,
|
|
viewerCanApprove: true,
|
|
approveBlockReason: null,
|
|
};
|
|
render(
|
|
<AgentSecretAccessEditor
|
|
config={{}}
|
|
secrets={secrets}
|
|
onChange={() => {}}
|
|
proposals={[proposal]}
|
|
onApproveProposal={(p) => approved.push(p.id)}
|
|
onRejectProposal={(p) => rejected.push(p.id)}
|
|
/>,
|
|
);
|
|
|
|
expect(container.textContent).toContain("Proposed access");
|
|
expect(container.textContent).toContain("STRIPE"); // alias + bound secret name
|
|
expect(container.textContent).toContain("ClaudeCoder"); // proposer
|
|
|
|
const approveButton = [...container.querySelectorAll("button")].find(
|
|
(button) => button.textContent?.trim() === "Approve",
|
|
)!;
|
|
flushSync(() => approveButton.click());
|
|
expect(approved).toEqual(["prop-1"]);
|
|
|
|
const rejectButton = [...container.querySelectorAll("button")].find(
|
|
(button) => button.textContent?.trim() === "Reject",
|
|
)!;
|
|
flushSync(() => rejectButton.click());
|
|
expect(rejected).toEqual(["prop-1"]);
|
|
});
|
|
});
|