mirror of
https://github.com/tiennm99/DocsGPT.git
synced 2026-10-13 12:14:00 +00:00
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.
This commit is contained in:
1 parent
26bd4e2a3f
commit
c958b68191
5 files changed
+80
-6
No files matched your search
@@ -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(
|
||||
<Provider store={store}>
|
||||
<ApiWriteAllowlist agent={a} onConfigChange={onConfigChange} />
|
||||
<ApiWriteAllowlist
|
||||
agent={a}
|
||||
onConfigChange={onConfigChange}
|
||||
getSavedConfig={getSavedConfig}
|
||||
/>
|
||||
</Provider>,
|
||||
);
|
||||
});
|
||||
@@ -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<Agent>);
|
||||
const onConfigChange = await render(draft, vi.fn(), () => saved);
|
||||
await act(async () =>
|
||||
container
|
||||
.querySelector<HTMLButtonElement>('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());
|
||||
|
||||
@@ -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();
|
||||
|
||||
@@ -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<string, unknown>) => void;
|
||||
getSavedConfig: () => Record<string, unknown> | 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',
|
||||
|
||||
@@ -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 && (
|
||||
<ShareToTeamModal
|
||||
|
||||
@@ -37,8 +37,10 @@ type AgentDetailsModalProps = {
|
||||
modalState: ActiveState;
|
||||
setModalState: (state: ActiveState) => 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({
|
||||
<ApiWriteAllowlist
|
||||
agent={agent}
|
||||
onConfigChange={onConfigChange}
|
||||
getSavedConfig={getSavedConfig}
|
||||
/>
|
||||
)}
|
||||
</div>
|
||||
|
||||
Reference in new issue
Block a user