diff --git a/ui/src/components/AgentActionButtons.test.tsx b/ui/src/components/AgentActionButtons.test.tsx index 507530bdbd..901717f9b7 100644 --- a/ui/src/components/AgentActionButtons.test.tsx +++ b/ui/src/components/AgentActionButtons.test.tsx @@ -126,7 +126,7 @@ describe("AgentActionButtons", () => { }); function render(agent: Agent, props: Partial> = {}) { - root = createRoot(container); + root ??= createRoot(container); root.render( @@ -205,4 +205,64 @@ describe("AgentActionButtons", () => { 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('[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('[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(); + }); }); diff --git a/ui/src/components/AgentActionButtons.tsx b/ui/src/components/AgentActionButtons.tsx index 2460e4e8eb..a062a71af2 100644 --- a/ui/src/components/AgentActionButtons.tsx +++ b/ui/src/components/AgentActionButtons.tsx @@ -1,4 +1,4 @@ -import { useCallback, useState, type ReactNode } from "react"; +import { useCallback, useRef, useState, type ReactNode } from "react"; import { useNavigate } from "@/lib/router"; import { useMutation, useQueryClient } from "@tanstack/react-query"; import { @@ -165,6 +165,8 @@ export function AgentActionButtons({ workActionsDisabled = false, workActionsDisabledReason, navigateToRunOnInvoke = true, + hasPendingNavigationChanges = false, + onBeforeNavigate, onActionError, onTerminateSuccess, pauseConfirm, @@ -182,6 +184,10 @@ export function AgentActionButtons({ workActionsDisabled?: boolean; workActionsDisabledReason?: string; navigateToRunOnInvoke?: boolean; + /** Whether the caller currently has an unsaved draft that navigation would discard. */ + hasPendingNavigationChanges?: boolean; + /** Return false to stop an action whose success would navigate away. */ + onBeforeNavigate?: () => boolean; /** * When set, pausing prompts a confirmation dialog first (e.g. for built-in * agents that power a feature). Omit for the immediate-pause default. @@ -207,6 +213,25 @@ export function AgentActionButtons({ const { pushToast } = useToastActions(); const [moreOpen, setMoreOpen] = useState(false); const [pauseConfirmOpen, setPauseConfirmOpen] = useState(false); + const pendingNavigationChangesRef = useRef(hasPendingNavigationChanges); + const beforeNavigateRef = useRef(onBeforeNavigate); + const agentActionStartedDirtyRef = useRef(false); + const duplicateStartedDirtyRef = useRef(false); + pendingNavigationChangesRef.current = hasPendingNavigationChanges; + beforeNavigateRef.current = onBeforeNavigate; + + function confirmNavigationStart(startedDirtyRef: React.MutableRefObject) { + startedDirtyRef.current = pendingNavigationChangesRef.current; + return beforeNavigateRef.current?.() !== false; + } + + function confirmLateNavigationChanges(startedDirtyRef: React.MutableRefObject) { + return ( + !pendingNavigationChangesRef.current || + startedDirtyRef.current || + beforeNavigateRef.current?.() !== false + ); + } const resolvedCompanyId = companyId ?? agent.companyId; const canonicalAgentRef = agentRouteRef(agent); @@ -251,9 +276,11 @@ export function AgentActionButtons({ onActionError?.(null); invalidateAgent(); if (action === "terminate") { + if (!confirmLateNavigationChanges(agentActionStartedDirtyRef)) return; onTerminateSuccess?.(data as Agent); } if (action === "invoke" && navigateToRunOnInvoke && data && typeof data === "object" && "id" in data) { + if (!confirmLateNavigationChanges(agentActionStartedDirtyRef)) return; navigate(`/agents/${canonicalAgentRef}/runs/${(data as HeartbeatRun).id}`); } }, @@ -285,6 +312,7 @@ export function AgentActionButtons({ await queryClient.invalidateQueries({ queryKey: queryKeys.agents.list(resolvedCompanyId) }); } pushToast({ title: "Agent duplicated", body: createdAgent.name, tone: "success" }); + if (!confirmLateNavigationChanges(duplicateStartedDirtyRef)) return; navigate(`/agents/${agentRouteRef(createdAgent)}/dashboard`); }, onError: (err) => { @@ -299,7 +327,7 @@ export function AgentActionButtons({ const nextName = duplicateAgentName(agent.name); const confirmed = window.confirm(`Duplicate ${agent.name} as ${nextName}?`); setMoreOpen(false); - if (!confirmed) return; + if (!confirmed || !confirmNavigationStart(duplicateStartedDirtyRef)) return; duplicateAgent.mutate(); }, [agent.name, duplicateAgent]); @@ -334,7 +362,10 @@ export function AgentActionButtons({ {assignLabel} agentAction.mutate("invoke")} + onClick={() => { + if (navigateToRunOnInvoke && !confirmNavigationStart(agentActionStartedDirtyRef)) return; + agentAction.mutate("invoke"); + }} disabled={assignAndRunDisabled} label={runLabel} size={size} @@ -423,8 +454,9 @@ export function AgentActionButtons({ + + + )} + +
+ {cards + ?

Secret access

+ :
Secret access
+ } +
+

{help.secretAccess}

+ createSecret.mutateAsync({ name, value })} + proposals={agentBindingProposals} + onApproveProposal={proposalReview.requestApprove} + onRejectProposal={proposalReview.requestReject} + /> + {proposalReview.dialogs} +
+
+ + ); + } + return (
{/* ---- Floating Save button (edit mode, when dirty) ---- */} @@ -1538,20 +1577,6 @@ export function AgentConfigForm(props: AgentConfigFormProps) { /> - {!isCreate && ( - - - {proposalReview.dialogs} - - )} - {/* Edit-only: timeout + grace period */} {!isCreate && ( <> diff --git a/ui/src/components/AgentSecretAccessEditor.test.tsx b/ui/src/components/AgentSecretAccessEditor.test.tsx index bc1df77a46..9078713752 100644 --- a/ui/src/components/AgentSecretAccessEditor.test.tsx +++ b/ui/src/components/AgentSecretAccessEditor.test.tsx @@ -5,18 +5,29 @@ import { createRoot, type Root } from "react-dom/client"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import type { CompanySecret, EnvSecretRefBinding } from "@paperclipai/shared"; -// Stub SecretBindingPicker so the editor renders without CompanyContext / -// react-query. The stub exposes a button that binds a fixed secret. -vi.mock("./SecretBindingPicker", () => ({ - SecretBindingPicker: ({ - onChange, - }: { - onChange: (next: { secretId: string; version?: number | "latest" } | null) => void; - }) => ( - - ), +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 <> + + {props.onCreateNew ? ( + + ) : null} + ; + }, })); import { @@ -155,12 +166,43 @@ describe("AgentSecretAccessEditor component", () => { // Bind a secret via the stubbed picker. const pick = container.querySelector('[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( + {}} + onCreateSecret={async () => secrets[0]!} + />, + ); + + const createButton = container.querySelector('[data-testid="create-secret"]')!; + flushSync(() => createButton.click()); + flushSync(() => vi.runAllTimers()); + expect(document.body.textContent).toContain("Create secret"); + + const pickerButton = container.querySelector('[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[] = []; diff --git a/ui/src/components/AgentSecretAccessEditor.tsx b/ui/src/components/AgentSecretAccessEditor.tsx index 1152964836..272e02568f 100644 --- a/ui/src/components/AgentSecretAccessEditor.tsx +++ b/ui/src/components/AgentSecretAccessEditor.tsx @@ -9,14 +9,16 @@ import type { import { cn } from "../lib/utils"; import { Input } from "@/components/ui/input"; import { Badge } from "@/components/ui/badge"; -import { SecretBindingPicker, type SecretBindingValue } from "./SecretBindingPicker"; +import { SecretPicker } from "./environment-variables-editor/SecretPicker"; +import { CreateSecretPopover } from "./environment-variables-editor/CreateSecretPopover"; +import { Popover, PopoverAnchor, PopoverContent } from "@/components/ui/popover"; import { AGENT_ACCESS_CONFIG_PATH_PREFIX, ENV_CONFIG_PATH_PREFIX, SECRET_ALIAS_RE, deliveryModeDescription, } from "../lib/secret-delivery"; -import { envKeyFromSecretName } from "./environment-variables-editor/model"; +import { envKeyFromSecretName, secretNameFromKey } from "./environment-variables-editor/model"; import { DeliveryBadge as ProposalDeliveryBadge, ProposalActions, @@ -157,6 +159,8 @@ export interface AgentSecretAccessEditorProps { * parent diffs this against the current `access.*` keys to add/remove them. */ onChange: (next: Record) => void; + /** Create a company secret from the shared picker's pinned create action. */ + onCreateSecret?: (name: string, value: string) => Promise; disabled?: boolean; /** Pending binding proposals targeting this agent (PAP-14731). */ proposals?: readonly SecretProposalView[]; @@ -191,6 +195,7 @@ export function AgentSecretAccessEditor({ config, secrets, onChange, + onCreateSecret, disabled, proposals, onApproveProposal, @@ -208,6 +213,8 @@ export function AgentSecretAccessEditor({ const incomingKey = useMemo(() => normalizeAccessMapKey(incomingMap), [incomingMap]); const [rows, setRows] = useState(() => entriesToRows(apiBindings)); + const [createRequest, setCreateRequest] = useState<{ rowId: string; name: string } | null>(null); + const pickerAnchorRefs = useRef(new Map()); const lastEmittedKeyRef = useRef(incomingKey); const lastIncomingKeyRef = useRef(incomingKey); @@ -334,9 +341,7 @@ export function AgentSecretAccessEditor({ const trimmedAlias = row.alias.trim(); const aliasInvalid = Boolean(trimmedAlias) && !SECRET_ALIAS_RE.test(trimmedAlias); const aliasDuplicate = Boolean(trimmedAlias) && (aliasCounts.get(trimmedAlias) ?? 0) > 1; - const bindingValue: SecretBindingValue | null = row.secretId - ? { secretId: row.secretId, version: row.version } - : null; + const selectedSecret = secrets.find((secret) => secret.id === row.secretId) ?? null; return (
@@ -360,23 +365,106 @@ export function AgentSecretAccessEditor({ )} />
-
- +
+ { + if (!open && createRequest?.rowId === row.id) setCreateRequest(null); + }} + > + +
{ + if (node) pickerAnchorRefs.current.set(row.id, node); + else pickerAnchorRefs.current.delete(row.id); + }} + className="min-w-0 flex-1" + > + + patchRow(row.id, { + secretId, + version: "latest", + alias: + !row.alias.trim() && secretId + ? envKeyFromSecretName(secretName(secretId)) + : row.alias, + }) + } + onCreateNew={onCreateSecret + ? (query) => { + window.setTimeout(() => { + setCreateRequest({ + rowId: row.id, + name: secretNameFromKey(query) || query.trim(), + }); + }, 0); + } + : undefined} + disabled={disabled} + triggerClassName="h-9 min-h-9" + /> +
+
+ { + // Closing the picker returns focus to its trigger inside + // this popover's anchor. Keep that focus restoration from + // dismissing the create form that just opened. + const target = event.detail.originalEvent.target as Node | null; + if (target && pickerAnchorRefs.current.get(row.id)?.contains(target)) { + event.preventDefault(); + } + }} + > + {createRequest?.rowId === row.id && onCreateSecret ? ( + secret.name)} + onCancel={() => setCreateRequest(null)} + onSubmit={async (name, value) => { + const created = await onCreateSecret(name, value); + patchRow(row.id, { + secretId: created.id, + version: "latest", + alias: row.alias.trim() || envKeyFromSecretName(created.name), + }); + setCreateRequest(null); + }} + /> + ) : null} + +
+
+
+
+ +
+
+