packages/bruno-app/src/components/EnvironmentSettingsTab/EnvironmentList/StyledWrapper.js (1)

215-266: Duplicate input/actions styles in .creating block.

The .environment-name-input and .inline-actions styles (lines 216-234) duplicate what's already defined in .rename-container (lines 194-212). Consider extracting these shared styles or relying on CSS inheritance from the parent selectors.

packages/bruno-app/src/components/Environments/EnvironmentSelector/index.js (1)

222-284: Consider extracting repeated addTab dispatch logic.

The same addTab dispatch pattern appears four times (lines 226-232, 242-248, 258-264, 275-281). Consider extracting to helper functions:

const openEnvironmentSettingsTab = () => {
  dispatch(
    addTab({
      uid: `${collection.uid}-environment-settings`,
      collectionUid: collection.uid,
      type: 'environment-settings'
    })
  );
};

const openGlobalEnvironmentSettingsTab = () => {
  dispatch(
    addTab({
      uid: `${collection.uid}-global-environment-settings`,
      collectionUid: collection.uid,
      type: 'global-environment-settings'
    })
  );
};

Then use as onEnvironmentCreated={openEnvironmentSettingsTab} etc.

packages/bruno-app/src/providers/ReduxStore/slices/tabs.js (1)

24-30: Singleton & non-preview behavior for env/security tabs looks right

Marking security-settings, environment-settings, and global-environment-settings as nonReplaceableTabTypes correctly:

• Avoids opening duplicate tabs per collectionUid (re-activates existing instead)
• Makes these tabs non-preview by default

That matches the mental model of “one settings view per collection”. If you also want collection-settings / collection-overview to behave as singletons, they could be added to this list later, but that’s optional and not required by this change.

Also applies to: 38-44, 63-66, 84-87

packages/bruno-app/src/components/Environments/EnvironmentSettings/EnvironmentList/index.js (1)

14-24: EnvironmentList props match new flow; originalEnvironmentVariables now looks unused

Dropping onClose from the props and from how EnvironmentDetails is used is consistent with moving the close/navigation responsibility up to the tab system instead of the inner details view.

However, originalEnvironmentVariables is still tracked locally and passed into <EnvironmentDetails ... originalEnvironmentVariables={originalEnvironmentVariables} />, while the EnvironmentDetails in this path no longer accepts or uses that prop. If unsaved-changes logic has been moved fully into EnvironmentVariables, consider removing the originalEnvironmentVariables state and prop here to avoid dead parameters and confusion.

Also applies to: 96-100, 143-148

packages/bruno-app/src/components/EnvironmentSettingsTab/EnvironmentList/EnvironmentDetails/EnvironmentVariables/StyledWrapper.js (1)

1-192: Theme usage is solid; add explicit focus styles for better accessibility

Nice use of props.theme for all colors and borders here, which is consistent with the styled-components theming guidance (no raw CSS vars in this context). Based on learnings, this is the right pattern.

One thing to tighten up: input[type='text'] (and buttons via the global button rule plus subclasses) remove the browser outline (outline: none !important) and don’t introduce a custom focus visual. That makes it hard to see where focus is when tabbing.

Consider adding a clear :focus style (e.g., border or background change using theme tokens) instead of, or in addition to, removing the default outline, so keyboard users still get a visible focus indicator.

packages/bruno-app/src/components/GlobalEnvironmentSettingsTab/index.js (1)

1-8: Wrapper is fine; clarify whether collection should be part of the API

GlobalEnvironmentSettingsTab is a thin wrapper over WorkspaceEnvironments and currently doesn’t take any props, which is fine for now.

Since RequestTabPanel is calling it as <GlobalEnvironmentSettingsTab collection={collection} />, consider either:

• Updating this component to accept and (if needed later) use collection, or
• Dropping the collection prop at the call site so the API surface matches the implementation.

Right now the extra prop is harmless but can be misleading when scanning the code.

packages/bruno-app/src/components/WorkspaceHome/WorkspaceEnvironments/EnvironmentList/EnvironmentDetails/EnvironmentVariables/index.js (2)

21-21: Unused prop originalEnvironmentVariables.

The originalEnvironmentVariables prop is destructured but never referenced in the component body. Remove it if no longer needed.

-const EnvironmentVariables = ({ environment, setIsModified, originalEnvironmentVariables, collection }) => {
+const EnvironmentVariables = ({ environment, setIsModified, collection }) => {

---

125-150: Optimize draft comparison to avoid unnecessary Redux dispatches.

Using JSON.stringify for deep comparison works but is expensive for large variable sets. More importantly, the effect dispatches on every formik.values change, and the comparison logic could be simplified.

Consider memoizing the comparison or debouncing the draft sync to reduce Redux overhead during rapid typing.

packages/bruno-app/src/providers/Hotkeys/index.js (2)

43-58: Missing save handling for environment-settings tabs.

The save hotkey handles folder-settings and collection-settings but doesn't handle environment-settings or global-environment-settings tab types. Users might expect Ctrl+S to save environment changes when viewing those tabs.

Consider adding cases for the new tab types:

          } else if (activeTab.type === 'collection-settings') {
            dispatch(saveCollectionSettings(collection.uid));
+         } else if (activeTab.type === 'environment-settings') {
+           // Handle environment save via appropriate action
+         } else if (activeTab.type === 'global-environment-settings') {
+           // Handle global environment save via appropriate action
          }

---

66-66: Unnecessary dependency in useEffect array.

saveRequest is an imported action creator and doesn't change between renders. Remove it from the dependency array.

-  }, [activeTabUid, tabs, saveRequest, collections, dispatch]);
+  }, [activeTabUid, tabs, collections, dispatch]);

packages/bruno-app/src/components/EnvironmentSettingsTab/index.js (1)

30-32: Redundant null check.

Since environments defaults to [] on line 30, the !environments check on line 32 is redundant.

