From 38ab7179c5944e353e1b7990d9d3c69f2b5e25db Mon Sep 17 00:00:00 2001 From: UGBOMEH OGOCHUKWU WILLIAMS Date: Sun, 17 May 2026 15:48:53 +0100 Subject: [PATCH 1/2] fix(ui): preserve pending env vars in Add Extension form (#8969) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When adding an MCP extension via the desktop UI, env vars typed into the empty input row at the bottom of the Environment Variables section were silently dropped if the user clicked Save without first clicking "+ Add". This caused MCPs that need secrets (most of them) to fail at activation with `Configuration value not found: KEY_NAME`. The bug was inconsistent handling between EnvVarsSection and the sibling HeadersSection: HeadersSection already passes `pendingHeader` upward via `onPendingInputChange` and the parent uses `getFinalHeaders()` to include it on submit. EnvVarsSection only passed a boolean `hasPendingInput`, so the parent had no way to recover the typed value. Mirror the headers pattern: 1. Extend `EnvVarsSection.onPendingInputChange` to pass the pending `{ key, value }` object alongside the boolean. 2. Add `pendingEnvVar` state and `getFinalEnvVars()` in `ExtensionModal` (mirrors `pendingHeader` + `getFinalHeaders()`). 3. Use `getFinalEnvVars()` in `handleSubmit` so the pending entry is included in both `storeSecret` calls and the data passed to `onSubmit`. Adds two regression tests in `ExtensionModal.test.tsx`: - pending env var typed without "+ Add" is captured on submit (the exact #8969 repro — fails before this fix, passes after) - pending env var with only a key (no value) is ignored Fixes #8969 --- .../extensions/modal/EnvVarsSection.tsx | 9 +- .../extensions/modal/ExtensionModal.test.tsx | 144 +++++++++++++++++- .../extensions/modal/ExtensionModal.tsx | 20 ++- 3 files changed, 169 insertions(+), 4 deletions(-) diff --git a/ui/desktop/src/components/settings/extensions/modal/EnvVarsSection.tsx b/ui/desktop/src/components/settings/extensions/modal/EnvVarsSection.tsx index b894574693b9..5d5af8be4c7a 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..3060c7b41c47 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,14 @@ export default function ExtensionModal({ return finalHeaders; }; + const getFinalEnvVars = () => { + const finalEnvVars = [...formData.envVars]; + if (pendingEnvVar && pendingEnvVar.key.trim() !== '' && pendingEnvVar.value.trim() !== '') { + finalEnvVars.push({ ...pendingEnvVar, isEdited: true }); + } + return finalEnvVars; + }; + const isHeadersValid = () => { return getFinalHeaders().every( ({ key, value }) => (key === '' && value === '') || (key !== '' && value !== '') @@ -323,6 +340,7 @@ export default function ExtensionModal({ if (isFormValid()) { const finalFormData = { ...formData, + envVars: getFinalEnvVars(), headers: getFinalHeaders(), }; @@ -437,7 +455,7 @@ export default function ExtensionModal({ onRemove={handleRemoveEnvVar} onChange={handleEnvVarChange} submitAttempted={submitAttempted} - onPendingInputChange={setHasPendingEnvVars} + onPendingInputChange={handlePendingEnvVarChange} /> From ff284eea01199689be9073c51f73cf78bfc76742 Mon Sep 17 00:00:00 2001 From: Douwe Osinga Date: Mon, 18 May 2026 10:36:30 -0400 Subject: [PATCH 2/2] fix: reject env var keys with spaces in getFinalEnvVars, make onPendingInputChange required Address codex review: getFinalEnvVars now applies the same space-in-key validation that handleAdd enforces, preventing bypass when a pending env var is auto-captured at submit time. Also make onPendingInputChange non-optional in EnvVarsSectionProps since it is always provided (matches HeadersSection pattern). Signed-off-by: Douwe Osinga --- .../settings/extensions/modal/EnvVarsSection.tsx | 4 ++-- .../settings/extensions/modal/ExtensionModal.tsx | 7 ++++++- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/ui/desktop/src/components/settings/extensions/modal/EnvVarsSection.tsx b/ui/desktop/src/components/settings/extensions/modal/EnvVarsSection.tsx index 5d5af8be4c7a..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,7 @@ interface EnvVarsSectionProps { onRemove: (index: number) => void; onChange: (index: number, field: 'key' | 'value', value: string) => void; submitAttempted: boolean; - onPendingInputChange?: ( + onPendingInputChange: ( hasPendingInput: boolean, pendingEnvVar: { key: string; value: string } | null ) => void; @@ -70,7 +70,7 @@ export default function EnvVarsSection({ const hasPendingInput = newKey.trim() !== '' || newValue.trim() !== ''; const pendingEnvVar = newKey.trim() && newValue.trim() ? { key: newKey, value: newValue } : null; - onPendingInputChange?.(hasPendingInput, pendingEnvVar); + onPendingInputChange(hasPendingInput, pendingEnvVar); }, [newKey, newValue, onPendingInputChange]); const handleAdd = () => { diff --git a/ui/desktop/src/components/settings/extensions/modal/ExtensionModal.tsx b/ui/desktop/src/components/settings/extensions/modal/ExtensionModal.tsx index 3060c7b41c47..97acdaab8e55 100644 --- a/ui/desktop/src/components/settings/extensions/modal/ExtensionModal.tsx +++ b/ui/desktop/src/components/settings/extensions/modal/ExtensionModal.tsx @@ -300,7 +300,12 @@ export default function ExtensionModal({ const getFinalEnvVars = () => { const finalEnvVars = [...formData.envVars]; - if (pendingEnvVar && pendingEnvVar.key.trim() !== '' && pendingEnvVar.value.trim() !== '') { + if ( + pendingEnvVar && + pendingEnvVar.key.trim() !== '' && + pendingEnvVar.value.trim() !== '' && + !pendingEnvVar.key.includes(' ') + ) { finalEnvVars.push({ ...pendingEnvVar, isEdited: true }); } return finalEnvVars;