From c958b68191b9019676f3ce98f68300441635e671 Mon Sep 17 00:00:00 2001 From: arc53-machine <232052973+arc53-machine@users.noreply.github.com> Date: Tue, 29 Sep 2026 15:11:07 +0100 Subject: [PATCH] Save the API write allowlist on the saved agent config A toggle sent the form's draft config, so unsaved guardrail edits were saved with it, and the form then read as changed because the saved snapshot kept the old config. The allowlist now saves on top of the last saved config and updates that snapshot. --- .../src/agents/ApiWriteAllowlist.test.tsx | 33 +++++++++++++++++-- frontend/src/agents/ApiWriteAllowlist.tsx | 8 ++++- frontend/src/agents/NewAgent.test.tsx | 24 +++++++++++++- frontend/src/agents/NewAgent.tsx | 15 ++++++++- frontend/src/modals/AgentDetailsModal.tsx | 6 +++- 5 files changed, 80 insertions(+), 6 deletions(-) diff --git a/frontend/src/agents/ApiWriteAllowlist.test.tsx b/frontend/src/agents/ApiWriteAllowlist.test.tsx index b7458e23..3a3c78c5 100644 --- a/frontend/src/agents/ApiWriteAllowlist.test.tsx +++ b/frontend/src/agents/ApiWriteAllowlist.test.tsx @@ -81,12 +81,20 @@ describe('ApiWriteAllowlist', () => { container.remove(); }); - const render = async (a: Agent, onConfigChange = vi.fn()) => { + const render = async ( + a: Agent, + onConfigChange = vi.fn(), + getSavedConfig?: () => Agent['config'], + ) => { store = makeStore(); await act(async () => { root.render( - + , ); }); @@ -125,6 +133,27 @@ describe('ApiWriteAllowlist', () => { }); }); + it('saves on top of the last saved config, not unsaved form edits', async () => { + updateAgent.mockResolvedValue({ ok: true }); + const saved = { guardrails: { controls: [] } }; + const draft = agent({ + config: { guardrails: { controls: [{ id: 'unsaved' }] } }, + } as unknown as Partial); + const onConfigChange = await render(draft, vi.fn(), () => saved); + await act(async () => + container + .querySelector('button[role="checkbox"]')! + .click(), + ); + const expected = { + guardrails: { controls: [] }, + api_write_allowlist: ['tg:telegram_send_message'], + }; + const form = updateAgent.mock.calls[0][1] as FormData; + expect(JSON.parse(form.get('config') as string)).toEqual(expected); + expect(onConfigChange).toHaveBeenCalledWith(expected); + }); + it('puts the choice back and says so when saving fails', async () => { updateAgent.mockResolvedValue({ ok: false }); await render(agent()); diff --git a/frontend/src/agents/ApiWriteAllowlist.tsx b/frontend/src/agents/ApiWriteAllowlist.tsx index d08d0ff0..a467f83f 100644 --- a/frontend/src/agents/ApiWriteAllowlist.tsx +++ b/frontend/src/agents/ApiWriteAllowlist.tsx @@ -36,13 +36,19 @@ type UserTool = { * The write actions on the owner's connected accounts that anyone calling * this agent with its API key may run. Nobody can approve an action in the * widget or the API, so the server refuses every other such write. + * + * A toggle saves at once, on top of the agent's last saved config + * (`getSavedConfig`), so edits still pending in the form are not saved with + * it. `onConfigChange` receives the config as saved. */ export default function ApiWriteAllowlist({ agent, onConfigChange, + getSavedConfig, }: { agent: Agent; onConfigChange?: (config: AgentConfig) => void; + getSavedConfig?: () => AgentConfig | undefined; }) { const { t } = useTranslation(); const dispatch = useDispatch(); @@ -101,7 +107,7 @@ export default function ApiWriteAllowlist({ : allowed.filter((item) => item !== entry); setAllowed(next); const config: AgentConfig = { - ...(agent.config ?? {}), + ...((getSavedConfig ? getSavedConfig() : agent.config) ?? {}), api_write_allowlist: next, }; const form = new FormData(); diff --git a/frontend/src/agents/NewAgent.test.tsx b/frontend/src/agents/NewAgent.test.tsx index 2c7aec76..74301b4c 100644 --- a/frontend/src/agents/NewAgent.test.tsx +++ b/frontend/src/agents/NewAgent.test.tsx @@ -35,6 +35,7 @@ const mocks = vi.hoisted(() => { catalog: [] as unknown[], deleteAgent: vi.fn(() => jsonResponse({})), guardrailsProps: vi.fn(), + detailsProps: vi.fn(), }; }); const { jsonResponse } = mocks; @@ -126,7 +127,12 @@ vi.mock('./components/GuardrailsSection', () => ({ guardrailsIncomplete: () => false, })); vi.mock('../upload/Upload', () => ({ default: () => null })); -vi.mock('../modals/AgentDetailsModal', () => ({ default: () => null })); +vi.mock('../modals/AgentDetailsModal', () => ({ + default: (props: unknown) => { + mocks.detailsProps(props); + return null; + }, +})); vi.mock('../teams/ShareToTeamModal', () => ({ default: () => null })); vi.mock('../modals/ConfirmationModal', () => ({ default: ({ @@ -793,6 +799,22 @@ describe('NewAgent gating by role', () => { expect(token.disabled).toBe(true); }); + it('keeps the form clean after the API write allowlist saves', async () => { + await renderEdit('owner', OWNER); + const details = () => + mocks.detailsProps.mock.calls.at(-1)![0] as { + onConfigChange: (config: Record) => void; + getSavedConfig: () => Record | undefined; + }; + const saved = { + ...details().getSavedConfig(), + api_write_allowlist: ['tool-1:send'], + }; + await act(async () => details().onConfigChange(saved)); + expect(details().getSavedConfig()).toEqual(saved); + expect(buttonByText('agents.form.buttons.cancel')).toBeUndefined(); + }); + it('hides Access details without manage_access_details', async () => { await renderEdit( 'editor', diff --git a/frontend/src/agents/NewAgent.tsx b/frontend/src/agents/NewAgent.tsx index c2e7977d..cfa89e10 100644 --- a/frontend/src/agents/NewAgent.tsx +++ b/frontend/src/agents/NewAgent.tsx @@ -1656,7 +1656,20 @@ export default function NewAgent({ mode }: { mode: 'new' | 'edit' | 'draft' }) { modalState={agentDetails} setModalState={setAgentDetails} onKeyRegenerated={(key) => setAgent((prev) => ({ ...prev, key }))} - onConfigChange={(config) => setAgent((prev) => ({ ...prev, config }))} + onConfigChange={(config) => { + // The allowlist is saved already: record it on the saved snapshot + // too, and keep any unsaved form edits to the rest of the config. + if (initialAgentRef.current) + initialAgentRef.current = { ...initialAgentRef.current, config }; + setAgent((prev) => ({ + ...prev, + config: { + ...(prev.config ?? {}), + api_write_allowlist: config.api_write_allowlist, + }, + })); + }} + getSavedConfig={() => initialAgentRef.current?.config ?? agent.config} /> {shareModalOpen && agent.id && ( void; onKeyRegenerated?: (key: string) => void; - /** The agent's config after its API-key write allowlist changed. */ + /** The agent's saved config after its API-key write allowlist changed. */ onConfigChange?: (config: AgentConfig) => void; + /** The agent's last saved config, which an allowlist change saves onto. */ + getSavedConfig?: () => AgentConfig | undefined; }; export default function AgentDetailsModal({ @@ -48,6 +50,7 @@ export default function AgentDetailsModal({ setModalState, onKeyRegenerated, onConfigChange, + getSavedConfig, }: AgentDetailsModalProps) { const { t } = useTranslation(); const token = useSelector(selectToken); @@ -278,6 +281,7 @@ export default function AgentDetailsModal({ )}