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

182-190: Consider formatting consistency and accessibility.

The implementation is functionally correct. Two optional improvements:

1. Formatting: For consistency with other icons in this file (lines 127, 177), consider single-line formatting since there are only 3 props.

2. Accessibility: Add an aria-label to improve screen reader support.

🔎 Suggested improvements:

-              <div className="flex items-center cursor-pointer short-tab">
-                {
-                  activeCollection && (
-                    <IconPlus
-                      size={18}
-                      strokeWidth={1.5}
-                      onClick={() => setNewRequestModalOpen(true)}
-                    />
-                  )
-                }
-              </div>
+              <div className="flex items-center cursor-pointer short-tab">
+                {activeCollection && (
+                  <IconPlus
+                    size={18}
+                    strokeWidth={1.5}
+                    onClick={() => setNewRequestModalOpen(true)}
+                    aria-label="Create new request"
+                  />
+                )}
+              </div>

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.

tests/request/encoding/curl-encoding.spec.ts (2)

17-60: Consider using test.step for improved test reporting.

Both test cases would benefit from wrapping logical steps in test.step blocks to improve readability of generated reports, as recommended in the coding guidelines.

Example refactor for the first test:

 test('Should generate code with proper URL encoding for unencoded input', async ({
   page,
   createTmpDir
 }) => {
   const collectionName = 'unencoded-test-collection';
   const requestName = 'curl-encoding-unencoded';

-  // Use plus icon button in new workspace UI
-  await page.getByTestId('collections-header-add-menu').click();
-  await page.locator('.tippy-box .dropdown-item').filter({ hasText: 'Create collection' }).click();
-  await page.getByLabel('Name').fill(collectionName);
-  const locationInput = page.getByLabel('Location');
-  if (await locationInput.isVisible()) {
-    await locationInput.fill(await createTmpDir(collectionName));
-  }
-  await page.locator('.bruno-modal').getByRole('button', { name: 'Create', exact: true }).click();
+  await test.step('Create collection with safe mode enabled', async () => {
+    await page.getByTestId('collections-header-add-menu').click();
+    await page.locator('.tippy-box .dropdown-item').filter({ hasText: 'Create collection' }).click();
+    await page.getByLabel('Name').fill(collectionName);
+    const locationInput = page.getByLabel('Location');
+    if (await locationInput.isVisible()) {
+      await locationInput.fill(await createTmpDir(collectionName));
+    }
+    await page.locator('.bruno-modal').getByRole('button', { name: 'Create', exact: true }).click();

-  await expect(page.locator('#sidebar-collection-name').filter({ hasText: collectionName })).toBeVisible();
-  await page.locator('#sidebar-collection-name').filter({ hasText: collectionName }).click();
-  await page.getByLabel('Safe Mode').check();
-  await page.getByRole('button', { name: 'Save' }).click();
+    await expect(page.locator('#sidebar-collection-name').filter({ hasText: collectionName })).toBeVisible();
+    await page.locator('#sidebar-collection-name').filter({ hasText: collectionName }).click();
+    await page.getByLabel('Safe Mode').check();
+    await page.getByRole('button', { name: 'Save' }).click();
+  });

-  // Create a new request using the dialog/modal flow
-  await createRequest(page, requestName, collectionName, { url: 'http://base.source?name=John Doe' });
+  await test.step('Create request with unencoded URL', async () => {
+    await createRequest(page, requestName, collectionName, { url: 'http://base.source?name=John Doe' });
+    await page.locator('.collection-item-name').filter({ hasText: requestName }).first().click();
+  });

-  // Click the request in the sidebar
-  await page.locator('.collection-item-name').filter({ hasText: requestName }).first().click();
-
-  await page.locator('#send-request .infotip').first().click();
-
-  await expect(page.getByRole('dialog')).toBeVisible();
-  await expect(page.getByRole('dialog').locator('.bruno-modal-header-title')).toContainText('Generate Code');
-
-  const codeEditor = page.locator('.editor-content .CodeMirror').first();
-  await expect(codeEditor).toBeVisible();
-
-  const generatedCode = await codeEditor.textContent();
-
-  expect(generatedCode).toContain('http://base.source/?name=John%20Doe');
+  await test.step('Verify URL encoding in generated code', async () => {
+    await page.locator('#send-request .infotip').first().click();
+
+    await expect(page.getByRole('dialog')).toBeVisible();
+    await expect(page.getByRole('dialog').locator('.bruno-modal-header-title')).toContainText('Generate Code');
+
+    const codeEditor = page.locator('.editor-content .CodeMirror').first();
+    await expect(codeEditor).toBeVisible();
+
+    const generatedCode = await codeEditor.textContent();
+    expect(generatedCode).toContain('http://base.source/?name=John%20Doe');
+  });

Also applies to: 62-105

---

24-37: Consider extracting duplicate collection setup logic.

The collection creation and safe mode configuration logic is duplicated between both tests. Consider extracting this into a helper function in the test utils for better maintainability.

Also applies to: 69-82

tests/collection/moving-requests/tag-persistence.spec.ts (1)

2-44: LGTM! Solid refactor to named requests with tag verification.

The migration to createRequest with a loop for creating multiple named requests is clean and maintains good test structure. The use of constants (collectionName, requestUrl, tagName, requestNames) improves readability and maintainability.

Optional: Consider reducing page.waitForTimeout() usage.

The new loop introduces several page.waitForTimeout() calls (lines 37, 40, 43). Per the coding guidelines, consider using Playwright's built-in wait mechanisms where possible:

Example improvement

-      await page.waitForTimeout(200);
       await locators.tags.input().fill(tagName);
       await locators.tags.input().press('Enter');
-      await page.waitForTimeout(200);
       await expect(locators.tags.item(tagName)).toBeVisible();

The expect(...).toBeVisible() already waits for the element, potentially making the explicit timeouts redundant.

tests/response/large-response-crash-prevention.spec.ts (1)

14-39: Consider using test.step for better test reports.

The test could benefit from wrapping logical sections in test.step blocks to improve readability of generated reports, as recommended in the coding guidelines.

🔎 View suggested refactor

 test('Show appropriate warning for responses over 10MB', async ({ page, createTmpDir }) => {
   const collectionName = 'size-warning-test';
   const requestName = 'large-response';

-  // Create collection
-  await createCollection(page, collectionName, await createTmpDir(collectionName), { openWithSandboxMode: 'safe' });
+  await test.step('Create collection and request', async () => {
+    await createCollection(page, collectionName, await createTmpDir(collectionName), { openWithSandboxMode: 'safe' });

-  // Create request using the dialog/modal flow
-  await createRequest(page, requestName, collectionName, {
-    url: 'https://samples.json-format.com/employees/json/employees_50MB.json'
+    await createRequest(page, requestName, collectionName, {
+      url: 'https://samples.json-format.com/employees/json/employees_50MB.json'
+    });
   });

-  // Send request
-  const sendButton = page.getByTestId('send-arrow-icon');
-  await sendButton.click();
+  await test.step('Send request', async () => {
+    const sendButton = page.getByTestId('send-arrow-icon');
+    await sendButton.click();
+  });

-  // Verify warning appears
-  await expect(page.getByText('Large Response Warning')).toBeVisible({ timeout: 60000 });
-
-  // Verify warning content
-  await expect(page.getByText('Handling responses over')).toBeVisible();
-  await expect(page.getByText('could degrade performance')).toBeVisible();
-
-  // Verify action button
-  await expect(page.getByRole('button', { name: 'View', exact: true })).toBeVisible();
+  await test.step('Verify large response warning', async () => {
+    await expect(page.getByText('Large Response Warning')).toBeVisible({ timeout: 60000 });
+    await expect(page.getByText('Handling responses over')).toBeVisible();
+    await expect(page.getByText('could degrade performance')).toBeVisible();
+    await expect(page.getByRole('button', { name: 'View', exact: true })).toBeVisible();
+  });
 });

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

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: 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

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

13-13: Remove unused CreateUntitledRequest import.

This import is no longer used since the component now uses IconPlus with the NewRequest modal.

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

5-5: Consolidate @tabler/icons imports.

IconPlus on line 14 should be merged with the existing import on line 5 to keep imports tidy.

🔎 Suggested fix:

-import { IconChevronRight, IconChevronLeft } from '@tabler/icons';
+import { IconChevronRight, IconChevronLeft, IconPlus } from '@tabler/icons';
...
-import { IconPlus } from '@tabler/icons';

Also applies to: 14-14

tests/collection/moving-requests/tag-persistence.spec.ts (2)

29-29: Consider reducing waitForTimeout calls.

Per coding guidelines, prefer Playwright's built-in waiting mechanisms over fixed timeouts. The 1000ms wait on line 29 seems particularly long. Consider replacing with explicit locator waits where possible (e.g., waiting for a specific element state after save).

Also applies to: 37-37, 40-40, 43-43

---

11-70: Consider using test.step for better report readability.

Per coding guidelines, test.step helps generate easier-to-read reports. The test has clear logical phases (setup, create requests, add tags, move, verify) that would benefit from step wrapping.

Example:

await test.step('Create collection and requests', async () => {
  // creation logic
});

await test.step('Add tags to requests', async () => {
  // tagging logic
});

await test.step('Move request and verify tag persistence', async () => {
  // move and verify logic
});

tests/collection/moving-requests/cross-collection-drag-drop-request.spec.ts (1)

26-30: Fragile parent selector pattern.

Using .locator('..') to traverse to parent depends on the exact DOM structure. If the markup changes, this will break silently. Consider extracting a more robust locator helper in buildCommonLocators that encapsulates this pattern.

Example approach:

// In locators.ts
collectionContainer: (name: string) => page.locator('.collection-container').filter({ has: page.locator('.collection-name', { hasText: name }) })

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: 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: 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: 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.

tests/collection/moving-requests/tag-persistence.spec.ts (1)

40-40: Cross-platform keyboard shortcut issue.

Meta+s only works on macOS. Use ControlOrMeta+s for cross-platform compatibility.

🔎 Suggested fix:

-      await page.keyboard.press('Meta+s');
+      await page.keyboard.press('ControlOrMeta+s');

tests/collection/create/create-collection.spec.ts (2)

10-42: Consider wrapping logical steps in test.step for clearer reports.

Per the coding guidelines, using test.step improves report readability. This test has clear logical phases (collection creation, request creation, URL setting, send request, verify response) that would benefit from grouping.

🔎 Example structure:

  test('Create collection and add a simple HTTP request', async ({ page, createTmpDir }) => {
    const collectionName = 'test-collection';
    const requestName = 'ping';

+   await test.step('Create a new collection', async () => {
      await page.getByTestId('collections-header-add-menu').click();
      await page.locator('.tippy-box .dropdown-item').filter({ hasText: 'Create collection' }).click();
      await page.getByLabel('Name').click();
      await page.getByLabel('Name').fill(collectionName);
      await page.getByLabel('Name').press('Tab');
      const locationInput = page.locator('.bruno-modal').getByLabel('Location');
      if (await locationInput.isVisible()) {
        await locationInput.fill(await createTmpDir(collectionName));
      }
      await page.locator('.bruno-modal').getByRole('button', { name: 'Create', exact: true }).click();
      await page.locator('#sidebar-collection-name').filter({ hasText: collectionName }).click();
+   });

+   await test.step('Create a new request', async () => {
      await createRequest(page, requestName, collectionName);
+   });
    // ... additional steps
  });

---

23-24: Add an assertion after collection creation to ensure it succeeded.

Before proceeding to create a request, verify the collection was actually created. This improves test reliability and provides a clearer failure point.

🔎 Suggested fix:

    await page.locator('.bruno-modal').getByRole('button', { name: 'Create', exact: true }).click();
+   await expect(page.locator('#sidebar-collection-name').filter({ hasText: collectionName })).toBeVisible();
    await page.locator('#sidebar-collection-name').filter({ hasText: collectionName }).click();

tests/request/encoding/curl-encoding.spec.ts (1)

17-58: Consider extracting shared setup into a helper or using test.step.

Both tests share nearly identical collection/request creation code. This duplication could be reduced with a shared helper function or by using a parameterized test approach. Additionally, wrapping logical steps in test.step would improve report readability per coding guidelines.

Also applies to: 60-101

tests/collection/moving-requests/tag-persistence.spec.ts (1)

27-27: Reduce page.waitForTimeout() usage where possible.

Per coding guidelines, minimize waitForTimeout calls when locators can be found using existing expect() assertions. Several of these waits could potentially be replaced with explicit visibility checks or removed if the preceding actions already ensure readiness.

🔎 Example improvements:

-    await page.waitForTimeout(1000);
+    await expect(locators.sidebar.collection(collectionName)).toBeVisible();
     // Create three requests via the dialog/modal flow, then add a tag to each
     const requestNames = ['request-1', 'request-2', 'request-3'];
     for (const requestName of requestNames) {
       await createRequest(page, requestName, collectionName, { url: requestUrl });
       await locators.sidebar.request(requestName).click();
       await locators.tabs.requestTab(requestName).waitFor({ state: 'visible' });
       await selectRequestPaneTab(page, 'Settings');
-      await page.waitForTimeout(200);
       await locators.tags.input().fill(tagName);
       await locators.tags.input().press('Enter');
-      await page.waitForTimeout(200);
       await expect(locators.tags.item(tagName)).toBeVisible();
       await page.keyboard.press('ControlOrMeta+s');
-      await page.waitForTimeout(200);
     }

Also applies to: 35-41, 65-66

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

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: 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