-  if (!environments || !environments.length) {
+  if (!environments.length) {

packages/bruno-app/src/components/EnvironmentSettingsTab/EnvironmentList/index.js (2)

62-65: Redundant condition in environment change check.

If hasSelectedEnvironmentChanged is true (via isEqual), then selectedEnvironment.uid !== _selectedEnvironment?.uid is implicitly covered. The second condition is only useful if isEqual returned false but UIDs differ, which would be a data inconsistency.

-      if (hasSelectedEnvironmentChanged || selectedEnvironment.uid !== _selectedEnvironment?.uid) {
+      if (hasSelectedEnvironmentChanged) {
         setSelectedEnvironment(_selectedEnvironment);
       }

---

89-117: Duplicate click-outside handler pattern.

These two useEffects share identical logic for handling outside clicks. Consider extracting to a custom hook like useClickOutside(ref, callback, isActive).

packages/bruno-app/src/components/EnvironmentSettingsTab/EnvironmentList/EnvironmentDetails/index.js (2)

23-49: Redundant length check and duplicate validation logic.

Line 28-30: If name.trim() === '' is false (line 24-26), then name.length >= 1 is guaranteed, making the check redundant.

Also, validateEnvironmentName is duplicated in EnvironmentList/index.js. Consider extracting to a shared utility.

 const validateEnvironmentName = (name) => {
   if (!name || name.trim() === '') {
     return 'Name is required';
   }

-  if (name.length < 1) {
-    return 'Must be at least 1 character';
-  }
-
   if (name.length > 255) {
     return 'Must be 255 characters or less';
   }

---

93-102: Potential UX issue with blur validation.

handleNameBlur validates and shows an error but doesn't auto-save or auto-cancel. Users might be confused when the error persists but clicking elsewhere doesn't resolve the state. Consider either auto-canceling on blur with invalid input or triggering save on blur with valid input.

packages/bruno-app/src/components/EnvironmentSettingsTab/EnvironmentList/EnvironmentDetails/EnvironmentVariables/index.js (3)

127-153: Performance consideration with JSON serialization on every change.

This effect runs on every formik.values change and performs multiple JSON.stringify operations. For large environment variable lists, this could impact performance.

Consider debouncing the draft sync or using a more efficient comparison (e.g., shallow compare of array length + deep compare only when lengths match).

---

104-124: Draft restoration effect intentionally omits formik from deps.

The effect correctly avoids including formik in the dependency array to prevent infinite loops. However, this pattern is fragile. Adding a brief comment explaining this design decision would help future maintainers.

 // Restore draft values on mount or environment switch
+// Note: formik intentionally omitted from deps to prevent infinite loops
 React.useEffect(() => {

---

220-223: Race condition risk with setTimeout + setFieldValue.

Using setTimeout to append a new row after handleChange completes could cause race conditions if the user types quickly. Consider using Formik's setValues to batch the update atomically.

 if (isLastRow) {
-  const newVariable = {
-    uid: uuid(),
-    name: '',
-    value: '',
-    type: 'text',
-    secret: false,
-    enabled: true
-  };
-  setTimeout(() => {
-    formik.setFieldValue(formik.values.length, newVariable, false);
-  }, 0);
+  const newVariable = {
+    uid: uuid(),
+    name: '',
+    value: '',
+    type: 'text',
+    secret: false,
+    enabled: true
+  };
+  formik.setValues([...formik.values, newVariable]);
 }

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Learnt from: CR
Repo: usebruno/bruno PR: 0
File: CODING_STANDARDS.md:0-0
Timestamp: 2025-12-05T20:31:33.005Z
Learning: Applies to **/*.{jsx,tsx} : Styled Components are used as wrappers to define both self and children components style; Tailwind classes are used specifically for layout based styles

Learnt from: CR
Repo: usebruno/bruno PR: 0
File: CODING_STANDARDS.md:0-0
Timestamp: 2025-12-05T20:31:33.005Z
Learning: Applies to **/*.{jsx,tsx} : Use styled component's theme prop to manage CSS colors and not CSS variables when in the context of a styled component or any React component using the styled component

Learnt from: CR
Repo: usebruno/bruno PR: 0
File: CODING_STANDARDS.md:0-0
Timestamp: 2025-12-05T20:31:33.005Z
Learning: Applies to **/*.{jsx,tsx} : Styled Component CSS might also change layout but Tailwind classes shouldn't define colors

packages/bruno-app/src/components/RequestTabPanel/index.js (1)

205-211: Verify collection prop usage in GlobalEnvironmentSettings.

Past review flagged that GlobalEnvironmentSettings may not use the collection prop being passed at line 210. If the component doesn't accept or use this prop, it should be removed to maintain API consistency.

Run the following script to verify prop usage:

#!/bin/bash
# Check if GlobalEnvironmentSettings component accepts collection prop

# Check component definition
cat packages/bruno-app/src/components/Environments/GlobalEnvironmentSettings/index.js | head -50

# Search for collection prop usage in the component
rg -n "collection" packages/bruno-app/src/components/Environments/GlobalEnvironmentSettings/ -C 3

tests/request/newlines/newlines-persistence.spec.ts (1)

5-78: Consider using test.step for better report readability.

The test could benefit from wrapping logical sections in test.step calls (e.g., "Create collection and accept sandbox", "Create request and add data", "Verify persistence after restart"). This makes Playwright reports easier to navigate and understand.

Example structure:

 test('should persist request with newlines across app restarts', async ({ createTmpDir, launchElectronApp }) => {
   const userDataPath = await createTmpDir('newlines-persistence-userdata');
   const collectionPath = await createTmpDir('newlines-persistence-collection');

-  // Create collection and request
-  const app1 = await launchElectronApp({ userDataPath });
-  const page = await app1.firstWindow();
-
-  await page.getByTestId('collections-header-add-menu').click();
-  ...
+  await test.step('Create collection and accept sandbox', async () => {
+    const app1 = await launchElectronApp({ userDataPath });
+    const page = await app1.firstWindow();
+    await page.getByTestId('collections-header-add-menu').click();
+    ...
+    await openCollectionAndAcceptSandbox(page, 'newlines-persistence', 'safe');
+  });
+
+  await test.step('Create request and add data with newlines', async () => {
+    ...
+  });
+
+  await test.step('Verify persistence after restart', async () => {
+    ...
+  });

As per coding guidelines, test.step usage is promoted for improved report readability.

tests/global-environments/non-string-values.spec.ts (3)

57-58: Consider more robust selectors for environment variable inputs.

Selecting inputs by value attribute (e.g., input[value="numericVar"]) can be fragile if the value isn't consistently set or if the DOM structure changes. Consider using data-testid attributes or a combination of text content and structural selectors for better stability.

Also applies to: 112-112

---

26-27: Extract repeated envTab locator to reduce duplication.

The envTab locator is defined three times with identical logic. Consider extracting it to a const at the test function level to follow DRY principles.

Example:

// At the top of the test function
const getEnvTab = () => page.locator('.request-tab').filter({ hasText: 'Global Environments' });

Then use getEnvTab() instead of redefining the locator each time.

Also applies to: 53-54, 125-126

---

37-38: Tab close pattern is repeated.

The pattern of hovering over the tab and clicking the close icon appears twice. If this pattern is used across multiple tests, consider extracting it to a helper function in the utils module.

Also applies to: 126-127

packages/bruno-app/src/components/WorkspaceHome/WorkspaceEnvironments/EnvironmentList/EnvironmentDetails/EnvironmentVariables/index.js (1)

175-210: handleRemoveVar dependency concern.

The useCallback depends on formik.values, which changes on every edit. This effectively disables memoization. Consider using a ref or restructuring to avoid the dependency.

Apply this change to improve memoization:

-  const handleRemoveVar = useCallback((id) => {
-    const currentValues = formik.values;
+  const handleRemoveVar = useCallback((id) => {
+    const currentValues = formik.values;

Or use formik.setValues with a function form to avoid the dependency:

   const handleRemoveVar = useCallback((id) => {
-    const currentValues = formik.values;
-
-    if (!currentValues || currentValues.length === 0) {
+    formik.setValues((currentValues) => {
+      if (!currentValues || currentValues.length === 0) {
-      return;
+        return currentValues;
-    }
+      }
-
-    const lastRow = currentValues[currentValues.length - 1];
-    const isLastEmptyRow = lastRow?.uid === id && (!lastRow.name || lastRow.name.trim() === '');
-
-    if (isLastEmptyRow) {
-      return;
-    }
-
-    const filteredValues = currentValues.filter((variable) => variable.uid !== id);
-
-    const hasEmptyLastRow = filteredValues.length > 0
-      && (!filteredValues[filteredValues.length - 1].name
-        || filteredValues[filteredValues.length - 1].name.trim() === '');
-
-    const newValues = hasEmptyLastRow
-      ? filteredValues
-      : [
-          ...filteredValues,
-          {
-            uid: uuid(),
-            name: '',
-            value: '',
-            type: 'text',
-            secret: false,
-            enabled: true
-          }
-        ];
-
-    formik.setValues(newValues);
-  }, [formik.values]);
+
+      const lastRow = currentValues[currentValues.length - 1];
+      const isLastEmptyRow = lastRow?.uid === id && (!lastRow.name || lastRow.name.trim() === '');
+
+      if (isLastEmptyRow) {
+        return currentValues;
+      }
+
+      const filteredValues = currentValues.filter((variable) => variable.uid !== id);
+
+      const hasEmptyLastRow = filteredValues.length > 0
+        && (!filteredValues[filteredValues.length - 1].name
+          || filteredValues[filteredValues.length - 1].name.trim() === '');
+
+      return hasEmptyLastRow
+        ? filteredValues
+        : [
+            ...filteredValues,
+            {
+              uid: uuid(),
+              name: '',
+              value: '',
+              type: 'text',
+              secret: false,
+              enabled: true
+            }
+          ];
+    });
+  }, []);

packages/bruno-app/src/components/Environments/EnvironmentSettings/EnvironmentList/EnvironmentDetails/EnvironmentVariables/index.js (1)

178-214: handleRemoveVar dependency concern.

Similar to the global variant, the useCallback depends on formik.values, which defeats memoization. Consider using the function form of formik.setValues to eliminate this dependency.

Use the function form of setValues to avoid depending on formik.values:

   const handleRemoveVar = useCallback((id) => {
-    const currentValues = formik.values;
-
-    if (!currentValues || currentValues.length === 0) {
-      return;
-    }
-
-    const lastRow = currentValues[currentValues.length - 1];
-    const isLastEmptyRow = lastRow?.uid === id && (!lastRow.name || lastRow.name.trim() === '');
-
-    if (isLastEmptyRow) {
-      return;
-    }
-
-    const filteredValues = currentValues.filter((variable) => variable.uid !== id);
-
-    const hasEmptyLastRow
-      = filteredValues.length > 0
-        && (!filteredValues[filteredValues.length - 1].name
-          || filteredValues[filteredValues.length - 1].name.trim() === '');
-
-    const newValues = hasEmptyLastRow
-      ? filteredValues
-      : [
-          ...filteredValues,
-          {
-            uid: uuid(),
-            name: '',
-            value: '',
-            type: 'text',
-            secret: false,
-            enabled: true
-          }
-        ];
-
-    formik.setValues(newValues);
-  }, [formik.values]);
+    formik.setValues((currentValues) => {
+      if (!currentValues || currentValues.length === 0) {
+        return currentValues;
+      }
+
+      const lastRow = currentValues[currentValues.length - 1];
+      const isLastEmptyRow = lastRow?.uid === id && (!lastRow.name || lastRow.name.trim() === '');
+
+      if (isLastEmptyRow) {
+        return currentValues;
+      }
+
+      const filteredValues = currentValues.filter((variable) => variable.uid !== id);
+
+      const hasEmptyLastRow
+        = filteredValues.length > 0
+          && (!filteredValues[filteredValues.length - 1].name
+            || filteredValues[filteredValues.length - 1].name.trim() === '');
+
+      return hasEmptyLastRow
+        ? filteredValues
+        : [
+            ...filteredValues,
+            {
+              uid: uuid(),
+              name: '',
+              value: '',
+              type: 'text',
+              secret: false,
+              enabled: true
+            }
+          ];
+    });
+  }, []);

tests/environments/import-environment/collection-env-import.spec.ts (1)

55-66: Consider wrapping verification in test.step for better reporting.

The tab verification and assertions work correctly, but wrapping this logic in a test.step would improve test report readability and align with the Playwright best practices promoted in the coding guidelines.

Apply this pattern:

+    await test.step('Verify imported environment variables in tab', async () => {
       const envTab = page.locator('.request-tab').filter({ hasText: 'Environments' });
       await expect(envTab).toBeVisible();
 
       await expect(page.locator('input[name="0.name"]')).toHaveValue('host');
       await expect(page.locator('input[name="1.name"]')).toHaveValue('userId');
       await expect(page.locator('input[name="2.name"]')).toHaveValue('apiKey');
       await expect(page.locator('input[name="3.name"]')).toHaveValue('postTitle');
       await expect(page.locator('input[name="4.name"]')).toHaveValue('postBody');
       await expect(page.locator('input[name="5.name"]')).toHaveValue('secretApiToken');
       await expect(page.locator('input[name="5.secret"]')).toBeChecked();
       await envTab.hover();
       await envTab.getByTestId('request-tab-close-icon').click();
+    });

tests/environments/import-environment/global-env-import.spec.ts (1)

50-61: Consider wrapping verification in test.step for better reporting.

Similar to the collection environment import test, wrapping this tab verification logic in a test.step would improve report readability and maintainability.

Apply this pattern:

+    await test.step('Verify imported global environment variables in tab', async () => {
       const envTab = page.locator('.request-tab').filter({ hasText: 'Global Environments' });
       await expect(envTab).toBeVisible();
 
       await expect(page.locator('input[name="0.name"]')).toHaveValue('host');
       await expect(page.locator('input[name="1.name"]')).toHaveValue('userId');
       await expect(page.locator('input[name="2.name"]')).toHaveValue('apiKey');
       await expect(page.locator('input[name="3.name"]')).toHaveValue('postTitle');
       await expect(page.locator('input[name="4.name"]')).toHaveValue('postBody');
       await expect(page.locator('input[name="5.name"]')).toHaveValue('secretApiToken');
       await expect(page.locator('input[name="5.secret"]')).toBeChecked();
       await envTab.hover();
       await envTab.getByTestId('request-tab-close-icon').click();
+    });

tests/environments/api-setEnvVar/api-setEnvVar-with-persist.spec.ts (1)

31-38: Consider extracting tab verification into a helper function.

The tab verification and close pattern appears twice in this test (lines 31-38 and 51-59). Extracting this into a reusable helper would reduce duplication and improve maintainability, especially since this pattern is also used across multiple test files.

Example helper:

async function verifyAndCloseEnvironmentsTab(page, expectedRows: string[]) {
  const envTab = page.locator('.request-tab').filter({ hasText: 'Environments' });
  await expect(envTab).toBeVisible();
  
  for (const rowName of expectedRows) {
    await expect(page.getByRole('row', { name: rowName })).toBeVisible();
  }
  
  await envTab.hover();
  await envTab.getByTestId('request-tab-close-icon').click();
}

tests/environments/global-env-config-selection/global-env-config-selection.spec.ts (1)

24-31: Consider wrapping verification in test.step for better reporting.

The tab verification logic would benefit from a test.step wrapper to improve test report readability, consistent with Playwright best practices.

+    await test.step('Verify global environment tab shows active environment', async () => {
       const envTab = page.locator('.request-tab').filter({ hasText: 'Global Environments' });
       await expect(envTab).toBeVisible();
 
       const activeEnvItem = page.locator('.environment-item.active');
       await expect(activeEnvItem).toContainText(currentEnvName);
 
       await envTab.hover();
       await envTab.getByTestId('request-tab-close-icon').click();
+    });

tests/environments/api-setEnvVar/api-setEnvVar-without-persist.spec.ts (1)

24-31: Consider extracting repeated verification pattern.

Similar to the persist test, this verification pattern is duplicated (lines 24-31 and 45-51). Consider extracting into a shared helper function to reduce duplication across test files.

Additionally, wrapping in test.step would improve report readability:

+    await test.step('Verify environment variables are set', async () => {
       const envTab = page.locator('.request-tab').filter({ hasText: 'Environments' });
       await expect(envTab).toBeVisible();
 
       await expect(page.getByRole('row', { name: 'token' }).getByRole('cell').nth(1)).toBeVisible();
       await expect(page.getByRole('row', { name: 'secret' }).getByRole('cell').nth(2)).toBeVisible();
       await envTab.hover();
       await envTab.getByTestId('request-tab-close-icon').click();
+    });

tests/environments/api-setEnvVar/multiple-persist-vars.spec.ts (2)

13-15: Reduce waitForTimeout usage.

The test uses waitForTimeout(200) multiple times, which the coding guidelines discourage. Consider replacing these with explicit locator waits or expectations.

Replace:

-    await page.waitForTimeout(200);
-    await page.locator('#configure-env').click();
-    await page.waitForTimeout(200);
+    await page.locator('#configure-env').click();
+    const envTab = page.locator('.request-tab').filter({ hasText: 'Environments' });
+    await expect(envTab).toBeVisible();

Based on coding guidelines: Try to reduce usage of page.waitForTimeout(); unless absolutely necessary.

---

76-77: Consider replacing waitForTimeout with locator-based waits.

Similar to the cleanup section, these timeouts could be replaced with explicit visibility checks for more reliable test execution.

-    await page.waitForTimeout(200);
-    await page.locator('#configure-env').click();
-    await page.waitForTimeout(200);
+    await page.locator('#configure-env').click();
+    const envTab = page.locator('.request-tab').filter({ hasText: 'Environments' });
+    await expect(envTab).toBeVisible();

tests/environments/import-environment/bruno-env-import/collection-env-import/collection-env-import.spec.ts (1)

99-99: Replace waitForTimeout with explicit wait.

Instead of a fixed timeout, consider waiting for a specific UI element to be ready.

-    await test.step('Verify both environments are available in selector', async () => {
-      await page.waitForTimeout(500);
-      await page.getByTestId('environment-selector-trigger').click();
+    await test.step('Verify both environments are available in selector', async () => {
+      await page.getByTestId('environment-selector-trigger').click();
+      await expect(page.locator('.dropdown-item').filter({ hasText: /^local$/ })).toBeVisible({ timeout: 10000 });

Based on coding guidelines.

tests/environments/update-global-environment-via-script/global-env-update-via-script.spec.ts (1)

35-56: Consider extracting repeated row locator pattern.

The pattern of locating a row by input value and reading CodeMirror content is repeated four times. Extracting this to a helper function would improve maintainability.

Example helper:

const getEnvVariableValue = async (page: Page, variableName: string): Promise<string> => {
  const row = page.locator('tbody tr').filter({ has: page.locator(`input[value="${variableName}"]`) });
  return await row.locator('.CodeMirror-line').first().textContent();
};

Then use:

const value = await getEnvVariableValue(page, 'existingEnvEnabled');
await expect(value).toContain('newExistingEnvEnabledValue');

tests/environments/export-environment/collection-env-export/collection-env-export.spec.ts (2)

45-45: Consider centralizing the export button selector.

The selector button[title="Export environment"] appears multiple times. Consider defining it once at the top of the test suite or in a locator helper.

const EXPORT_BUTTON = 'button[title="Export environment"]';
// Then use:
await page.locator(EXPORT_BUTTON).click();

This improves maintainability if the selector changes.

Also applies to: 102-102, 170-170, 227-227, 282-282, 337-337, 396-396

---

39-40: Repeated envTab locator pattern.

The pattern of defining and verifying the Environments tab is repeated across multiple tests. Consider extracting to a helper function in the test utilities.

Example:

const verifyEnvironmentsTab = async (page: Page, type: 'collection' | 'global' = 'collection') => {
  const tabLabel = type === 'collection' ? 'Environments' : 'Global Environments';
  const envTab = page.locator('.request-tab').filter({ hasText: tabLabel });
  await expect(envTab).toBeVisible();
  return envTab;
};

Also applies to: 96-97, 164-165, 222-223, 277-278, 332-333, 390-391

tests/environments/export-environment/global-env-export/global-env-export.spec.ts (1)

300-300: Remove unnecessary waitForTimeout.

The waitForTimeout(200) appears unnecessary here since the success message visibility check immediately follows.

     await test.step('Execute export and verify success', async () => {
       // Export the environments
       await page.getByRole('button', { name: 'Export 2 Environments' }).click();
-      await page.waitForTimeout(200);
       // Verify success message
       await expect(page.getByText('Environment(s) exported successfully', { exact: false }).first()).toBeVisible();
     });

The expect().toBeVisible() already includes an implicit wait.

packages/bruno-app/src/components/Environments/EnvironmentSettings/index.js (1)

30-30: Optional: Simplify with default parameter.

The environments extraction could be simplified using a default parameter.

-const EnvironmentSettings = ({ collection }) => {
+const EnvironmentSettings = ({ collection = {} }) => {
   const [isModified, setIsModified] = useState(false);
   const [selectedEnvironment, setSelectedEnvironment] = useState(null);
   const [tab, setTab] = useState('default');
   const [showExportModal, setShowExportModal] = useState(false);

-  const environments = collection?.environments || [];
+  const environments = collection.environments || [];

packages/bruno-app/src/components/Environments/EnvironmentSettings/EnvironmentList/EnvironmentDetails/index.js (2)

23-49: Minor: Redundant length validation.

Lines 28-30 check if (name.length < 1) which is redundant since lines 24-26 already check if (!name || name.trim() === ''). An empty string will have length 0, so this condition will never be reached independently.

   if (!name || name.trim() === '') {
     return 'Name is required';
   }

-  if (name.length < 1) {
-    return 'Must be at least 1 character';
-  }
-
   if (name.length > 255) {
     return 'Must be 255 characters or less';
   }

---

51-59: Consider adding a comment for the setTimeout delay.

The 50ms setTimeout is used to ensure the input is rendered before focusing. A brief comment would clarify this timing for future maintainers.

   const handleRenameClick = () => {
     setIsRenaming(true);
     setNewName(environment.name);
     setNameError('');
+    // Wait for input to render before focusing
     setTimeout(() => {
       inputRef.current?.focus();
       inputRef.current?.select();
     }, 50);
   };

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Learnt from: CR
Repo: usebruno/bruno PR: 0
File: CODING_STANDARDS.md:0-0
Timestamp: 2025-12-05T20:31:33.005Z
Learning: Applies to **/*.{jsx,tsx} : Styled Components are used as wrappers to define both self and children components style; Tailwind classes are used specifically for layout based styles

Learnt from: CR
Repo: usebruno/bruno PR: 0
File: CODING_STANDARDS.md:0-0
Timestamp: 2025-12-05T20:31:33.005Z
Learning: Applies to **/*.{jsx,tsx} : Styled Component CSS might also change layout but Tailwind classes shouldn't define colors

Learnt from: CR
Repo: usebruno/bruno PR: 0
File: CODING_STANDARDS.md:0-0
Timestamp: 2025-12-05T20:31:33.005Z
Learning: Applies to **/*.{jsx,tsx} : Use styled component's theme prop to manage CSS colors and not CSS variables when in the context of a styled component or any React component using the styled component

Learnt from: sanish-bruno
Repo: usebruno/bruno PR: 6090
File: tests/scripting/hooks/init-user-data/ui-state-snapshot.json:1-8
Timestamp: 2025-12-16T07:16:08.934Z
Learning: For e2e tests in the bruno repository: Collections that are shared between CLI and UI tests (comprehensive test suites testing core functionality) should be placed in `packages/bruno-tests/` to avoid duplication. The `tests/**/fixtures/collection` pattern should be used for test-specific collections that test particular UI behaviors or are specific to a single test file.

Learnt from: NikHillAgar
Repo: usebruno/bruno PR: 6279
File: packages/bruno-app/src/components/RequestPane/PromptVariables/PromptVariablesModal/index.js:79-86
Timestamp: 2025-12-02T20:48:05.670Z
Learning: In React 19+, ref callbacks support returning cleanup functions. When a ref callback returns a function, React will call it when the element unmounts or the ref changes, similar to useEffect cleanup. This is documented at https://react.dev/learn/manipulating-the-dom-with-refs#how-to-manage-a-list-of-refs-using-a-ref-callback

Learnt from: CR
Repo: usebruno/bruno PR: 0
File: CODING_STANDARDS.md:0-0
Timestamp: 2025-12-05T20:31:33.005Z
Learning: Use consistent patterns and helper utilities where they improve clarity. Prefer shared test utilities over copy-pasted setup code, but only when it actually reduces complexity

Learnt from: CR
Repo: usebruno/bruno PR: 0
File: CODING_STANDARDS.md:0-0
Timestamp: 2025-12-05T20:31:33.005Z
Learning: Applies to **/*.test.{js,jsx,ts,tsx} : Add tests for any new functionality or meaningful changes. If code is added, removed, or significantly modified, corresponding tests should be updated or created

tests/global-environments/non-string-values.spec.ts (1)

75-75: Remove unnecessary .nth(0) now that selectors are scoped to rows.

Since the selectors are now scoped to numericRow and booleanRow respectively, the .nth(0) is redundant. Each row should only contain one disabled-info-icon, making the index unnecessary and improving readability.

Apply this refactor:

-    const infoIcon = numericRow.locator('[id$="-disabled-info-icon"]').nth(0);
+    const infoIcon = numericRow.locator('[id$="-disabled-info-icon"]');

And for the boolean variable:

-    const infoIcon = booleanRow.locator('[id$="-disabled-info-icon"]').nth(0);
+    const infoIcon = booleanRow.locator('[id$="-disabled-info-icon"]');

Also applies to: 102-102

packages/bruno-app/src/components/RequestTabs/RequestTab/index.js (2)

251-266: Missing feedback when draft is incomplete (flagged previously).

As noted in the earlier review, if draft?.environmentUid or draft?.variables is missing, the save handler does nothing and the modal remains open without user feedback.

Add an else branch to handle incomplete drafts:

 onSaveAndClose={() => {
   const draft = collection.environmentsDraft;
   if (draft?.environmentUid && draft?.variables) {
     dispatch(saveEnvironment(draft.variables, draft.environmentUid, collection.uid))
       .then(() => {
         dispatch(clearEnvironmentsDraft({ collectionUid: collection.uid }));
         dispatch(closeTabs({ tabUids: [tab.uid] }));
         setShowConfirmEnvironmentClose(false);
         toast.success('Environment saved');
       })
       .catch((err) => {
         console.log('err', err);
         toast.error('Failed to save environment');
       });
+  } else {
+    toast.error('Incomplete environment draft — cannot save');
+    setShowConfirmEnvironmentClose(false);
   }
 }}

---

278-294: Missing feedback when global draft is incomplete (flagged previously).

As noted in the earlier review, if the global environment draft is missing required fields, the save handler silently does nothing and the modal remains open.

Add an else branch to mirror the collection environment handling:

 onSaveAndClose={() => {
   const draft = globalEnvironmentDraft;
   if (draft?.environmentUid && draft?.variables) {
     dispatch(saveGlobalEnvironment({ variables: draft.variables, environmentUid: draft.environmentUid }))
       .then(() => {
         dispatch(clearGlobalEnvironmentDraft());
         dispatch(closeTabs({ tabUids: [tab.uid] }));
         setShowConfirmGlobalEnvironmentClose(false);
         toast.success('Global environment saved');
       })
       .catch((err) => {
         console.log('err', err);
         toast.error('Failed to save global environment');
       });
+  } else {
+    toast.error('Incomplete global environment draft — cannot save');
+    setShowConfirmGlobalEnvironmentClose(false);
   }
 }}

packages/bruno-app/src/components/Environments/ConfirmCloseEnvironment/index.js (1)

5-5: Consider adding JSDoc for component documentation.

A brief JSDoc comment would help other developers understand the component's purpose and usage, especially in the context of the tab-based environment flow.

As per coding guidelines, add JSDoc to provide more details:

+/**
+ * Confirmation modal displayed when attempting to close an environment tab with unsaved changes.
+ * Provides options to save, discard, or cancel the close action.
+ *
+ * @param {Function} onCancel - Called when user clicks Cancel
+ * @param {Function} onCloseWithoutSave - Called when user clicks Don't Save
+ * @param {Function} onSaveAndClose - Called when user clicks Save
+ * @param {boolean} isGlobal - Whether this is for global or collection environment
+ */
 const ConfirmCloseEnvironment = ({ onCancel, onCloseWithoutSave, onSaveAndClose, isGlobal }) => {

tests/global-environments/non-string-values.spec.ts (1)

26-26: Consider extracting the repeated envTab locator.

The locator pattern .request-tab with filter { hasText: 'Global Environments' } appears three times in the test. Extracting it to a variable at the test scope would improve maintainability and reduce duplication.

Example refactor:

 test('should seed non-string globals via request and verify read-only + tooltip', async ({
   pageWithUserData: page
 }) => {
+  const getEnvTab = () => page.locator('.request-tab').filter({ hasText: 'Global Environments' });
+
   await openCollectionAndAcceptSandbox(page, 'global-env-non-string', 'safe');

   await test.step('Create a new global environment with a string variable', async () => {
     // ...
-    const envTab = page.locator('.request-tab').filter({ hasText: 'Global Environments' });
+    const envTab = getEnvTab();

Also applies to: 53-53, 125-125

packages/bruno-app/src/components/Environments/EnvironmentSettings/EnvironmentList/StyledWrapper.js (1)

276-276: Avoid fragile color string concatenation.

Appending 15 to the color value assumes it's in hex format. If the theme ever uses rgb(), hsl(), or other formats, this will break.

Consider using a CSS color-mix or rgba transformation helper from your theme/utils, or apply opacity via a separate opacity property if the theme supports it.

packages/bruno-app/src/components/RequestTabs/RequestTab/index.js (1)

262-262: Consider structured error logging.

Console.log is used for error handling in the catch blocks. While this follows the existing pattern in the file (lines 209, 237, 407), consider using a structured logging service for better observability in production.

Also applies to: 289-289

packages/bruno-app/src/components/WorkspaceHome/WorkspaceEnvironments/EnvironmentList/EnvironmentDetails/EnvironmentVariables/index.js (5)

21-21: Remove unused originalEnvironmentVariables prop.

This parameter is destructured but never used in the component. Remove it to keep the API clean.

-const EnvironmentVariables = ({ environment, setIsModified, originalEnvironmentVariables, collection }) => {
+const EnvironmentVariables = ({ environment, setIsModified, collection }) => {

---

61-77: Yup $isLastRow context is never provided — this validation logic is dead code.

The when('$isLastRow', ...) requires a validation context to be passed (e.g., via validateAt or validate with { context: { isLastRow } }). Since formik doesn't receive this context, the conditional always falls through to otherwise. The custom validate function at lines 78-96 handles this logic correctly, making this Yup schema redundant.

Simplify by removing the non-functional conditional:

  validationSchema: Yup.array().of(Yup.object({
    enabled: Yup.boolean(),
-   name: Yup.string()
-     .when('$isLastRow', {
-       is: true,
-       then: (schema) => schema.optional(),
-       otherwise: (schema) => schema
-         .required('Name cannot be empty')
-         .matches(variableNameRegex,
-           'Name contains invalid characters. Must only contain alphanumeric characters, "-", "_", "." and cannot start with a digit.')
-         .trim()
-     }),
+   name: Yup.string(),
    secret: Yup.boolean(),
    type: Yup.string(),
    uid: Yup.string(),
    value: Yup.mixed().nullable()
  })),

---

101-122: Missing formik.setValues in effect dependency array.

The effect calls formik.setValues but doesn't include it in dependencies. While formik's setValues is typically stable, the linter will flag this. Consider suppressing with a comment or using a ref pattern for clarity:

+ const setValuesRef = useRef(formik.setValues);
+ setValuesRef.current = formik.setValues;

  React.useEffect(() => {
    // ...
    if ((isMount || envChanged) && hasDraftForThisEnv && globalEnvironmentDraft?.variables) {
-     formik.setValues([
+     setValuesRef.current([
        ...globalEnvironmentDraft.variables,
        // ...
      ]);
    }
- }, [environment.uid, hasDraftForThisEnv, globalEnvironmentDraft?.variables]);
+ }, [environment.uid, hasDraftForThisEnv, globalEnvironmentDraft?.variables]);

Alternatively, add an eslint-disable comment if the current behavior is intentional.

---

225-227: setTimeout with stale closure risk.

formik.values.length is captured when setTimeout is scheduled. If rapid inputs occur, the index could be stale. Consider using a callback pattern or formik's arrayHelpers instead:

-     setTimeout(() => {
-       formik.setFieldValue(formik.values.length, newVariable, false);
-     }, 0);
+     const currentLength = formik.values.length;
+     setTimeout(() => {
+       formik.setFieldValue(currentLength, newVariable, false);
+     }, 0);

This makes the capture explicit, though the underlying race condition remains. If dynamic arrays become more complex, consider FieldArray.

---

152-173: Inline component recreated on every render.

ErrorMessage is defined inside the component body, causing React to treat it as a new component type on each render. For small tables this is fine, but consider extracting it outside the component or memoizing if performance becomes a concern.

packages/bruno-app/src/components/Environments/EnvironmentSettings/EnvironmentList/EnvironmentDetails/EnvironmentVariables/index.js (3)

33-38: Consider memoizing the collection cloning.

The collection is cloned on every render, which could be expensive for large collections. Since this depends on collection and globalEnvironmentVariables, wrap this in useMemo.

-  let _collection = collection ? cloneDeep(collection) : {};
-
-  const globalEnvironmentVariables = getGlobalEnvironmentVariables({ globalEnvironments, activeGlobalEnvironmentUid });
-  if (_collection) {
-    _collection.globalEnvironmentVariables = globalEnvironmentVariables;
-  }
+  const globalEnvironmentVariables = getGlobalEnvironmentVariables({ globalEnvironments, activeGlobalEnvironmentUid });
+  
+  const _collection = useMemo(() => {
+    const cloned = collection ? cloneDeep(collection) : {};
+    if (cloned) {
+      cloned.globalEnvironmentVariables = globalEnvironmentVariables;
+    }
+    return cloned;
+  }, [collection, globalEnvironmentVariables]);

---

279-281: Consider if setTimeout is necessary.

The setTimeout(0) is used to defer the new row addition, likely to avoid state update conflicts. If you encounter issues, consider using formik.setValues with a callback or ensure this pattern is actually needed.

---

296-342: LGTM - Consider extracting validation logic.

The save flow is correct and handles errors appropriately. For maintainability, you could extract the validation logic (lines 306-314) into a separate helper function, but this is optional.

Example:

const validateVariables = (variables) => {
  return variables.some((variable) => {
    if (!variable.name || variable.name.trim() === '') return true;
    if (!variableNameRegex.test(variable.name)) return true;
    return false;
  });
};

const handleSave = () => {
  // ... filtering logic ...
  if (validateVariables(variablesToSave)) {
    toast.error('Please fix validation errors before saving');
    return;
  }
  // ... rest of save logic ...
};

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Learnt from: CR
Repo: usebruno/bruno PR: 0
File: CODING_STANDARDS.md:0-0
Timestamp: 2025-12-05T20:31:33.005Z
Learning: Applies to **/*.{jsx,tsx} : Styled Components are used as wrappers to define both self and children components style; Tailwind classes are used specifically for layout based styles

Learnt from: CR
Repo: usebruno/bruno PR: 0
File: CODING_STANDARDS.md:0-0
Timestamp: 2025-12-05T20:31:33.005Z
Learning: Applies to **/*.{jsx,tsx} : Use styled component's theme prop to manage CSS colors and not CSS variables when in the context of a styled component or any React component using the styled component

Learnt from: CR
Repo: usebruno/bruno PR: 0
File: CODING_STANDARDS.md:0-0
Timestamp: 2025-12-05T20:31:33.005Z
Learning: Applies to **/*.{jsx,tsx} : Styled Component CSS might also change layout but Tailwind classes shouldn't define colors

Learnt from: NikHillAgar
Repo: usebruno/bruno PR: 6279
File: packages/bruno-app/src/components/RequestPane/PromptVariables/PromptVariablesModal/index.js:79-86
Timestamp: 2025-12-02T20:48:05.670Z
Learning: In React 19+, ref callbacks support returning cleanup functions. When a ref callback returns a function, React will call it when the element unmounts or the ref changes, similar to useEffect cleanup. This is documented at https://react.dev/learn/manipulating-the-dom-with-refs#how-to-manage-a-list-of-refs-using-a-ref-callback

Learnt from: CR
Repo: usebruno/bruno PR: 0
File: CODING_STANDARDS.md:0-0
Timestamp: 2025-12-05T20:31:33.005Z
Learning: Applies to **/*.test.{js,jsx,ts,tsx} : Add tests for any new functionality or meaningful changes. If code is added, removed, or significantly modified, corresponding tests should be updated or created

packages/bruno-app/src/components/RequestTabs/RequestTab/index.js (2)

255-270: Missing feedback when draft is incomplete.

The onSaveAndClose handler only executes when draft?.environmentUid && draft?.variables are truthy. If either is falsy, clicking "Save" does nothing and the modal stays open without user feedback.

🔎 Suggested fix

 onSaveAndClose={() => {
   const draft = collection.environmentsDraft;
   if (draft?.environmentUid && draft?.variables) {
     dispatch(saveEnvironment(draft.variables, draft.environmentUid, collection.uid))
       .then(() => {
         dispatch(clearEnvironmentsDraft({ collectionUid: collection.uid }));
         dispatch(closeTabs({ tabUids: [tab.uid] }));
         setShowConfirmEnvironmentClose(false);
         toast.success('Environment saved');
       })
       .catch((err) => {
         console.log('err', err);
         toast.error('Failed to save environment');
       });
+  } else {
+    toast.error('No environment changes to save');
+    setShowConfirmEnvironmentClose(false);
   }
 }}

---

282-298: Same issue: missing fallback for incomplete global environment draft.

Mirror the same handling pattern as collection environments for consistency.

🔎 Suggested fix

 onSaveAndClose={() => {
   const draft = globalEnvironmentDraft;
   if (draft?.environmentUid && draft?.variables) {
     dispatch(saveGlobalEnvironment({ variables: draft.variables, environmentUid: draft.environmentUid }))
       .then(() => {
         dispatch(clearGlobalEnvironmentDraft());
         dispatch(closeTabs({ tabUids: [tab.uid] }));
         setShowConfirmGlobalEnvironmentClose(false);
         toast.success('Global environment saved');
       })
       .catch((err) => {
         console.log('err', err);
         toast.error('Failed to save global environment');
       });
+  } else {
+    toast.error('No global environment changes to save');
+    setShowConfirmGlobalEnvironmentClose(false);
   }
 }}

tests/utils/page/actions.ts (1)

399-410: Consider splitting verification logic from environment creation.

The function now does more than just "create" - it also opens the selector, switches tabs, selects the environment, and verifies selection. While thorough, this verification adds latency to every test using createEnvironment.

Consider:

• Extracting lines 400-410 into a separate verifyEnvironmentCreated helper
• Or making the post-creation verification optional via a parameter
• This would allow tests that don't need full verification to run faster

Potential refactor approach

 const createEnvironment = async (
   page: Page,
   environmentName: string,
-  type: EnvironmentType = 'collection'
+  type: EnvironmentType = 'collection',
+  options: { verify?: boolean } = { verify: true }
 ) => {
   await test.step(`Create ${type} environment "${environmentName}"`, async () => {
     await openEnvironmentSelector(page, type);
     
     await page.locator('button[id="create-env"]').click();
     
     const nameInput = type === 'collection'
       ? page.locator('input[name="name"]')
       : page.locator('#environment-name');
     await expect(nameInput).toBeVisible();
     await nameInput.fill(environmentName);
     await page.getByRole('button', { name: 'Create' }).click();
 
-    const tabLabel = type === 'collection' ? 'Environments' : 'Global Environments';
-    await expect(page.locator('.request-tab').filter({ hasText: tabLabel })).toBeVisible();
-
-    const locators = buildCommonLocators(page);
-    await locators.environment.selector().click();
-    if (type === 'global') {
-      await locators.environment.globalTab().click();
-    }
-    await locators.environment.envOption(environmentName).click();
-    await expect(page.locator('.current-environment')).toContainText(environmentName);
+    if (options.verify) {
+      const tabLabel = type === 'collection' ? 'Environments' : 'Global Environments';
+      await expect(page.locator('.request-tab').filter({ hasText: tabLabel })).toBeVisible();
+
+      const locators = buildCommonLocators(page);
+      await locators.environment.selector().click();
+      if (type === 'global') {
+        await locators.environment.globalTab().click();
+      }
+      await locators.environment.envOption(environmentName).click();
+      await expect(page.locator('.current-environment')).toContainText(environmentName);
+    }
   });
 };

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Learnt from: naman-bruno
Repo: usebruno/bruno PR: 6407
File: packages/bruno-app/src/components/Environments/ConfirmCloseEnvironment/index.js:5-41
Timestamp: 2025-12-17T21:41:24.730Z
Learning: Do not suggest PropTypes validation for React components in the Bruno codebase. The project does not use PropTypes, so reviews should avoid proposing PropTypes and rely on the existing typing/validation approach (e.g., TypeScript or alternative runtime checks) if applicable. This guideline applies broadly to all JavaScript/JSX components in the repo.

packages/bruno-app/src/components/Environments/EnvironmentSettings/EnvironmentList/EnvironmentDetails/EnvironmentVariables/index.js (4)

107-151: Redundant validation: schema's $isLastRow context is never passed.

The validationSchema uses when('$isLastRow', ...) but Formik isn't configured with validationContext to provide this value. The schema validation will always use the otherwise branch. Since the custom validate function (lines 129-149) correctly handles the last-row logic, the schema's conditional is effectively dead code.

Consider simplifying by removing the when clause from the schema and relying solely on the custom validate function, or remove the custom validate and properly pass context to the schema.

🔎 Simplified schema (relying on custom validate):

     validationSchema: Yup.array().of(
       Yup.object({
         enabled: Yup.boolean(),
-        name: Yup.string()
-          .when('$isLastRow', {
-            is: true,
-            then: (schema) => schema.optional(),
-            otherwise: (schema) =>
-              schema
-                .required('Name cannot be empty')
-                .matches(
-                  variableNameRegex,
-                  'Name contains invalid characters. Must only contain alphanumeric characters, "-", "_", "." and cannot start with a digit.'
-                )
-                .trim()
-          }),
+        name: Yup.string().trim(),
         secret: Yup.boolean(),
         type: Yup.string(),
         uid: Yup.string(),
         value: Yup.mixed().nullable()
       })
     ),

Note: The static analysis hint about then on line 113 is a false positive—Yup's .when() uses then as a configuration property, not a Promise method.

---

214-235: Consider extracting ErrorMessage outside the component.

This inner component is recreated on every render. While the impact is minimal for this simple presentational component, extracting it outside and passing formik as a prop would be cleaner and slightly more performant.

---

275-292: Potential stale closure in setTimeout.

Line 289 captures formik.values.length at call time, but the setTimeout executes asynchronously. If the user types rapidly, the captured length might be stale. In practice, setFieldValue with an out-of-bounds index appends correctly, so this works, but using a functional update pattern would be clearer.

🔎 Alternative using current values:

     if (isLastRow) {
       const newVariable = {
         uid: uuid(),
         name: '',
         value: '',
         type: 'text',
         secret: false,
         enabled: true
       };
       setTimeout(() => {
-        formik.setFieldValue(formik.values.length, newVariable, false);
+        formik.setValues((currentValues) => [...currentValues, newVariable]);
       }, 0);
     }

---

305-351: Consider using formik.validateForm() instead of duplicating validation.

Lines 315-327 duplicate the validation logic that already exists in the custom validate function. You could simplify by calling formik.validateForm() and checking if errors exist:

const errors = await formik.validateForm();
if (Object.keys(errors).length > 0) {
  toast.error('Please fix validation errors before saving');
  return;
}