packages/bruno-cli/src/commands/run.js (1)

369-378: Extension handling inconsistency in __name__ derivation.

For YAML files (line 373), fileExt includes the leading dot (e.g., .yml), which is correct for path.basename(filePath, fileExt). However, for the fallback branch (lines 374-378), unknown extensions are treated as .bru format but always strip .bru from the filename, even if the actual extension differs.

This could result in incorrect __name__ values for files with non-standard extensions.

Suggested improvement

     } else {
       const content = fs.readFileSync(filePath, 'utf8').replace(/\r\n/g, '\n');
       const envJson = parseEnvironment(content);
       result = getEnvVars(envJson);
-      result.__name__ = nameOverride || path.basename(filePath, '.bru');
+      const actualExt = path.extname(filePath);
+      result.__name__ = nameOverride || path.basename(filePath, actualExt || '.bru');
     }

tests/runner/cli-env-combined/collection/bruno.json (1)

1-5: Consider moving to fixtures/collection subdirectory.

Per the coding guidelines, test-specific collections should follow the tests/**/fixtures/collection pattern. The current path tests/runner/cli-env-combined/collection/ should ideally be tests/runner/cli-env-combined/fixtures/collection/.

The JSON content itself is valid. Based on learnings, this pattern helps maintain consistency across test fixtures.

tests/runner/cli-env-combined/cli-env-combined.spec.ts (4)

7-8: Fixtures should be nested inside a fixtures folder.

Per coding guidelines, test fixtures should follow the pattern tests/**/fixtures/collection. The collection directory should be at tests/runner/cli-env-combined/fixtures/collection rather than tests/runner/cli-env-combined/collection.

Suggested structure

- const collectionPath = path.resolve(__dirname, 'collection');
+ const collectionPath = path.resolve(__dirname, 'fixtures/collection');

Move the collection directory into a new fixtures folder accordingly.

---

20-36: Use test.afterEach for cleanup and test.step for report readability.

Per coding guidelines:

1. Promote the use of test.step so generated reports are easier to read.
2. Repeated cleanup logic in each test should be consolidated into test.afterEach.

Proposed refactor

const outputFiles: string[] = [];

test.afterEach(async () => {
  for (const file of outputFiles) {
    try {
      fs.unlinkSync(file);
    } catch (_) {}
  }
  outputFiles.length = 0;
});

test('CLI: Should allow --env and --env-file to be used together', async () => {
  await test.step('Run CLI with both --env and --env-file', async () => {
    const envFilePath = path.join(collectionPath, 'global-env.json');
    const outputPath = path.join(collectionPath, 'combined-out.json');
    outputFiles.push(outputPath);

    const exitCode = runFrom(
      collectionPath,
      `run request.bru --env CollectionEnv --env-file "${envFilePath}" --reporter-json "${outputPath}"`
    );
    expect(exitCode).toBe(0);
  });

  await test.step('Verify output file was created', async () => {
    expect(fs.existsSync(outputPath)).toBe(true);
  });
});

Also applies to: 38-68, 70-89, 91-113

---

33-35: Empty catch blocks silently swallow errors.

The cleanup try/catch blocks with empty catches (catch (_) {}) hide potential issues. If cleanup fails for unexpected reasons (e.g., permissions), you'd want visibility.

Optional improvement

  try {
    fs.unlinkSync(outputPath);
- } catch (_) {}
+ } catch (e) {
+   if ((e as NodeJS.ErrnoException).code !== 'ENOENT') {
+     console.warn(`Cleanup warning: ${e}`);
+   }
+ }

Or better yet, use fs.rmSync(outputPath, { force: true }) which ignores missing files:

- try {
-   fs.unlinkSync(outputPath);
- } catch (_) {}
+ fs.rmSync(outputPath, { force: true });

Also applies to: 65-67, 86-88, 110-112

---

20-36: Tests are async but don't await anything.

All test functions are declared async but contain no await expressions. Either remove async or restructure to use test.step with async callbacks for consistency.

Also applies to: 38-68, 70-89, 91-113

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

Learnt from: naman-bruno
Repo: usebruno/bruno PR: 6735
File: packages/bruno-electron/src/ipc/collection.js:936-939
Timestamp: 2026-01-07T18:53:37.000Z
Learning: In the bruno repository, the Redux reducer determines collection format by checking for the `opencollection` property in brunoConfig. The `version` property is not used for format detection, so having both `version` and `opencollection` properties together doesn't cause issues since `opencollection` takes precedence.

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.