From 67616dd7ea725de7818c735af4cd8e807ec633de Mon Sep 17 00:00:00 2001 From: Dotta <34892728+cryppadotta@users.noreply.github.com> Date: Sat, 18 Jul 2026 14:39:06 -0500 Subject: [PATCH] Rework secrets dialog and add in-sheet agent access (#9797) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Thinking Path > - Paperclip is the open source control plane people use to manage AI agents and the credentials those agents need for work. > - The secrets UI is responsible for making credential creation understandable and for showing where credentials are used. > - The create flow previously exposed an editable generated key too early, used an unnatural field order, and rendered uneven provider tab rows. > - The detail sheet also lacked an in-context way to see and manage which agents reference a selected secret. > - This pull request makes the create dialog follow a predictable name-to-value flow and adds agent access management directly to the secret details sheet. > - The benefit is a clearer secrets workflow with fewer accidental key edits and less navigation when granting or revoking agent access. ## Linked Issues or Issue Description **Subsystem affected:** `ui/` — React + Vite board UI **Problem or motivation:** Creating a secret currently makes its generated key look immediately editable, places fields in an awkward keyboard order, and applies provider-tab overrides that produce uneven rows. After creation, operators cannot inspect or change agent access from the selected secret's detail sheet. **Proposed solution:** Generate the key from the path-style name and keep it read-only until an explicit Edit action; place Value directly after Name; use the standard tab sizing; and add an Agent access section that reads and updates `secret_ref` / `user_secret_ref` environment bindings through agent adapter configuration. **Alternatives considered:** Keeping access management only on agent configuration screens was rejected because it hides a secret-centric question—“which agents can use this?”—and requires repetitive navigation. Keeping the key always editable was rejected because the generated value should be the safe default. **Roadmap alignment:** No matching item was found in `ROADMAP.md`; this is a focused usability and access-management improvement to the existing secrets surface. **Additional context:** GitHub search found no duplicate PR for this dialog and in-sheet access change. PR #9321 also mentions user-secret resolution but addresses unrelated skills-route behavior. ## What Changed - Auto-generate the create-secret key from Name, keep it read-only by default, and expose an explicit Edit action. - Use a path-style Name placeholder (`/dev/foo/bar`) and place Value immediately after Name for natural keyboard navigation. - Remove tab sizing/whitespace overrides that caused uneven provider-tab row heights. - Add an Agent access section to the Details tab that lists referencing agents and grants or revokes `secret_ref` / `user_secret_ref` environment bindings in place. - Cover company secrets and each-user definitions with focused render tests. ## Verification - `pnpm exec vitest run ui/src/pages/Secrets.render.test.tsx` — 17/17 passed. - `pnpm --filter @paperclipai/ui typecheck` — passed. - `pnpm check:token-gates` — changed files are clean; the repository-wide command currently reports five pre-existing `#9627` violations in unrelated files (`Sidebar.tsx`, `Inbox.tsx`, `IssueDetail.tsx`, `Issues.tsx`, and `Routines.tsx`). - Additional secrets verification completed during implementation: 35/35 adjacent secrets tests passed, and the flow was checked in a real browser in light and dark themes. ## Risks - Agent access mutations update adapter environment configuration, so malformed legacy env entries could affect how a binding is displayed or revoked. - Each-user definitions use `user_secret_ref` rather than `secret_ref`; focused tests cover selecting the correct binding type. - No database or API contract changes are included. > 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 - Anthropic Claude Fable 5 assisted with the implementation using repository-aware code editing and test execution. - OpenAI GPT-5.5 via Codex CLI assisted with PR preparation, branch hygiene, focused verification, GitHub operations, and review/check loops. ## 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: Claude Fable 5 Co-authored-by: Paperclip --- ui/src/pages/Secrets.render.test.tsx | 208 +++++++++ ui/src/pages/Secrets.tsx | 625 ++++++++++++++++++++------- 2 files changed, 669 insertions(+), 164 deletions(-) diff --git a/ui/src/pages/Secrets.render.test.tsx b/ui/src/pages/Secrets.render.test.tsx index a983b62932..c42a0d6680 100644 --- a/ui/src/pages/Secrets.render.test.tsx +++ b/ui/src/pages/Secrets.render.test.tsx @@ -52,6 +52,12 @@ const mockSecretsApi = vi.hoisted(() => ({ removeMyUserSecret: vi.fn(), })); +const mockAgentsApi = vi.hoisted(() => ({ + list: vi.fn(), + get: vi.fn(), + update: vi.fn(), +})); + const mockSetBreadcrumbs = vi.hoisted(() => vi.fn()); const mockPushToast = vi.hoisted(() => vi.fn()); @@ -59,6 +65,10 @@ vi.mock("../api/secrets", () => ({ secretsApi: mockSecretsApi, })); +vi.mock("../api/agents", () => ({ + agentsApi: mockAgentsApi, +})); + vi.mock("../context/CompanyContext", () => ({ useCompany: () => ({ selectedCompanyId: "company-1", @@ -346,6 +356,7 @@ describe("Secrets page layout", () => { mockSecretsApi.listUserSecretDefinitions.mockResolvedValue([]); mockSecretsApi.userSecretDefinitionCoverage.mockResolvedValue(userSecretCoverage); mockSecretsApi.listMyUserSecrets.mockResolvedValue([]); + mockAgentsApi.list.mockResolvedValue([]); }); afterEach(() => { @@ -673,6 +684,18 @@ describe("Secrets page layout", () => { }); await flushReact(); + const companyKeyInput = document.getElementById("new-secret-key") as HTMLInputElement; + expect(companyKeyInput.readOnly).toBe(true); + + const editKeyButton = Array.from(document.body.querySelectorAll("button")).find( + (button) => button.textContent?.trim() === "Edit", + ) as HTMLButtonElement | undefined; + await act(async () => { + editKeyButton?.click(); + }); + await flushReact(); + expect(companyKeyInput.readOnly).toBe(false); + const eachUserButton = Array.from(document.body.querySelectorAll("button")).find( (button) => button.textContent?.trim() === "Each user", ) as HTMLButtonElement | undefined; @@ -686,6 +709,12 @@ describe("Secrets page layout", () => { const nameInput = document.getElementById("new-secret-name") as HTMLInputElement; const keyInput = document.getElementById("new-secret-key") as HTMLInputElement; const usageGuidance = document.getElementById("new-secret-usage-guidance") as HTMLTextAreaElement; + expect(keyInput.readOnly).toBe(true); + expect( + Array.from(document.body.querySelectorAll("button")).some( + (button) => button.textContent?.trim() === "Edit", + ), + ).toBe(true); expect(document.getElementById("new-secret-provider")).toBeNull(); expect(document.getElementById("new-secret-vault")).toBeNull(); expect(document.getElementById("new-secret-value")).toBeNull(); @@ -1191,6 +1220,185 @@ describe("Secrets page layout", () => { }); }); + it("auto-generates the key from the name and keeps it read-only until Edit", async () => { + const root = createRoot(container); + const queryClient = new QueryClient({ + defaultOptions: { queries: { retry: false } }, + }); + + await act(async () => { + root.render( + + + + + , + ); + }); + await flushReact(); + await flushReact(); + + const newSecretButton = Array.from(container.querySelectorAll("button")).find( + (button) => button.textContent?.includes("New secret"), + ) as HTMLButtonElement | undefined; + await act(async () => { + newSecretButton?.click(); + }); + await flushReact(); + + const nameInput = document.getElementById("new-secret-name") as HTMLInputElement; + const keyInput = document.getElementById("new-secret-key") as HTMLInputElement; + const valueTextarea = document.getElementById("new-secret-value") as HTMLTextAreaElement; + + // Path-style placeholder and value directly after name for natural tab order. + expect(nameInput.placeholder).toBe("/dev/foo/bar"); + expect( + nameInput.compareDocumentPosition(valueTextarea) & Node.DOCUMENT_POSITION_FOLLOWING, + ).toBeTruthy(); + expect( + valueTextarea.compareDocumentPosition(keyInput) & Node.DOCUMENT_POSITION_FOLLOWING, + ).toBeTruthy(); + + expect(keyInput.readOnly).toBe(true); + await act(async () => { + setInputValue(nameInput, "OpenAI API Key"); + }); + await flushReact(); + expect(keyInput.value).toBe("openai-api-key"); + + const editKeyButton = Array.from(document.body.querySelectorAll("button")).find( + (button) => button.textContent?.trim() === "Edit", + ) as HTMLButtonElement | undefined; + expect(editKeyButton).toBeDefined(); + await act(async () => { + editKeyButton?.click(); + }); + await flushReact(); + + expect((document.getElementById("new-secret-key") as HTMLInputElement).readOnly).toBe(false); + + await act(async () => { + setInputValue(document.getElementById("new-secret-key") as HTMLInputElement, "custom-key"); + setInputValue(nameInput, "OpenAI API Key v2"); + }); + await flushReact(); + + // Once edited, the key stops following the name. + expect((document.getElementById("new-secret-key") as HTMLInputElement).value).toBe("custom-key"); + + await act(async () => { + root.unmount(); + }); + }); + + it("grants and revokes agent access from the secret detail sheet", async () => { + mockSecretsApi.list.mockResolvedValue([makeCompanySecret()]); + mockSecretsApi.usage.mockResolvedValue({ secretId: "secret-openai", bindings: [] }); + mockSecretsApi.accessEvents.mockResolvedValue([]); + const coder = { + id: "agent-coder", + name: "CodexCoder", + status: "active", + adapterConfig: {}, + }; + const reviewer = { + id: "agent-reviewer", + name: "Reviewer", + status: "active", + adapterConfig: { + env: { OPENAI_API_KEY: { type: "secret_ref", secretId: "secret-openai" } }, + }, + }; + mockAgentsApi.list.mockResolvedValue([coder, reviewer]); + mockAgentsApi.get.mockImplementation(async (id: string) => + id === "agent-coder" ? coder : reviewer, + ); + mockAgentsApi.update.mockImplementation(async (id: string) => + id === "agent-coder" ? coder : reviewer, + ); + + const root = createRoot(container); + const queryClient = new QueryClient({ + defaultOptions: { queries: { retry: false } }, + }); + + await act(async () => { + root.render( + + + + + , + ); + }); + await flushReact(); + await flushReact(); + + const companyRow = Array.from(container.querySelectorAll("[role='row']")).find( + (row) => row.textContent?.includes("OPENAI_API_KEY"), + ) as HTMLElement | undefined; + await act(async () => { + companyRow?.click(); + }); + await flushReact(); + await flushReact(); + + // Existing access is listed right in the Details tab. + expect(document.body.textContent).toContain("Agent access"); + expect(document.body.textContent).toContain("Reviewer"); + + const agentSelect = document.getElementById("agent-access-agent") as HTMLSelectElement; + const envKeyInput = document.getElementById("agent-access-env-key") as HTMLInputElement; + expect(envKeyInput.value).toBe("OPENAI_API_KEY"); + // Agents that already have access are not offered again. + expect(Array.from(agentSelect.options).map((option) => option.textContent)).not.toContain("Reviewer"); + + await act(async () => { + setSelectValue(agentSelect, "agent-coder"); + }); + await flushReact(); + + const addButton = Array.from(document.body.querySelectorAll("button")).find( + (button) => button.textContent?.trim() === "Add", + ) as HTMLButtonElement | undefined; + await act(async () => { + addButton?.click(); + }); + await flushReact(); + await flushReact(); + + expect(mockAgentsApi.update).toHaveBeenCalledWith( + "agent-coder", + { + adapterConfig: { + env: { OPENAI_API_KEY: { type: "secret_ref", secretId: "secret-openai" } }, + }, + replaceAdapterConfig: true, + }, + "company-1", + ); + + const revokeButton = document.body.querySelector( + 'button[aria-label="Remove access for Reviewer"]', + ) as HTMLButtonElement | null; + expect(revokeButton).not.toBeNull(); + await act(async () => { + revokeButton?.click(); + }); + await flushReact(); + await flushReact(); + + expect(mockAgentsApi.update).toHaveBeenCalledWith( + "agent-reviewer", + { adapterConfig: { env: {} }, replaceAdapterConfig: true }, + "company-1", + ); + + await act(async () => { + root.unmount(); + }); + }); + it("shows an empty AWS discovery result without blocking manual entry", async () => { mockSecretsApi.providerConfigDiscoveryPreview.mockResolvedValueOnce( makeDiscoveryPreview({ candidates: [], sampledSecretCount: 0 }), diff --git a/ui/src/pages/Secrets.tsx b/ui/src/pages/Secrets.tsx index c3beceea05..7463862d57 100644 --- a/ui/src/pages/Secrets.tsx +++ b/ui/src/pages/Secrets.tsx @@ -58,6 +58,8 @@ import { } from "../api/secrets"; import { ApiError } from "../api/client"; import { accessApi, type CompanyUserDirectoryEntry } from "../api/access"; +import { agentsApi } from "../api/agents"; +import { envKeyFromSecretName } from "../components/environment-variables-editor/model"; import { queryKeys } from "../lib/queryKeys"; import { EmptyState } from "../components/EmptyState"; import { Button } from "@/components/ui/button"; @@ -615,6 +617,7 @@ export function Secrets() { const [createMode, setCreateMode] = useState("managed"); const [editingDefinition, setEditingDefinition] = useState(null); const [createKeyDirty, setCreateKeyDirty] = useState(false); + const [createKeyEditable, setCreateKeyEditable] = useState(false); const [createForm, setCreateForm] = useState({ name: "", key: "", @@ -712,6 +715,14 @@ export function Secrets() { () => userDefinitions.find((definition) => definition.id === selectedDefinitionId) ?? null, [selectedDefinitionId, userDefinitions], ); + const selectedSecretAccessReference = useMemo( + () => selectedSecret ? { kind: "company", secret: selectedSecret } : null, + [selectedSecret], + ); + const selectedDefinitionAccessReference = useMemo( + () => selectedDefinition ? { kind: "user", definition: selectedDefinition } : null, + [selectedDefinition], + ); const selectedDefinitionMyEntry = useMemo(() => { if (!selectedDefinition) return null; return myUserSecrets.find((entry) => entry.definition.id === selectedDefinition.id) ?? { @@ -845,6 +856,7 @@ export function Secrets() { setSecretValueProvider("company"); setCreateMode("managed"); setCreateKeyDirty(false); + setCreateKeyEditable(false); setCreateError(null); setCreateForm({ name: "", @@ -864,6 +876,7 @@ export function Secrets() { setSecretValueProvider("user"); setCreateMode("managed"); setCreateKeyDirty(true); + setCreateKeyEditable(false); setCreateError(null); setCreateForm({ name: definition.name, @@ -933,6 +946,7 @@ export function Secrets() { setEditingDefinition(null); setSecretValueProvider("company"); setCreateKeyDirty(false); + setCreateKeyEditable(false); setCreateForm({ name: "", key: "", @@ -1835,12 +1849,18 @@ export function Secrets() {
- setSecretDetailTab("usage")} - /> +
+ + setSecretDetailTab("usage")} + /> +
@@ -1968,11 +1988,17 @@ export function Secrets() {
- setSecretDetailTab("coverage")} - /> +
+ + setSecretDetailTab("coverage")} + /> +
-
-
-
- - { - const name = event.target.value; +
+ {!editingDefinition ? ( +
+

Who provides the value?

+ { + const next = value as SecretValueProvider; + setSecretValueProvider(next); + setCreateKeyEditable(false); setCreateForm((current) => ({ ...current, - name, key: createKeyDirty ? current.key - : secretValueProvider === "user" - ? normalizeUserSecretKeyForPreview(name) - : normalizeSecretKeyForPreview(name), + : next === "user" + ? normalizeUserSecretKeyForPreview(current.name) + : normalizeSecretKeyForPreview(current.name), })); }} - placeholder={secretValueProvider === "user" ? "Personal GitHub token" : "OPENAI_API_KEY"} - autoFocus - /> -
-
- - { - setCreateKeyDirty(true); - setCreateForm((current) => ({ ...current, key: event.target.value })); - }} - placeholder={secretValueProvider === "user" ? "PERSONAL_GH_TOKEN" : "auto from name"} - disabled={Boolean(editingDefinition)} - className={secretValueProvider === "user" ? "font-mono text-sm" : undefined} - /> -

- {secretValueProvider === "user" - ? editingDefinition - ? "Stable env binding key. Cannot be changed." - : "Env-style key used by user-secret bindings." - : "Shared secret keys keep lowercase dash normalization."} + > + + Company + Each user + + +

+ Company stores one shared value. Each user lets every member supply their own value under My secrets.

+ ) : null} + + {secretValueProvider === "company" && !editingDefinition ? ( + setCreateMode(value as CreateMode)}> + + Managed value + External reference + + + ) : null} + +
+ + { + const name = event.target.value; + setCreateForm((current) => ({ + ...current, + name, + key: createKeyDirty + ? current.key + : secretValueProvider === "user" + ? normalizeUserSecretKeyForPreview(name) + : normalizeSecretKeyForPreview(name), + })); + }} + placeholder={secretValueProvider === "user" ? "Personal GitHub token" : "/dev/foo/bar"} + autoFocus + />
+ + {secretValueProvider === "company" && createMode === "managed" ? ( +
+ +