mirror of
https://github.com/danny-avila/LibreChat.git
synced 2026-08-04 14:57:42 +00:00
fix: Scope agent drafts by user
This commit is contained in:
parent
0156310b26
commit
018edee845
6 changed files with 145 additions and 73 deletions
|
|
@ -10,6 +10,7 @@ import type { AgentForm } from '~/common';
|
|||
|
||||
// Mock toast context - define this after all mocks
|
||||
let mockShowToast: jest.Mock;
|
||||
const MOCK_USER_ID = 'user-123';
|
||||
|
||||
// Mock notification severity enum before other imports
|
||||
jest.mock('~/common/types', () => ({
|
||||
|
|
@ -99,7 +100,7 @@ jest.mock('~/utils', () => ({
|
|||
jest.mock('~/hooks', () => ({
|
||||
useSelectAgent: () => ({ onSelect: jest.fn() }),
|
||||
useLocalize: () => (key: string) => key,
|
||||
useAuthContext: () => ({ user: { id: 'user-123', role: 'USER' } }),
|
||||
useAuthContext: () => ({ user: { id: MOCK_USER_ID, role: 'USER' } }),
|
||||
}));
|
||||
|
||||
jest.mock('~/hooks/useResourcePermissions', () => ({
|
||||
|
|
@ -414,26 +415,30 @@ describe('AgentPanel - Update Agent Toast Messages', () => {
|
|||
});
|
||||
mockUpdateAgent.mockResolvedValue(createMockAgent({ name: 'Test Agent', version: 2 }));
|
||||
|
||||
saveAgentDraft('agent-123', {
|
||||
id: 'agent-123',
|
||||
name: 'Unsaved Agent',
|
||||
description: '',
|
||||
instructions: 'Unsaved instructions',
|
||||
model: 'gpt-4',
|
||||
model_parameters: {},
|
||||
provider: 'openai',
|
||||
category: 'general',
|
||||
execute_code: false,
|
||||
file_search: false,
|
||||
web_search: false,
|
||||
} as AgentForm);
|
||||
saveAgentDraft(
|
||||
'agent-123',
|
||||
{
|
||||
id: 'agent-123',
|
||||
name: 'Unsaved Agent',
|
||||
description: '',
|
||||
instructions: 'Unsaved instructions',
|
||||
model: 'gpt-4',
|
||||
model_parameters: {},
|
||||
provider: 'openai',
|
||||
category: 'general',
|
||||
execute_code: false,
|
||||
file_search: false,
|
||||
web_search: false,
|
||||
} as AgentForm,
|
||||
MOCK_USER_ID,
|
||||
);
|
||||
|
||||
expect(getAgentDraft('agent-123')).toBeDefined();
|
||||
expect(getAgentDraft('agent-123', MOCK_USER_ID)).toBeDefined();
|
||||
|
||||
await renderAndSubmitForm();
|
||||
|
||||
await waitFor(() => {
|
||||
expect(getAgentDraft('agent-123')).toBeUndefined();
|
||||
expect(getAgentDraft('agent-123', MOCK_USER_ID)).toBeUndefined();
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -266,7 +266,11 @@ export default function AgentPanel() {
|
|||
const agentQuery = canEdit && expandedAgentQuery.data ? expandedAgentQuery : basicAgentQuery;
|
||||
|
||||
const models = useMemo(() => modelsQuery.data ?? {}, [modelsQuery.data]);
|
||||
const draftValues = useMemo(() => getAgentDraft(current_agent_id), [current_agent_id]);
|
||||
const draftUserId = user?.id;
|
||||
const draftValues = useMemo(
|
||||
() => getAgentDraft(current_agent_id, draftUserId),
|
||||
[current_agent_id, draftUserId],
|
||||
);
|
||||
const defaultValues = useMemo(() => getDraftFormValues(draftValues), [draftValues]);
|
||||
const methods = useForm<AgentForm>({
|
||||
defaultValues,
|
||||
|
|
@ -284,12 +288,13 @@ export default function AgentPanel() {
|
|||
formState: { dirtyFields },
|
||||
} = methods;
|
||||
const currentAgentIdRef = useRef<string | undefined>(current_agent_id);
|
||||
const draftUserIdRef = useRef<string | undefined>(draftUserId);
|
||||
const [hasDraft, setHasDraft] = useState(draftValues != null);
|
||||
const shouldPersistDraftRef = useRef(hasDraft);
|
||||
const [isAvatarUploadInFlight, setIsAvatarUploadInFlight] = useState(false);
|
||||
|
||||
const persistAgentDraft = useCallback((agentId: string | undefined, values: AgentForm) => {
|
||||
saveAgentDraft(agentId, values);
|
||||
saveAgentDraft(agentId, values, draftUserIdRef.current);
|
||||
if (agentId === currentAgentIdRef.current) {
|
||||
setHasDraft(true);
|
||||
}
|
||||
|
|
@ -297,8 +302,8 @@ export default function AgentPanel() {
|
|||
|
||||
const clearDraftsForAgentIds = useCallback((agentIds: Array<string | null | undefined>) => {
|
||||
shouldPersistDraftRef.current = false;
|
||||
clearAgentDrafts(agentIds);
|
||||
setHasDraft(getAgentDraft(currentAgentIdRef.current) != null);
|
||||
clearAgentDrafts(agentIds, draftUserIdRef.current);
|
||||
setHasDraft(getAgentDraft(currentAgentIdRef.current, draftUserIdRef.current) != null);
|
||||
}, []);
|
||||
|
||||
const handleAgentChange = useCallback(
|
||||
|
|
@ -328,23 +333,32 @@ export default function AgentPanel() {
|
|||
);
|
||||
|
||||
useEffect(() => {
|
||||
if (currentAgentIdRef.current === current_agent_id) {
|
||||
const agentChanged = currentAgentIdRef.current !== current_agent_id;
|
||||
const userChanged = draftUserIdRef.current !== draftUserId;
|
||||
|
||||
if (!agentChanged && !userChanged) {
|
||||
return;
|
||||
}
|
||||
|
||||
if (shouldPersistDraftRef.current) {
|
||||
saveAgentDraft(currentAgentIdRef.current, getValues());
|
||||
if (agentChanged && shouldPersistDraftRef.current) {
|
||||
saveAgentDraft(currentAgentIdRef.current, getValues(), draftUserIdRef.current);
|
||||
}
|
||||
|
||||
currentAgentIdRef.current = current_agent_id;
|
||||
const nextDraft = getAgentDraft(current_agent_id);
|
||||
draftUserIdRef.current = draftUserId;
|
||||
const nextDraft = getAgentDraft(current_agent_id, draftUserId);
|
||||
const nextHasDraft = nextDraft != null;
|
||||
shouldPersistDraftRef.current = nextHasDraft;
|
||||
setHasDraft(nextHasDraft);
|
||||
if (nextDraft) {
|
||||
reset(getDraftFormValues(nextDraft));
|
||||
return;
|
||||
}
|
||||
}, [current_agent_id, getValues, reset]);
|
||||
|
||||
if (userChanged) {
|
||||
reset(getDefaultAgentFormValues());
|
||||
}
|
||||
}, [current_agent_id, draftUserId, getValues, reset]);
|
||||
|
||||
useEffect(() => {
|
||||
if (hasDraft) {
|
||||
|
|
@ -373,7 +387,7 @@ export default function AgentPanel() {
|
|||
|
||||
return () => {
|
||||
if (shouldPersistDraftRef.current) {
|
||||
saveAgentDraft(currentAgentIdRef.current, getValues());
|
||||
saveAgentDraft(currentAgentIdRef.current, getValues(), draftUserIdRef.current);
|
||||
}
|
||||
subscription.unsubscribe();
|
||||
};
|
||||
|
|
|
|||
|
|
@ -15,7 +15,7 @@ import type { UseMutationResult } from '@tanstack/react-query';
|
|||
import { logger, getDefaultAgentFormValues } from '~/utils';
|
||||
import { useDeleteAgentMutation } from '~/data-provider';
|
||||
import { isEphemeralAgent } from '~/common';
|
||||
import { useLocalize } from '~/hooks';
|
||||
import { useAuthContext, useLocalize } from '~/hooks';
|
||||
import store from '~/store';
|
||||
import { clearAgentDraft } from './drafts';
|
||||
|
||||
|
|
@ -29,6 +29,7 @@ function DeleteButton({
|
|||
createMutation: UseMutationResult<Agent, Error, AgentCreateParams>;
|
||||
}) {
|
||||
const localize = useLocalize();
|
||||
const { user } = useAuthContext();
|
||||
const { reset } = useFormContext();
|
||||
const { showToast } = useToastContext();
|
||||
const setConversation = useSetRecoilState(store.conversationByIndex(0));
|
||||
|
|
@ -45,7 +46,7 @@ function DeleteButton({
|
|||
message: localize('com_ui_agent_deleted'),
|
||||
status: 'success',
|
||||
});
|
||||
clearAgentDraft(vars.agent_id);
|
||||
clearAgentDraft(vars.agent_id, user?.id);
|
||||
|
||||
if (createMutation.data?.id ?? '') {
|
||||
logger.log('agents', 'resetting createMutation');
|
||||
|
|
|
|||
|
|
@ -12,6 +12,7 @@ import { getAgentDraft, saveAgentDraft, clearAllAgentDrafts } from '../drafts';
|
|||
|
||||
let mockCurrentAgentId: string | undefined;
|
||||
let mockLastAgentSelectHasDraft: boolean | undefined;
|
||||
let mockUserId: string | undefined;
|
||||
|
||||
const AGENT_SELECT_LABEL = 'Agent Select';
|
||||
const ADVANCED_PANEL_LABEL = 'Advanced Panel';
|
||||
|
|
@ -22,6 +23,7 @@ const AGENTS_BUTTON_LABEL = 'Agents';
|
|||
const FILES_BUTTON_LABEL = 'Files';
|
||||
const FILES_PANEL_LABEL = 'Files panel';
|
||||
const PROGRAMMATIC_UPDATE_LABEL = 'Programmatic agent update';
|
||||
const MOCK_USER_ID = 'user-123';
|
||||
|
||||
type MockButtonProps = React.ButtonHTMLAttributes<HTMLButtonElement> & {
|
||||
size?: string;
|
||||
|
|
@ -78,7 +80,7 @@ jest.mock('~/data-provider', () => ({
|
|||
jest.mock('~/hooks', () => ({
|
||||
useSelectAgent: () => ({ onSelect: jest.fn() }),
|
||||
useLocalize: () => (key: string) => key,
|
||||
useAuthContext: () => ({ user: { id: 'user-123', role: 'USER' } }),
|
||||
useAuthContext: () => ({ user: { id: mockUserId, role: 'USER' } }),
|
||||
}));
|
||||
|
||||
jest.mock('~/hooks/useResourcePermissions', () => ({
|
||||
|
|
@ -241,6 +243,7 @@ describe('AgentPanel draft preservation', () => {
|
|||
localStorage.setItem('side:active-panel', 'agents');
|
||||
mockCurrentAgentId = undefined;
|
||||
mockLastAgentSelectHasDraft = undefined;
|
||||
mockUserId = MOCK_USER_ID;
|
||||
clearAllAgentDrafts();
|
||||
});
|
||||
|
||||
|
|
@ -269,21 +272,25 @@ describe('AgentPanel draft preservation', () => {
|
|||
|
||||
it('clears an existing draft when Create new agent is clicked', () => {
|
||||
mockCurrentAgentId = 'agent-123';
|
||||
saveAgentDraft('agent-123', {
|
||||
id: 'agent-123',
|
||||
name: 'Unsaved saved-agent name',
|
||||
description: '',
|
||||
instructions: 'Unsaved saved-agent instructions',
|
||||
model: 'gpt-4o',
|
||||
model_parameters: {},
|
||||
provider: { label: 'OpenAI', value: 'openAI' },
|
||||
tools: [],
|
||||
tool_options: {},
|
||||
category: 'general',
|
||||
execute_code: false,
|
||||
file_search: false,
|
||||
web_search: false,
|
||||
} as AgentForm);
|
||||
saveAgentDraft(
|
||||
'agent-123',
|
||||
{
|
||||
id: 'agent-123',
|
||||
name: 'Unsaved saved-agent name',
|
||||
description: '',
|
||||
instructions: 'Unsaved saved-agent instructions',
|
||||
model: 'gpt-4o',
|
||||
model_parameters: {},
|
||||
provider: { label: 'OpenAI', value: 'openAI' },
|
||||
tools: [],
|
||||
tool_options: {},
|
||||
category: 'general',
|
||||
execute_code: false,
|
||||
file_search: false,
|
||||
web_search: false,
|
||||
} as AgentForm,
|
||||
MOCK_USER_ID,
|
||||
);
|
||||
|
||||
render(<Harness />);
|
||||
|
||||
|
|
@ -295,8 +302,8 @@ describe('AgentPanel draft preservation', () => {
|
|||
expect(screen.getByLabelText('Draft name')).toHaveValue('');
|
||||
expect(screen.getByLabelText('Draft instructions')).toHaveValue('');
|
||||
expect(screen.getByLabelText('Draft model')).toHaveValue('');
|
||||
expect(getAgentDraft('agent-123')).toBeUndefined();
|
||||
expect(getAgentDraft(undefined)).toBeUndefined();
|
||||
expect(getAgentDraft('agent-123', MOCK_USER_ID)).toBeUndefined();
|
||||
expect(getAgentDraft(undefined, MOCK_USER_ID)).toBeUndefined();
|
||||
expect(mockLastAgentSelectHasDraft).toBe(false);
|
||||
});
|
||||
|
||||
|
|
@ -310,7 +317,7 @@ describe('AgentPanel draft preservation', () => {
|
|||
await waitFor(() => {
|
||||
expect(screen.getByLabelText('Draft name')).toHaveValue('Saved from API');
|
||||
});
|
||||
expect(getAgentDraft('agent-123')).toBeUndefined();
|
||||
expect(getAgentDraft('agent-123', MOCK_USER_ID)).toBeUndefined();
|
||||
expect(mockLastAgentSelectHasDraft).toBe(false);
|
||||
});
|
||||
|
||||
|
|
@ -321,21 +328,25 @@ describe('AgentPanel draft preservation', () => {
|
|||
fireEvent.change(screen.getByLabelText('Draft name'), {
|
||||
target: { value: 'Previous agent draft' },
|
||||
});
|
||||
saveAgentDraft('agent-456', {
|
||||
id: 'agent-456',
|
||||
name: 'Mounted switch draft',
|
||||
description: '',
|
||||
instructions: 'Draft for the switched agent',
|
||||
model: 'gpt-4o',
|
||||
model_parameters: {},
|
||||
provider: { label: 'OpenAI', value: 'openAI' },
|
||||
tools: [],
|
||||
tool_options: {},
|
||||
category: 'general',
|
||||
execute_code: false,
|
||||
file_search: false,
|
||||
web_search: false,
|
||||
} as AgentForm);
|
||||
saveAgentDraft(
|
||||
'agent-456',
|
||||
{
|
||||
id: 'agent-456',
|
||||
name: 'Mounted switch draft',
|
||||
description: '',
|
||||
instructions: 'Draft for the switched agent',
|
||||
model: 'gpt-4o',
|
||||
model_parameters: {},
|
||||
provider: { label: 'OpenAI', value: 'openAI' },
|
||||
tools: [],
|
||||
tool_options: {},
|
||||
category: 'general',
|
||||
execute_code: false,
|
||||
file_search: false,
|
||||
web_search: false,
|
||||
} as AgentForm,
|
||||
MOCK_USER_ID,
|
||||
);
|
||||
|
||||
mockCurrentAgentId = 'agent-456';
|
||||
rerender(<Harness />);
|
||||
|
|
@ -344,7 +355,25 @@ describe('AgentPanel draft preservation', () => {
|
|||
expect(screen.getByLabelText('Draft name')).toHaveValue('Mounted switch draft');
|
||||
});
|
||||
expect(screen.getByLabelText('Draft instructions')).toHaveValue('Draft for the switched agent');
|
||||
expect(getAgentDraft('agent-123')?.name).toBe('Previous agent draft');
|
||||
expect(getAgentDraft('agent-123', MOCK_USER_ID)?.name).toBe('Previous agent draft');
|
||||
expect(mockLastAgentSelectHasDraft).toBe(true);
|
||||
});
|
||||
|
||||
it('does not carry drafts across user changes while mounted', async () => {
|
||||
const { rerender } = render(<Harness />);
|
||||
|
||||
fireEvent.change(screen.getByLabelText('Draft name'), {
|
||||
target: { value: 'First user draft' },
|
||||
});
|
||||
expect(getAgentDraft(undefined, MOCK_USER_ID)?.name).toBe('First user draft');
|
||||
|
||||
mockUserId = 'user-456';
|
||||
rerender(<Harness />);
|
||||
|
||||
await waitFor(() => {
|
||||
expect(screen.getByLabelText('Draft name')).toHaveValue('');
|
||||
});
|
||||
expect(getAgentDraft(undefined, 'user-456')).toBeUndefined();
|
||||
expect(getAgentDraft(undefined, MOCK_USER_ID)?.name).toBe('First user draft');
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -80,4 +80,17 @@ describe('agent drafts', () => {
|
|||
expect(getAgentDraft('agent-a')).toBeUndefined();
|
||||
expect(getAgentDraft('agent-b')?.name).toBe('Agent B');
|
||||
});
|
||||
|
||||
it('keeps drafts isolated by user id', () => {
|
||||
saveAgentDraft(undefined, createForm({ name: 'User A new draft' }), 'user-a');
|
||||
saveAgentDraft(undefined, createForm({ name: 'User B new draft' }), 'user-b');
|
||||
saveAgentDraft('agent-1', createForm({ name: 'User A saved-agent draft' }), 'user-a');
|
||||
|
||||
clearAgentDraft(undefined, 'user-a');
|
||||
|
||||
expect(getAgentDraft(undefined, 'user-a')).toBeUndefined();
|
||||
expect(getAgentDraft(undefined, 'user-b')?.name).toBe('User B new draft');
|
||||
expect(getAgentDraft('agent-1', 'user-a')?.name).toBe('User A saved-agent draft');
|
||||
expect(getAgentDraft('agent-1', 'user-b')).toBeUndefined();
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -6,8 +6,8 @@ export type AgentDraftValues = Partial<AgentForm>;
|
|||
|
||||
const drafts = new Map<string, AgentDraftValues>();
|
||||
|
||||
export function getAgentDraftKey(agentId?: string | null): string {
|
||||
return agentId || NEW_AGENT_DRAFT_KEY;
|
||||
export function getAgentDraftKey(agentId?: string | null, userId?: string | null): string {
|
||||
return `${userId || 'anonymous'}:${agentId || NEW_AGENT_DRAFT_KEY}`;
|
||||
}
|
||||
|
||||
const sanitizeFile = ({ file: _file, ...value }: ExtendedFile): ExtendedFile => value;
|
||||
|
|
@ -37,20 +37,30 @@ export function sanitizeAgentDraft(values: AgentForm): AgentDraftValues {
|
|||
return draft;
|
||||
}
|
||||
|
||||
export function getAgentDraft(agentId?: string | null): AgentDraftValues | undefined {
|
||||
return drafts.get(getAgentDraftKey(agentId));
|
||||
export function getAgentDraft(
|
||||
agentId?: string | null,
|
||||
userId?: string | null,
|
||||
): AgentDraftValues | undefined {
|
||||
return drafts.get(getAgentDraftKey(agentId, userId));
|
||||
}
|
||||
|
||||
export function saveAgentDraft(agentId: string | null | undefined, values: AgentForm): void {
|
||||
drafts.set(getAgentDraftKey(agentId), sanitizeAgentDraft(values));
|
||||
export function saveAgentDraft(
|
||||
agentId: string | null | undefined,
|
||||
values: AgentForm,
|
||||
userId?: string | null,
|
||||
): void {
|
||||
drafts.set(getAgentDraftKey(agentId, userId), sanitizeAgentDraft(values));
|
||||
}
|
||||
|
||||
export function clearAgentDraft(agentId?: string | null): void {
|
||||
drafts.delete(getAgentDraftKey(agentId));
|
||||
export function clearAgentDraft(agentId?: string | null, userId?: string | null): void {
|
||||
drafts.delete(getAgentDraftKey(agentId, userId));
|
||||
}
|
||||
|
||||
export function clearAgentDrafts(agentIds: Array<string | null | undefined>): void {
|
||||
agentIds.forEach(clearAgentDraft);
|
||||
export function clearAgentDrafts(
|
||||
agentIds: Array<string | null | undefined>,
|
||||
userId?: string | null,
|
||||
): void {
|
||||
agentIds.forEach((agentId) => clearAgentDraft(agentId, userId));
|
||||
}
|
||||
|
||||
export function clearAllAgentDrafts(): void {
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue