diff --git a/ui/desktop/src/components/settings/extensions/modal/EnvVarsSection.tsx b/ui/desktop/src/components/settings/extensions/modal/EnvVarsSection.tsx index b894574693b9..bd6d27534aed 100644 --- a/ui/desktop/src/components/settings/extensions/modal/EnvVarsSection.tsx +++ b/ui/desktop/src/components/settings/extensions/modal/EnvVarsSection.tsx @@ -42,7 +42,10 @@ interface EnvVarsSectionProps { onRemove: (index: number) => void; onChange: (index: number, field: 'key' | 'value', value: string) => void; submitAttempted: boolean; - onPendingInputChange?: (hasPendingInput: boolean) => void; + onPendingInputChange: ( + hasPendingInput: boolean, + pendingEnvVar: { key: string; value: string } | null + ) => void; } export default function EnvVarsSection({ @@ -65,7 +68,9 @@ export default function EnvVarsSection({ // Notify parent when pending input changes React.useEffect(() => { const hasPendingInput = newKey.trim() !== '' || newValue.trim() !== ''; - onPendingInputChange?.(hasPendingInput); + const pendingEnvVar = + newKey.trim() && newValue.trim() ? { key: newKey, value: newValue } : null; + onPendingInputChange(hasPendingInput, pendingEnvVar); }, [newKey, newValue, onPendingInputChange]); const handleAdd = () => { diff --git a/ui/desktop/src/components/settings/extensions/modal/ExtensionModal.test.tsx b/ui/desktop/src/components/settings/extensions/modal/ExtensionModal.test.tsx index 1ae077570f97..839cac578508 100644 --- a/ui/desktop/src/components/settings/extensions/modal/ExtensionModal.test.tsx +++ b/ui/desktop/src/components/settings/extensions/modal/ExtensionModal.test.tsx @@ -1,9 +1,20 @@ -import { describe, it, expect, vi } from 'vitest'; +import { describe, it, expect, vi, beforeEach } from 'vitest'; import { render, type RenderOptions, screen, waitFor } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import ExtensionModal from './ExtensionModal'; import { ExtensionFormData } from '../utils'; import { IntlTestWrapper } from '../../../../i18n/test-utils'; +import { upsertConfig } from '../../../../api'; + +vi.mock('../../../../api', async () => { + const actual = await vi.importActual('../../../../api'); + return { + ...actual, + upsertConfig: vi.fn().mockResolvedValue({ data: {} }), + }; +}); + +const mockedUpsertConfig = vi.mocked(upsertConfig); const renderWithIntl = (ui: React.ReactElement, options?: RenderOptions) => render(ui, { wrapper: IntlTestWrapper, ...options }); @@ -246,4 +257,135 @@ describe('ExtensionModal', () => { { key: 'Authorization', value: 'Bearer abc123', isEdited: true }, ]); }); + + describe('pending env var capture (fix for #8969)', () => { + beforeEach(() => { + mockedUpsertConfig.mockClear(); + mockedUpsertConfig.mockResolvedValue({ data: {} }); + }); + + const emptyInitialData: ExtensionFormData = { + name: '', + description: '', + type: 'stdio', + cmd: '', + endpoint: '', + enabled: true, + timeout: 300, + envVars: [], + headers: [], + }; + + // Returns the env-var key+value inputs (scoped to the "Environment Variables" section, + // disambiguated from the header inputs which share the "Value" placeholder). + function getEnvVarInputs() { + const envVarKeyInput = screen.getByPlaceholderText('Variable name'); + const envVarValueInput = screen + .getAllByPlaceholderText('Value') + .find((input) => + input.parentElement?.parentElement?.parentElement?.textContent?.includes( + 'Environment Variables' + ) + ); + return { envVarKeyInput, envVarValueInput }; + } + + it('captures a pending env var typed but not "+ Added" when Submit is clicked', async () => { + const user = userEvent.setup(); + const mockOnSubmit = vi.fn(); + const mockOnClose = vi.fn(); + + renderWithIntl( + + ); + + await user.type(screen.getByPlaceholderText('Enter extension name...'), 'WooMCP'); + await user.type( + screen.getByPlaceholderText(/^e\.g\. npx/), + 'npx -y @automattic/mcp-wordpress-remote@latest' + ); + + const { envVarKeyInput, envVarValueInput } = getEnvVarInputs(); + await user.type(envVarKeyInput, 'JWT_TOKEN'); + if (envVarValueInput) { + await user.type(envVarValueInput, 'my_very_long_token'); + } + + // Note: intentionally NOT clicking the "+ Add" button — this is the #8969 repro. + await user.click(screen.getByTestId('extension-submit-btn')); + + await waitFor(() => { + expect(mockOnSubmit).toHaveBeenCalled(); + }); + + expect(mockedUpsertConfig).toHaveBeenCalledWith( + expect.objectContaining({ + body: expect.objectContaining({ + is_secret: true, + key: 'JWT_TOKEN', + value: 'my_very_long_token', + }), + }) + ); + + const submittedData = mockOnSubmit.mock.calls[0][0]; + expect(submittedData.envVars).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + key: 'JWT_TOKEN', + value: 'my_very_long_token', + isEdited: true, + }), + ]) + ); + }); + + it('does not capture a pending env var when only the key is filled', async () => { + const user = userEvent.setup(); + const mockOnSubmit = vi.fn(); + const mockOnClose = vi.fn(); + + renderWithIntl( + + ); + + await user.type(screen.getByPlaceholderText('Enter extension name...'), 'WooMCP'); + await user.type(screen.getByPlaceholderText(/^e\.g\. npx/), 'npx -y something'); + + const { envVarKeyInput } = getEnvVarInputs(); + await user.type(envVarKeyInput, 'LONELY_KEY'); + // Intentionally leaving the value field empty. + + await user.click(screen.getByTestId('extension-submit-btn')); + + await waitFor(() => { + expect(mockOnSubmit).toHaveBeenCalled(); + }); + + expect(mockedUpsertConfig).not.toHaveBeenCalledWith( + expect.objectContaining({ + body: expect.objectContaining({ key: 'LONELY_KEY' }), + }) + ); + + const submittedData = mockOnSubmit.mock.calls[0][0]; + expect(submittedData.envVars).not.toEqual( + expect.arrayContaining([expect.objectContaining({ key: 'LONELY_KEY' })]) + ); + }); + }); }); diff --git a/ui/desktop/src/components/settings/extensions/modal/ExtensionModal.tsx b/ui/desktop/src/components/settings/extensions/modal/ExtensionModal.tsx index c9051f0acc94..97acdaab8e55 100644 --- a/ui/desktop/src/components/settings/extensions/modal/ExtensionModal.tsx +++ b/ui/desktop/src/components/settings/extensions/modal/ExtensionModal.tsx @@ -83,6 +83,7 @@ export default function ExtensionModal({ const [submitAttempted, setSubmitAttempted] = useState(false); const [showCloseConfirmation, setShowCloseConfirmation] = useState(false); const [hasPendingEnvVars, setHasPendingEnvVars] = useState(false); + const [pendingEnvVar, setPendingEnvVar] = useState<{ key: string; value: string } | null>(null); const [hasPendingHeaders, setHasPendingHeaders] = useState(false); const [pendingHeader, setPendingHeader] = useState<{ key: string; value: string } | null>(null); @@ -232,6 +233,14 @@ export default function ExtensionModal({ [] ); + const handlePendingEnvVarChange = useCallback( + (hasPending: boolean, envVar: { key: string; value: string } | null) => { + setHasPendingEnvVars(hasPending); + setPendingEnvVar(envVar); + }, + [] + ); + // Function to store a secret value const storeSecret = async (key: string, value: string) => { try { @@ -289,6 +298,19 @@ export default function ExtensionModal({ return finalHeaders; }; + const getFinalEnvVars = () => { + const finalEnvVars = [...formData.envVars]; + if ( + pendingEnvVar && + pendingEnvVar.key.trim() !== '' && + pendingEnvVar.value.trim() !== '' && + !pendingEnvVar.key.includes(' ') + ) { + finalEnvVars.push({ ...pendingEnvVar, isEdited: true }); + } + return finalEnvVars; + }; + const isHeadersValid = () => { return getFinalHeaders().every( ({ key, value }) => (key === '' && value === '') || (key !== '' && value !== '') @@ -323,6 +345,7 @@ export default function ExtensionModal({ if (isFormValid()) { const finalFormData = { ...formData, + envVars: getFinalEnvVars(), headers: getFinalHeaders(), }; @@ -437,7 +460,7 @@ export default function ExtensionModal({ onRemove={handleRemoveEnvVar} onChange={handleEnvVarChange} submitAttempted={submitAttempted} - onPendingInputChange={setHasPendingEnvVars} + onPendingInputChange={handlePendingEnvVarChange} />