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

1403-1406: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Minor duplication of default item settings shape.

The { encodeUrl: true, forwardAuthorizationHeader: false } literal also appears in newEphemeralHttpRequest in packages/bruno-app/src/providers/ReduxStore/slices/collections/index.js (lines 966-969). Consider extracting a shared DEFAULT_HTTP_ITEM_SETTINGS constant if a third setting is ever added, to avoid drift between the two defaults.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/bruno-app/src/providers/ReduxStore/slices/collections/actions.js`
around lines 1403 - 1406, The HTTP item settings default shape is duplicated
between actions.js and the newEphemeralHttpRequest flow, which can drift over
time. Extract the shared default object into a common DEFAULT_HTTP_ITEM_SETTINGS
constant (or equivalent shared symbol used by both create/update paths) and have
both the settings initializer in actions.js and newEphemeralHttpRequest in
collections/index.js reference it instead of inlining the literal.

packages/bruno-converters/src/postman/postman-to-bruno.js (1)

508-511: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Consider adding/asserting converter test coverage for the new default.

This and the sibling Insomnia/OpenAPI/Swagger2 converters all now hardcode forwardAuthorizationHeader: false for every imported request — a security-relevant behavior change. Worth confirming existing converter test suites assert this default so a future refactor doesn't silently regress it.

As per coding guidelines, "Add tests for any new functionality or meaningful changes. If code is added, removed, or significantly modified, corresponding tests should be updated or created."

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/bruno-converters/src/postman/postman-to-bruno.js` around lines 508 -
511, The new hardcoded forwardAuthorizationHeader default in the Postman
converter needs test coverage so this security-relevant behavior is locked in.
Update the relevant converter test suite around postmanToBruno (and, if
applicable, the sibling converter test suites) to assert imported requests
always set forwardAuthorizationHeader to false, so future refactors cannot
silently change the default.

Source: Coding guidelines

tests/request/settings/redirect-auth-strip.spec.ts (2)

8-9: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Extract repeated sidebar locator into a variable.

The #sidebar-collection-name → getByText('settings-test') locator is rebuilt identically in both tests. As per path instructions, "Use locator variables for locators."

Also applies to: 34-35

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/request/settings/redirect-auth-strip.spec.ts` around lines 8 - 9, The
repeated sidebar locator built with `#sidebar-collection-name` and
getByText('settings-test') should be extracted into a shared locator variable.
Update the affected tests in redirect-auth-strip.spec.ts to define a reusable
locator once and reuse it for both the visibility assertion and the click,
following the existing locator-variable pattern used elsewhere in the test file.

Source: Path instructions

---

4-28: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Wrap test phases in test.step for readable reports.

Neither test breaks its Arrange/Act/Assert/Cleanup phases into test.step calls. As per path instructions, "Promote the use of test.step as much as possible so the generated reports are easier to read."

♻️ Example refactor for the first test

   test('should strip Authorization and Proxy-Authorization on cross-origin redirects when setting is OFF', async ({
     pageWithUserData: page
   }) => {
-    // Open collection
-    await expect(page.locator('`#sidebar-collection-name`').getByText('settings-test')).toBeVisible();
-    await page.locator('`#sidebar-collection-name`').getByText('settings-test').click();
-
-    // Open request
-    await page.getByRole('complementary').getByText('cross-origin-redirect-auth-strip').click();
-
-    // Send request
-    await page.getByTestId('send-arrow-icon').click();
-
-    // Verify status code
-    await expect(page.getByTestId('response-status-code')).toContainText('200', { timeout: 15000 });
-
-    // Verify headers are stripped
-    const responseTexts = await page.getByTestId('response-preview-container').locator('.CodeMirror-scroll').allInnerTexts();
-    const fullText = responseTexts.join('\n');
-    expect(fullText).not.toContain('"authorization":');
-    expect(fullText).not.toContain('"proxy-authorization":');
-
-    // Close tab
-    await page.locator('.close-icon-container').click({ force: true });
+    const collectionItem = page.locator('`#sidebar-collection-name`').getByText('settings-test');
+
+    await test.step('Open collection and request', async () => {
+      await expect(collectionItem).toBeVisible();
+      await collectionItem.click();
+      await page.getByRole('complementary').getByText('cross-origin-redirect-auth-strip').click();
+    });
+
+    await test.step('Send request', async () => {
+      await page.getByTestId('send-arrow-icon').click();
+      await expect(page.getByTestId('response-status-code')).toContainText('200', { timeout: 15000 });
+    });
+
+    await test.step('Verify headers are stripped', async () => {
+      const responseTexts = await page.getByTestId('response-preview-container').locator('.CodeMirror-scroll').allInnerTexts();
+      const fullText = responseTexts.join('\n');
+      expect(fullText).not.toContain('"authorization":');
+      expect(fullText).not.toContain('"proxy-authorization":');
+    });
+
+    await test.step('Cleanup', async () => {
+      await page.locator('.close-icon-container').click({ force: true });
+    });
   });

Also applies to: 30-54

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/request/settings/redirect-auth-strip.spec.ts` around lines 4 - 28, The
test in redirect-auth-strip.spec.ts should be refactored to wrap its
Arrange/Act/Assert/Cleanup phases in test.step blocks for clearer Playwright
reports. Update the cross-origin-redirect-auth-strip flow by grouping the
existing page interactions and assertions inside named steps within the current
test('should strip Authorization and Proxy-Authorization on cross-origin
redirects when setting is OFF') so the sequence is easier to follow. Apply the
same pattern to the other affected test in this file as well, keeping the
existing locators and assertions intact while moving them into test.step calls.

Source: Path instructions

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes