From f4114e53c1eec897ca3eab3f2ec552c997e86411 Mon Sep 17 00:00:00 2001 From: Ravi Kumar L Date: Fri, 10 Jul 2026 17:48:15 +0200 Subject: [PATCH] fix(langfuse): simplify export enablement controls --- .../Integrations/LangfuseConnection.tsx | 117 +++++++++++------- .../__tests__/LangfuseConnection.spec.tsx | 64 ++++++---- client/src/locales/en/translation.json | 6 +- 3 files changed, 117 insertions(+), 70 deletions(-) diff --git a/client/src/components/Nav/SettingsTabs/Integrations/LangfuseConnection.tsx b/client/src/components/Nav/SettingsTabs/Integrations/LangfuseConnection.tsx index c955dded53..fa3de8da92 100644 --- a/client/src/components/Nav/SettingsTabs/Integrations/LangfuseConnection.tsx +++ b/client/src/components/Nav/SettingsTabs/Integrations/LangfuseConnection.tsx @@ -10,7 +10,6 @@ import { Input, Label, Spinner, - Switch, useToastContext, } from '@librechat/client'; import type { TLangfuseConnectionStatus } from 'librechat-data-provider'; @@ -23,7 +22,7 @@ import { import { useLocalize } from '~/hooks'; import { ESide } from '~/common'; -type ConnectionTestState = 'idle' | 'checking' | 'connected' | 'failed'; +type ConnectionTestState = 'idle' | 'unverified' | 'checking' | 'connected' | 'failed'; function getStoredConnectionTestKey(status?: TLangfuseConnectionStatus): string | undefined { if (status?.configured !== true || !status.destination || !status.publicKey) { @@ -41,6 +40,8 @@ function getConnectionStatusLabelKey(state: ConnectionTestState): TranslationKey return 'com_ui_langfuse_status_connected'; case 'failed': return 'com_ui_langfuse_status_failed'; + case 'unverified': + return 'com_ui_langfuse_status_not_verified'; case 'idle': default: return 'com_ui_langfuse_status_not_configured'; @@ -78,7 +79,6 @@ export default function LangfuseConnection() { const testMutation = useTestLangfuseConnectionMutation(); const [connectionStatus, setConnectionStatus] = useState(); - const [enabled, setEnabled] = useState(false); const [destination, setDestination] = useState(''); const [publicKey, setPublicKey] = useState(''); const [secretKey, setSecretKey] = useState(''); @@ -88,7 +88,6 @@ export default function LangfuseConnection() { const [connectionTestMessage, setConnectionTestMessage] = useState(''); const autoTestedConnectionRef = useRef(); const connectionTestRequestRef = useRef(0); - const skipConnectionStatusSyncRef = useRef(false); const publicKeyInputRef = useRef(null); const secretKeyInputRef = useRef(null); @@ -115,11 +114,6 @@ export default function LangfuseConnection() { if (!connectionStatus) { return; } - setEnabled(connectionStatus.enabled === true); - if (skipConnectionStatusSyncRef.current) { - skipConnectionStatusSyncRef.current = false; - return; - } const availableDestinations = connectionStatus.destinations ?? []; const storedDestination = availableDestinations.some( (option) => option.key === connectionStatus.destination, @@ -142,14 +136,14 @@ export default function LangfuseConnection() { const secretInputVisible = !secretConfigured || isEditingSecretKey; const displayPublicKey = getDisplayPublicKey(publicKey); const hasUnsavedChanges = - (!secretConfigured && enabled !== (connectionStatus?.enabled === true)) || destination !== (connectionStatus?.destination ?? '') || trimmedPublicKey !== (connectionStatus?.publicKey ?? '') || trimmedSecretKey !== ''; - const showActions = + const isEditing = !secretConfigured || isEditingPublicKey || isEditingSecretKey || hasUnsavedChanges; const canSubmit = destination !== '' && trimmedPublicKey !== '' && (secretConfigured || trimmedSecretKey !== ''); + const busy = testMutation.isLoading || updateMutation.isLoading; useEffect(() => { const storedConnectionTestKey = getStoredConnectionTestKey(connectionStatus); @@ -203,7 +197,7 @@ export default function LangfuseConnection() { const handleSave = () => { const requestId = ++connectionTestRequestRef.current; const payload = { - enabled, + enabled: true, destination, publicKey: trimmedPublicKey, ...(trimmedSecretKey ? { secretKey: trimmedSecretKey } : {}), @@ -214,7 +208,7 @@ export default function LangfuseConnection() { onSuccess: (nextStatus) => { autoTestedConnectionRef.current = getStoredConnectionTestKey(nextStatus); setConnectionStatus(nextStatus); - setConnectionTestState(enabled ? 'connected' : 'idle'); + setConnectionTestState('connected'); setConnectionTestMessage(''); setSecretKey(''); setIsEditingPublicKey(false); @@ -226,11 +220,6 @@ export default function LangfuseConnection() { }); }; - if (!enabled) { - saveConnection(); - return; - } - testMutation.mutate( { destination, @@ -275,12 +264,36 @@ export default function LangfuseConnection() { ) ? connectionStatus?.destination : undefined; - setEnabled(connectionStatus?.enabled === true); setDestination(storedDestination ?? ''); setPublicKey(connectionStatus?.publicKey ?? ''); setSecretKey(''); setIsEditingPublicKey(false); setIsEditingSecretKey(false); + + if (!storedDestination || !connectionStatus?.publicKey) { + setConnectionTestState('idle'); + setConnectionTestMessage(''); + return; + } + + const requestId = ++connectionTestRequestRef.current; + setConnectionTestState('checking'); + setConnectionTestMessage(''); + testMutation.mutate( + { destination: storedDestination, publicKey: connectionStatus.publicKey }, + { + onSuccess: (result) => { + if (requestId !== connectionTestRequestRef.current) return; + setConnectionTestState(result.success ? 'connected' : 'failed'); + setConnectionTestMessage(result.success ? '' : (result.message ?? '')); + }, + onError: () => { + if (requestId !== connectionTestRequestRef.current) return; + setConnectionTestState('failed'); + setConnectionTestMessage(localize('com_ui_langfuse_test_error')); + }, + }, + ); }; const handleDestinationChange = (nextDestination: string) => { @@ -324,13 +337,12 @@ export default function LangfuseConnection() { ); }; - const handleEnabledChange = (nextEnabled: boolean) => { - setEnabled(nextEnabled); + const handleEnabledChange = () => { if (!secretConfigured || !connectionStatus?.destination || !connectionStatus.publicKey) { return; } - const previousEnabled = connectionStatus.enabled === true; + const nextEnabled = connectionStatus.enabled !== true; const requestId = ++connectionTestRequestRef.current; const saveEnabledState = () => { updateMutation.mutate( @@ -345,7 +357,6 @@ export default function LangfuseConnection() { return; } autoTestedConnectionRef.current = getStoredConnectionTestKey(nextStatus); - skipConnectionStatusSyncRef.current = true; setConnectionStatus(nextStatus); showToast({ message: localize('com_ui_langfuse_saved'), status: 'success' }); }, @@ -353,7 +364,6 @@ export default function LangfuseConnection() { if (requestId !== connectionTestRequestRef.current) { return; } - setEnabled(previousEnabled); showToast({ message: localize('com_ui_langfuse_save_error'), status: 'error' }); }, }, @@ -367,12 +377,10 @@ export default function LangfuseConnection() {
-
+
-
- {localize('com_ui_langfuse_title')} -
+
{localize('com_ui_langfuse_title')}
{localize('com_ui_beta')}
@@ -384,12 +392,6 @@ export default function LangfuseConnection() { {localize('com_ui_langfuse_description')}
-
setIsEditingPublicKey(true)} > @@ -451,8 +454,13 @@ export default function LangfuseConnection() { data-bwignore="true" data-form-type="other" value={publicKey} + disabled={busy} placeholder="pk-lf-..." - onChange={(e) => setPublicKey(e.target.value)} + onChange={(e) => { + setPublicKey(e.target.value); + setConnectionTestState('unverified'); + setConnectionTestMessage(''); + }} /> )}
@@ -464,6 +472,7 @@ export default function LangfuseConnection() { type="button" className="w-full rounded-lg border border-border-light px-3 py-2 text-left hover:border-border-medium focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-ring-primary" aria-label={`${localize('com_ui_edit')} ${localize('com_ui_langfuse_secret_key')}`} + disabled={busy} onClick={() => setIsEditingSecretKey(true)} > @@ -482,34 +491,46 @@ export default function LangfuseConnection() { data-bwignore="true" data-form-type="other" value={secretKey} + disabled={busy} placeholder="sk-lf-..." - onChange={(e) => setSecretKey(e.target.value)} + onChange={(e) => { + setSecretKey(e.target.value); + setConnectionTestState('unverified'); + setConnectionTestMessage(''); + }} /> )}
- {showActions && ( + {isEditing ? ( <> - {secretConfigured && ( - - )} - + + ) : ( + )}
diff --git a/client/src/components/Nav/SettingsTabs/Integrations/__tests__/LangfuseConnection.spec.tsx b/client/src/components/Nav/SettingsTabs/Integrations/__tests__/LangfuseConnection.spec.tsx index 22dfcd9cfa..38d9859832 100644 --- a/client/src/components/Nav/SettingsTabs/Integrations/__tests__/LangfuseConnection.spec.tsx +++ b/client/src/components/Nav/SettingsTabs/Integrations/__tests__/LangfuseConnection.spec.tsx @@ -98,10 +98,18 @@ describe('LangfuseConnection', () => { expect(screen.queryByRole('button', { name: 'Show secret' })).not.toBeInTheDocument(); expect(screen.queryByText('com_ui_langfuse_test')).not.toBeInTheDocument(); expect(screen.getByText('com_ui_langfuse_status_not_configured')).toBeInTheDocument(); + expect(screen.getByRole('button', { name: 'com_ui_cancel' })).toBeVisible(); + expect(screen.getByRole('button', { name: 'com_ui_langfuse_save_and_enable' })).toBeVisible(); + expect( + screen.queryByRole('button', { name: 'com_ui_langfuse_enable' }), + ).not.toBeInTheDocument(); + expect( + screen.queryByRole('button', { name: 'com_ui_langfuse_disable' }), + ).not.toBeInTheDocument(); expect(mockTest).not.toHaveBeenCalled(); }); - it('prefills stored values, tests on load, and keeps toggle and destination editable', async () => { + it('prefills stored values, tests on load, and keeps destination editable', async () => { mockGet.mockReturnValue({ data: { configured: true, @@ -122,9 +130,9 @@ describe('LangfuseConnection', () => { expect(screen.getByText('pk-lf-...515f')).toBeInTheDocument(); expect(screen.queryByLabelText('com_ui_langfuse_secret_key')).not.toBeInTheDocument(); expect(screen.getByText('sk-lf-...515f')).toBeInTheDocument(); - expect(screen.queryByText('com_ui_save')).not.toBeInTheDocument(); + expect(screen.queryByText('com_ui_langfuse_save_and_enable')).not.toBeInTheDocument(); expect(screen.getByTestId('langfuse-destination')).toBeEnabled(); - expect(screen.getByRole('switch', { name: 'com_ui_langfuse_title' })).toBeEnabled(); + expect(screen.getByRole('button', { name: 'com_ui_langfuse_disable' })).toBeEnabled(); await waitFor(() => expect(mockTest).toHaveBeenCalledTimes(1)); expect(mockTest.mock.calls[0][0]).toEqual({ destination: 'us', @@ -161,7 +169,6 @@ describe('LangfuseConnection', () => { it('tests and saves the typed secret key when enabling a new connection', async () => { render(); - await userEvent.click(screen.getByRole('switch', { name: 'com_ui_langfuse_title' })); await selectDestination('us'); fireEvent.change(screen.getByLabelText('com_ui_langfuse_public_key'), { target: { value: 'pk-lf-1' }, @@ -170,7 +177,7 @@ describe('LangfuseConnection', () => { target: { value: 'sk-lf-secret' }, }); - await userEvent.click(screen.getByText('com_ui_save')); + await userEvent.click(screen.getByText('com_ui_langfuse_save_and_enable')); expect(mockTest).toHaveBeenCalledTimes(1); expect(mockTest.mock.calls[0][0]).toEqual({ @@ -203,7 +210,6 @@ describe('LangfuseConnection', () => { }); render(); - await userEvent.click(screen.getByRole('switch', { name: 'com_ui_langfuse_title' })); await selectDestination('us'); fireEvent.change(screen.getByLabelText('com_ui_langfuse_public_key'), { target: { value: 'pk-lf-1' }, @@ -212,7 +218,7 @@ describe('LangfuseConnection', () => { target: { value: 'sk-lf-secret' }, }); - await userEvent.click(screen.getByText('com_ui_save')); + await userEvent.click(screen.getByText('com_ui_langfuse_save_and_enable')); expect(mockTest).toHaveBeenCalledTimes(1); expect(screen.queryByLabelText('com_ui_langfuse_secret_key')).not.toBeInTheDocument(); @@ -248,7 +254,7 @@ describe('LangfuseConnection', () => { expect(screen.getByText('com_ui_langfuse_status_connected')).toBeInTheDocument(); mockTest.mockClear(); - await userEvent.click(screen.getByText('com_ui_save')); + await userEvent.click(screen.getByText('com_ui_langfuse_save_and_enable')); expect(mockTest).toHaveBeenCalledTimes(1); expect(mockUpdate).toHaveBeenCalledTimes(1); @@ -279,6 +285,11 @@ describe('LangfuseConnection', () => { }), ); + expect(screen.getByRole('button', { name: 'com_ui_cancel' })).toBeVisible(); + expect(screen.getByRole('button', { name: 'com_ui_langfuse_save_and_enable' })).toBeVisible(); + expect( + screen.queryByRole('button', { name: 'com_ui_langfuse_disable' }), + ).not.toBeInTheDocument(); expect(screen.getByLabelText('com_ui_langfuse_public_key')).toHaveValue('pk-lf-1'); expect(screen.getByLabelText('com_ui_langfuse_public_key')).toHaveFocus(); expect( @@ -298,7 +309,7 @@ describe('LangfuseConnection', () => { fireEvent.change(secretKeyInput, { target: { value: 'sk-lf-replacement' }, }); - await userEvent.click(screen.getByText('com_ui_save')); + await userEvent.click(screen.getByText('com_ui_langfuse_save_and_enable')); expect(mockTest).toHaveBeenCalledTimes(1); expect(mockTest.mock.calls[0][0]).toMatchObject({ @@ -331,7 +342,11 @@ describe('LangfuseConnection', () => { render(); await waitFor(() => expect(mockTest).toHaveBeenCalledTimes(1)); + mockTest.mockImplementationOnce((_payload, options) => { + options?.onSuccess?.({ success: false, message: 'invalid edited connection' }); + }); await selectDestination('us'); + expect(await screen.findByText('invalid edited connection')).toBeVisible(); await userEvent.click( screen.getByRole('button', { name: 'com_ui_edit com_ui_langfuse_public_key', @@ -348,14 +363,21 @@ describe('LangfuseConnection', () => { fireEvent.change(screen.getByLabelText(/com_ui_langfuse_secret_key/), { target: { value: 'sk-lf-edited' }, }); + mockTest.mockImplementationOnce((_payload, options) => { + options?.onSuccess?.({ success: true }); + }); await userEvent.click(screen.getByRole('button', { name: 'com_ui_cancel' })); - expect(screen.getByRole('switch', { name: 'com_ui_langfuse_title' })).toBeChecked(); - expect(screen.getByRole('switch', { name: 'com_ui_langfuse_title' })).toBeEnabled(); + expect(await screen.findByText('com_ui_langfuse_status_connected')).toBeVisible(); + expect(mockTest.mock.calls.at(-1)?.[0]).toEqual({ + destination: 'eu', + publicKey: 'pk-lf-original', + }); + expect(screen.getByRole('button', { name: 'com_ui_langfuse_disable' })).toBeEnabled(); expect(screen.getByTestId('langfuse-destination')).toHaveTextContent(destinationLabels.eu); expect(screen.getByText('pk-lf-...inal')).toBeInTheDocument(); expect(screen.getByText('sk-lf-...515f')).toBeInTheDocument(); - expect(screen.queryByText('com_ui_save')).not.toBeInTheDocument(); + expect(screen.queryByText('com_ui_langfuse_save_and_enable')).not.toBeInTheDocument(); expect(mockUpdate).not.toHaveBeenCalled(); }); @@ -364,7 +386,6 @@ describe('LangfuseConnection', () => { options?.onSuccess?.({ success: false, message: 'bad key' }); }); render(); - await userEvent.click(screen.getByRole('switch', { name: 'com_ui_langfuse_title' })); await selectDestination('us'); fireEvent.change(screen.getByLabelText('com_ui_langfuse_public_key'), { target: { value: 'pk-lf-1' }, @@ -373,7 +394,7 @@ describe('LangfuseConnection', () => { target: { value: 'sk-lf-secret' }, }); - await userEvent.click(screen.getByText('com_ui_save')); + await userEvent.click(screen.getByText('com_ui_langfuse_save_and_enable')); expect(mockTest).toHaveBeenCalledTimes(1); expect(mockUpdate).not.toHaveBeenCalled(); @@ -408,7 +429,8 @@ describe('LangfuseConnection', () => { fireEvent.change(screen.getByLabelText('com_ui_langfuse_public_key'), { target: { value: 'pk-lf-mangled' }, }); - await userEvent.click(screen.getByText('com_ui_save')); + expect(screen.getByText('com_ui_langfuse_status_not_verified')).toBeVisible(); + await userEvent.click(screen.getByText('com_ui_langfuse_save_and_enable')); expect(mockTest).toHaveBeenCalledWith( expect.objectContaining({ publicKey: 'pk-lf-mangled' }), @@ -446,7 +468,7 @@ describe('LangfuseConnection', () => { }); }); - await userEvent.click(screen.getByRole('switch', { name: 'com_ui_langfuse_title' })); + await userEvent.click(screen.getByRole('button', { name: 'com_ui_langfuse_disable' })); expect(mockTest).not.toHaveBeenCalled(); expect(mockUpdate).toHaveBeenCalledTimes(1); @@ -455,8 +477,8 @@ describe('LangfuseConnection', () => { destination: 'eu', publicKey: 'pk-lf-1', }); - expect(screen.queryByText('com_ui_save')).not.toBeInTheDocument(); - expect(screen.getByRole('switch', { name: 'com_ui_langfuse_title' })).not.toBeChecked(); + expect(screen.queryByText('com_ui_langfuse_save_and_enable')).not.toBeInTheDocument(); + expect(screen.getByRole('button', { name: 'com_ui_langfuse_enable' })).toBeEnabled(); }); it('saves immediately without testing when enabling a configured connection', async () => { @@ -485,14 +507,14 @@ describe('LangfuseConnection', () => { await waitFor(() => expect(mockTest).toHaveBeenCalledTimes(1)); mockTest.mockClear(); - await userEvent.click(screen.getByRole('switch', { name: 'com_ui_langfuse_title' })); + await userEvent.click(screen.getByRole('button', { name: 'com_ui_langfuse_enable' })); expect(mockTest).not.toHaveBeenCalled(); expect(mockUpdate).toHaveBeenCalledWith( { enabled: true, destination: 'eu', publicKey: 'pk-lf-1' }, expect.any(Object), ); - expect(screen.queryByText('com_ui_save')).not.toBeInTheDocument(); - expect(screen.getByRole('switch', { name: 'com_ui_langfuse_title' })).toBeChecked(); + expect(screen.queryByText('com_ui_langfuse_save_and_enable')).not.toBeInTheDocument(); + expect(screen.getByRole('button', { name: 'com_ui_langfuse_disable' })).toBeEnabled(); }); }); diff --git a/client/src/locales/en/translation.json b/client/src/locales/en/translation.json index a04e80fbc8..9fd6aa6644 100644 --- a/client/src/locales/en/translation.json +++ b/client/src/locales/en/translation.json @@ -1643,7 +1643,7 @@ "com_ui_settings_section_integrations": "Integrations", "com_ui_langfuse_title": "Langfuse export", "com_ui_langfuse_description": "Send this organization's traces and feedback scores to your own Langfuse project.", - "com_ui_langfuse_beta_info": "This feature is in beta. Enabling this setting will export traces from all agents in your org to this Langfuse connection.", + "com_ui_langfuse_beta_info": "This feature is in beta. Enabling Langfuse export will send traces from all agents in your org to this Langfuse connection.", "com_ui_langfuse_destination": "Destination", "com_ui_langfuse_public_key": "Public key", "com_ui_langfuse_secret_key": "Secret key", @@ -1652,7 +1652,11 @@ "com_ui_langfuse_status_failed": "Connection failed", "com_ui_langfuse_status_failed_hover": "Check Langfuse to see if traces are still failing. A one-time ping with the keys just failed.", "com_ui_langfuse_status_not_configured": "Not configured", + "com_ui_langfuse_status_not_verified": "Not verified", "com_ui_langfuse_testing": "Testing connection", + "com_ui_langfuse_save_and_enable": "Save & enable", + "com_ui_langfuse_enable": "Enable", + "com_ui_langfuse_disable": "Disable", "com_ui_langfuse_saved": "Langfuse connection saved", "com_ui_langfuse_save_error": "Failed to save the Langfuse connection", "com_ui_langfuse_test_error": "Could not connect to Langfuse",