Skip to content

Conversation

@majiayu000
Copy link

Summary

  • Fixed $.cwd('.'), $.cwd('./'), and $.cwd('') returning undefined path
  • Now falls back to process.cwd() when defaultCwd is not set

Test plan

  • Added regression test in test/regression/issue/25579.test.ts
  • Tests $.cwd('.'), $.cwd('./'), $.cwd(''), $.cwd(), and absolute paths

Fixes #25579

When calling $.cwd('.') or $.cwd(''), the defaultCwd variable was
undefined, causing the path to become 'undefined'. Now we fall back
to process.cwd() when defaultCwd is not set.

Fixes oven-sh#25579

Signed-off-by: lif <1835304752@qq.com>
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 25, 2025

Walkthrough

The changes modify how the current working directory is resolved in the shell implementation when defaultCwd is undefined, adding a fallback to process.cwd(). A regression test file is added to verify cwd behavior with various arguments.

Changes

Cohort / File(s) Summary
Shell cwd resolution logic
src/js/builtins/shell.ts
Modified default cwd resolution in BunShell template and ShellPrototype.cwd to use defaultCwd ?? process.cwd() instead of defaultCwd, ensuring a fallback to the process working directory when defaultCwd is undefined.
Regression test for issue #25579
test/regression/issue/25579.test.ts
Added tests for Bun.$\pwd`.cwdwith arguments'.', './', ''`, and no argument to verify current-dir behavior, plus an absolute path test with macOS symlink handling expectations.

Suggested reviewers

  • nektro

Pre-merge checks

✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title 'fix(shell): resolve '.' to process.cwd() in cwd() method' accurately describes the main change: fixing the shell's cwd() method to resolve '.' and similar indicators to process.cwd().
Description check ✅ Passed The PR description covers the core issue and test plan, though it doesn't explicitly follow the template sections. The content addresses what was fixed and how it was verified, matching the template's intent.

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Disabled knowledge base sources:

  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 73d3600 and 480d8e1.

📒 Files selected for processing (1)
  • test/regression/issue/25579.test.ts
🧰 Additional context used
📓 Path-based instructions (5)
test/**/*.test.{ts,js,jsx,tsx,mjs,cjs}

📄 CodeRabbit inference engine (test/CLAUDE.md)

test/**/*.test.{ts,js,jsx,tsx,mjs,cjs}: Use bun:test with files that end in *.test.{ts,js,jsx,tsx,mjs,cjs}
Do not write flaky tests. Never wait for time to pass in tests; always wait for the condition to be met instead of using an arbitrary amount of time
Never use hardcoded port numbers in tests. Always use port: 0 to get a random port
Prefer concurrent tests over sequential tests using test.concurrent or describe.concurrent when multiple tests spawn processes or write files, unless it's very difficult to make them concurrent
When spawning Bun processes in tests, use bunExe and bunEnv from harness to ensure the same build of Bun is used and debug logging is silenced
Use -e flag for single-file tests when spawning Bun processes
Use tempDir() from harness to create temporary directories with files for multi-file tests instead of creating files manually
Prefer async/await over callbacks in tests
When callbacks must be used and it's just a single callback, use Promise.withResolvers to create a promise that can be resolved or rejected from a callback
Do not set a timeout on tests. Bun already has timeouts
Use Buffer.alloc(count, fill).toString() instead of 'A'.repeat(count) to create repetitive strings in tests, as ''.repeat is very slow in debug JavaScriptCore builds
Use describe blocks for grouping related tests
Always use await using or using to ensure proper resource cleanup in tests for APIs like Bun.listen, Bun.connect, Bun.spawn, Bun.serve, etc
Always check exit codes and test error scenarios in error tests
Use describe.each() for parameterized tests
Use toMatchSnapshot() for snapshot testing
Use beforeAll(), afterEach(), beforeEach() for setup/teardown in tests
Track resources (servers, clients) in arrays for cleanup in afterEach()

Files:

  • test/regression/issue/25579.test.ts
test/regression/issue/**/*.test.ts

📄 CodeRabbit inference engine (test/CLAUDE.md)

Regression tests for specific issues go in /test/regression/issue/${issueNumber}.test.ts. Do not put tests without issue numbers in the regression directory

Files:

  • test/regression/issue/25579.test.ts
**/*.test.ts?(x)

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.test.ts?(x): Never use bun test directly - always use bun bd test to run tests with debug build changes
For single-file tests, prefer -e flag over tempDir
For multi-file tests, prefer tempDir and Bun.spawn over single-file tests
Use normalizeBunSnapshot to normalize snapshot output of tests
Never write tests that check for 'panic', 'uncaught exception', or similar strings in test output
Use tempDir from harness to create temporary directories - do not use tmpdirSync or fs.mkdtempSync
When spawning processes in tests, expect stdout before expecting exit code for more useful error messages on test failure
Do not write flaky tests - do not use setTimeout in tests; instead await the condition to be met
Verify tests fail with USE_SYSTEM_BUN=1 bun test <file> and pass with bun bd test <file> - tests are invalid if they pass with USE_SYSTEM_BUN=1
Test files must end with .test.ts or .test.tsx
Avoid shell commands like find or grep in tests - use Bun's Glob and built-in tools instead

Files:

  • test/regression/issue/25579.test.ts
test/regression/issue/*.test.ts

📄 CodeRabbit inference engine (CLAUDE.md)

Place regression tests for specific GitHub issues in test/regression/issue/${issueNumber}.test.ts with real issue numbers only

Files:

  • test/regression/issue/25579.test.ts
test/**/*.test.ts?(x)

📄 CodeRabbit inference engine (CLAUDE.md)

Always use port: 0 in tests - do not hardcode ports or use custom random port number functions

Files:

  • test/regression/issue/25579.test.ts
🧠 Learnings (21)
📚 Learning: 2025-11-24T18:37:30.259Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:30.259Z
Learning: Applies to test/regression/issue/**/*.test.ts : Regression tests for specific issues go in `/test/regression/issue/${issueNumber}.test.ts`. Do not put tests without issue numbers in the regression directory

Applied to files:

  • test/regression/issue/25579.test.ts
📚 Learning: 2025-11-24T18:36:59.706Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:36:59.706Z
Learning: Applies to src/bun.js/bindings/v8/test/v8/v8.test.ts : Add corresponding test cases to test/v8/v8.test.ts using checkSameOutput() function to compare Node.js and Bun output

Applied to files:

  • test/regression/issue/25579.test.ts
📚 Learning: 2025-12-16T00:21:32.179Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-16T00:21:32.179Z
Learning: Applies to test/regression/issue/*.test.ts : Place regression tests for specific GitHub issues in `test/regression/issue/${issueNumber}.test.ts` with real issue numbers only

Applied to files:

  • test/regression/issue/25579.test.ts
📚 Learning: 2025-12-16T00:21:32.179Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-16T00:21:32.179Z
Learning: Applies to **/*.test.ts?(x) : For multi-file tests, prefer `tempDir` and `Bun.spawn` over single-file tests

Applied to files:

  • test/regression/issue/25579.test.ts
📚 Learning: 2025-10-19T02:44:46.354Z
Learnt from: theshadow27
Repo: oven-sh/bun PR: 23798
File: packages/bun-otel/context-propagation.test.ts:1-1
Timestamp: 2025-10-19T02:44:46.354Z
Learning: In the Bun repository, standalone packages under packages/ (e.g., bun-vscode, bun-inspector-protocol, bun-plugin-yaml, bun-plugin-svelte, bun-debug-adapter-protocol, bun-otel) co-locate their tests with package source code using *.test.ts files. This follows standard npm/monorepo patterns. The test/ directory hierarchy (test/js/bun/, test/cli/, test/js/node/) is reserved for testing Bun's core runtime APIs and built-in functionality, not standalone packages.

Applied to files:

  • test/regression/issue/25579.test.ts
📚 Learning: 2025-12-16T00:21:32.179Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-16T00:21:32.179Z
Learning: Applies to **/*.test.ts?(x) : Verify tests fail with `USE_SYSTEM_BUN=1 bun test <file>` and pass with `bun bd test <file>` - tests are invalid if they pass with USE_SYSTEM_BUN=1

Applied to files:

  • test/regression/issue/25579.test.ts
📚 Learning: 2025-11-24T18:37:30.259Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:30.259Z
Learning: Applies to test/**/*.test.{ts,js,jsx,tsx,mjs,cjs} : Use `tempDir()` from harness to create temporary directories with files for multi-file tests instead of creating files manually

Applied to files:

  • test/regression/issue/25579.test.ts
📚 Learning: 2025-11-24T18:37:30.259Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:30.259Z
Learning: Applies to test/**/*.test.{ts,js,jsx,tsx,mjs,cjs} : When spawning Bun processes in tests, use `bunExe` and `bunEnv` from `harness` to ensure the same build of Bun is used and debug logging is silenced

Applied to files:

  • test/regression/issue/25579.test.ts
📚 Learning: 2025-11-24T18:37:30.259Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:30.259Z
Learning: Applies to test/**/*.test.{ts,js,jsx,tsx,mjs,cjs} : Use `-e` flag for single-file tests when spawning Bun processes

Applied to files:

  • test/regression/issue/25579.test.ts
📚 Learning: 2025-12-16T00:21:32.179Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-16T00:21:32.179Z
Learning: Applies to **/*.test.ts?(x) : Never use `bun test` directly - always use `bun bd test` to run tests with debug build changes

Applied to files:

  • test/regression/issue/25579.test.ts
📚 Learning: 2025-12-16T00:21:32.179Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-16T00:21:32.179Z
Learning: Applies to **/*.test.ts?(x) : Avoid shell commands like `find` or `grep` in tests - use Bun's Glob and built-in tools instead

Applied to files:

  • test/regression/issue/25579.test.ts
📚 Learning: 2025-11-24T18:37:30.259Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:30.259Z
Learning: Applies to test/**/*.test.{ts,js,jsx,tsx,mjs,cjs} : Prefer concurrent tests over sequential tests using `test.concurrent` or `describe.concurrent` when multiple tests spawn processes or write files, unless it's very difficult to make them concurrent

Applied to files:

  • test/regression/issue/25579.test.ts
📚 Learning: 2025-12-16T00:21:32.179Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-16T00:21:32.179Z
Learning: Applies to **/*.test.ts?(x) : For single-file tests, prefer `-e` flag over `tempDir`

Applied to files:

  • test/regression/issue/25579.test.ts
📚 Learning: 2025-11-24T18:37:30.259Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:30.259Z
Learning: Applies to test/**/*.test.{ts,js,jsx,tsx,mjs,cjs} : Always use `await using` or `using` to ensure proper resource cleanup in tests for APIs like Bun.listen, Bun.connect, Bun.spawn, Bun.serve, etc

Applied to files:

  • test/regression/issue/25579.test.ts
📚 Learning: 2025-11-24T18:37:30.259Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:30.259Z
Learning: Applies to test/js/node/test/{parallel,sequential}/*.js : For test/js/node/test/{parallel,sequential}/*.js files without a .test extension, use `bun bd <file>` instead of `bun bd test <file>` since these expect exit code 0 and don't use bun's test runner

Applied to files:

  • test/regression/issue/25579.test.ts
📚 Learning: 2025-10-26T01:32:04.844Z
Learnt from: Jarred-Sumner
Repo: oven-sh/bun PR: 24082
File: test/cli/test/coverage.test.ts:60-112
Timestamp: 2025-10-26T01:32:04.844Z
Learning: In the Bun repository test files (test/cli/test/*.test.ts), when spawning Bun CLI commands with Bun.spawnSync for testing, prefer using stdio: ["inherit", "inherit", "inherit"] to inherit stdio streams rather than piping them.

Applied to files:

  • test/regression/issue/25579.test.ts
📚 Learning: 2025-12-16T00:21:32.179Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-16T00:21:32.179Z
Learning: Applies to **/*.test.ts?(x) : When spawning processes in tests, expect stdout before expecting exit code for more useful error messages on test failure

Applied to files:

  • test/regression/issue/25579.test.ts
📚 Learning: 2025-11-24T18:37:30.259Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:30.259Z
Learning: Applies to test/**/*.test.{ts,js,jsx,tsx,mjs,cjs} : Use `describe.each()` for parameterized tests

Applied to files:

  • test/regression/issue/25579.test.ts
📚 Learning: 2025-11-07T17:28:51.204Z
Learnt from: markovejnovic
Repo: oven-sh/bun PR: 24423
File: test/js/node/path/browserify.test.js:472-475
Timestamp: 2025-11-07T17:28:51.204Z
Learning: In test files under test/js/node/path/, markovejnovic prefers functional programming patterns (e.g., reduce, map, filter chains) over imperative loops for clarity and readability.

Applied to files:

  • test/regression/issue/25579.test.ts
📚 Learning: 2025-11-06T00:58:23.965Z
Learnt from: markovejnovic
Repo: oven-sh/bun PR: 24417
File: test/js/bun/spawn/spawn.test.ts:903-918
Timestamp: 2025-11-06T00:58:23.965Z
Learning: In Bun test files, `await using` with spawn() is appropriate for long-running processes that need guaranteed cleanup on scope exit or when explicitly testing disposal behavior. For short-lived processes that exit naturally (e.g., console.log scripts), the pattern `const proc = spawn(...); await proc.exited;` is standard and more common, as evidenced by 24 instances vs 4 `await using` instances in test/js/bun/spawn/spawn.test.ts.

Applied to files:

  • test/regression/issue/25579.test.ts
📚 Learning: 2025-11-24T18:37:30.259Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:30.259Z
Learning: Applies to test/**/*.test.{ts,js,jsx,tsx,mjs,cjs} : Use `bun:test` with files that end in `*.test.{ts,js,jsx,tsx,mjs,cjs}`

Applied to files:

  • test/regression/issue/25579.test.ts
🔇 Additional comments (2)
test/regression/issue/25579.test.ts (2)

1-8: LGTM! Proper regression test structure.

The file header correctly documents the issue being tested and links to the GitHub issue. The import follows guidelines by using bun:test.


9-35: Tests correctly validate the regression fix.

The test suite appropriately covers the reported issue #25579 by testing:

  • Current directory references (., ./, '', and no argument)
  • Absolute paths with platform-specific handling for macOS symlinks

The functional logic is correct and the tests will properly catch regressions.

Note: Previous reviews suggested making tests concurrent and using parameterized tests. These remain valid optional improvements for better performance and maintainability, per coding guidelines.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Disabled knowledge base sources:

  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 2247c38 and 73d3600.

📒 Files selected for processing (2)
  • src/js/builtins/shell.ts
  • test/regression/issue/25579.test.ts
🧰 Additional context used
📓 Path-based instructions (7)
src/js/{builtins,node,bun,thirdparty,internal}/**/*.{ts,js}

📄 CodeRabbit inference engine (src/js/CLAUDE.md)

src/js/{builtins,node,bun,thirdparty,internal}/**/*.{ts,js}: Use .$call() and .$apply() instead of .call() and .apply() to prevent user tampering with function invocation
Use string literal require() statements only; dynamic requires are not permitted
Export modules using export default { ... } syntax; modules are NOT ES modules
Use JSC intrinsics (prefixed with $) such as $Array.from(), $isCallable(), and $newArrayWithSize() for performance-critical operations
Use private globals and methods with $ prefix (e.g., $Array, map.$set()) instead of public JavaScript globals
Use $debug() for debug logging and $assert() for assertions; both are stripped in release builds
Validate function arguments using validators from internal/validators and throw $ERR_* error codes for invalid arguments
Use process.platform and process.arch for platform detection; these values are inlined and dead-code eliminated at build time

Files:

  • src/js/builtins/shell.ts
src/js/{builtins,node,bun,thirdparty,internal}/**/*.ts

📄 CodeRabbit inference engine (src/js/CLAUDE.md)

Builtin functions must include this parameter typing in TypeScript to enable direct method binding in C++

Files:

  • src/js/builtins/shell.ts
test/**/*.test.{ts,js,jsx,tsx,mjs,cjs}

📄 CodeRabbit inference engine (test/CLAUDE.md)

test/**/*.test.{ts,js,jsx,tsx,mjs,cjs}: Use bun:test with files that end in *.test.{ts,js,jsx,tsx,mjs,cjs}
Do not write flaky tests. Never wait for time to pass in tests; always wait for the condition to be met instead of using an arbitrary amount of time
Never use hardcoded port numbers in tests. Always use port: 0 to get a random port
Prefer concurrent tests over sequential tests using test.concurrent or describe.concurrent when multiple tests spawn processes or write files, unless it's very difficult to make them concurrent
When spawning Bun processes in tests, use bunExe and bunEnv from harness to ensure the same build of Bun is used and debug logging is silenced
Use -e flag for single-file tests when spawning Bun processes
Use tempDir() from harness to create temporary directories with files for multi-file tests instead of creating files manually
Prefer async/await over callbacks in tests
When callbacks must be used and it's just a single callback, use Promise.withResolvers to create a promise that can be resolved or rejected from a callback
Do not set a timeout on tests. Bun already has timeouts
Use Buffer.alloc(count, fill).toString() instead of 'A'.repeat(count) to create repetitive strings in tests, as ''.repeat is very slow in debug JavaScriptCore builds
Use describe blocks for grouping related tests
Always use await using or using to ensure proper resource cleanup in tests for APIs like Bun.listen, Bun.connect, Bun.spawn, Bun.serve, etc
Always check exit codes and test error scenarios in error tests
Use describe.each() for parameterized tests
Use toMatchSnapshot() for snapshot testing
Use beforeAll(), afterEach(), beforeEach() for setup/teardown in tests
Track resources (servers, clients) in arrays for cleanup in afterEach()

Files:

  • test/regression/issue/25579.test.ts
test/regression/issue/**/*.test.ts

📄 CodeRabbit inference engine (test/CLAUDE.md)

Regression tests for specific issues go in /test/regression/issue/${issueNumber}.test.ts. Do not put tests without issue numbers in the regression directory

Files:

  • test/regression/issue/25579.test.ts
**/*.test.ts?(x)

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.test.ts?(x): Never use bun test directly - always use bun bd test to run tests with debug build changes
For single-file tests, prefer -e flag over tempDir
For multi-file tests, prefer tempDir and Bun.spawn over single-file tests
Use normalizeBunSnapshot to normalize snapshot output of tests
Never write tests that check for 'panic', 'uncaught exception', or similar strings in test output
Use tempDir from harness to create temporary directories - do not use tmpdirSync or fs.mkdtempSync
When spawning processes in tests, expect stdout before expecting exit code for more useful error messages on test failure
Do not write flaky tests - do not use setTimeout in tests; instead await the condition to be met
Verify tests fail with USE_SYSTEM_BUN=1 bun test <file> and pass with bun bd test <file> - tests are invalid if they pass with USE_SYSTEM_BUN=1
Test files must end with .test.ts or .test.tsx
Avoid shell commands like find or grep in tests - use Bun's Glob and built-in tools instead

Files:

  • test/regression/issue/25579.test.ts
test/regression/issue/*.test.ts

📄 CodeRabbit inference engine (CLAUDE.md)

Place regression tests for specific GitHub issues in test/regression/issue/${issueNumber}.test.ts with real issue numbers only

Files:

  • test/regression/issue/25579.test.ts
test/**/*.test.ts?(x)

📄 CodeRabbit inference engine (CLAUDE.md)

Always use port: 0 in tests - do not hardcode ports or use custom random port number functions

Files:

  • test/regression/issue/25579.test.ts
🧠 Learnings (19)
📚 Learning: 2025-11-24T18:37:11.466Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/js/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:11.466Z
Learning: Applies to src/js/{builtins,node,bun,thirdparty,internal}/**/*.{ts,js} : Use `process.platform` and `process.arch` for platform detection; these values are inlined and dead-code eliminated at build time

Applied to files:

  • src/js/builtins/shell.ts
📚 Learning: 2025-11-24T18:37:11.466Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/js/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:11.466Z
Learning: Applies to src/js/{builtins,node,bun,thirdparty,internal}/**/*.{ts,js} : Export modules using `export default { ... }` syntax; modules are NOT ES modules

Applied to files:

  • src/js/builtins/shell.ts
📚 Learning: 2025-11-24T18:37:11.466Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/js/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:11.466Z
Learning: Applies to src/js/{builtins,node,bun,thirdparty,internal}/**/*.{ts,js} : Use `.$call()` and `.$apply()` instead of `.call()` and `.apply()` to prevent user tampering with function invocation

Applied to files:

  • src/js/builtins/shell.ts
📚 Learning: 2025-11-24T18:37:11.466Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/js/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:11.466Z
Learning: Applies to src/js/{builtins,node,bun,thirdparty,internal}/**/*.{ts,js} : Use private globals and methods with `$` prefix (e.g., `$Array`, `map.$set()`) instead of public JavaScript globals

Applied to files:

  • src/js/builtins/shell.ts
📚 Learning: 2025-11-24T18:37:30.259Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:30.259Z
Learning: Applies to test/regression/issue/**/*.test.ts : Regression tests for specific issues go in `/test/regression/issue/${issueNumber}.test.ts`. Do not put tests without issue numbers in the regression directory

Applied to files:

  • test/regression/issue/25579.test.ts
📚 Learning: 2025-11-24T18:36:59.706Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:36:59.706Z
Learning: Applies to src/bun.js/bindings/v8/test/v8/v8.test.ts : Add corresponding test cases to test/v8/v8.test.ts using checkSameOutput() function to compare Node.js and Bun output

Applied to files:

  • test/regression/issue/25579.test.ts
📚 Learning: 2025-12-16T00:21:32.179Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-16T00:21:32.179Z
Learning: Applies to test/regression/issue/*.test.ts : Place regression tests for specific GitHub issues in `test/regression/issue/${issueNumber}.test.ts` with real issue numbers only

Applied to files:

  • test/regression/issue/25579.test.ts
📚 Learning: 2025-12-16T00:21:32.179Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-16T00:21:32.179Z
Learning: Applies to **/*.test.ts?(x) : For multi-file tests, prefer `tempDir` and `Bun.spawn` over single-file tests

Applied to files:

  • test/regression/issue/25579.test.ts
📚 Learning: 2025-10-19T02:44:46.354Z
Learnt from: theshadow27
Repo: oven-sh/bun PR: 23798
File: packages/bun-otel/context-propagation.test.ts:1-1
Timestamp: 2025-10-19T02:44:46.354Z
Learning: In the Bun repository, standalone packages under packages/ (e.g., bun-vscode, bun-inspector-protocol, bun-plugin-yaml, bun-plugin-svelte, bun-debug-adapter-protocol, bun-otel) co-locate their tests with package source code using *.test.ts files. This follows standard npm/monorepo patterns. The test/ directory hierarchy (test/js/bun/, test/cli/, test/js/node/) is reserved for testing Bun's core runtime APIs and built-in functionality, not standalone packages.

Applied to files:

  • test/regression/issue/25579.test.ts
📚 Learning: 2025-11-24T18:37:30.259Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:30.259Z
Learning: Applies to test/**/*.test.{ts,js,jsx,tsx,mjs,cjs} : When spawning Bun processes in tests, use `bunExe` and `bunEnv` from `harness` to ensure the same build of Bun is used and debug logging is silenced

Applied to files:

  • test/regression/issue/25579.test.ts
📚 Learning: 2025-11-24T18:37:30.259Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:30.259Z
Learning: Applies to test/**/*.test.{ts,js,jsx,tsx,mjs,cjs} : Use `tempDir()` from harness to create temporary directories with files for multi-file tests instead of creating files manually

Applied to files:

  • test/regression/issue/25579.test.ts
📚 Learning: 2025-12-16T00:21:32.179Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-16T00:21:32.179Z
Learning: Applies to **/*.test.ts?(x) : Verify tests fail with `USE_SYSTEM_BUN=1 bun test <file>` and pass with `bun bd test <file>` - tests are invalid if they pass with USE_SYSTEM_BUN=1

Applied to files:

  • test/regression/issue/25579.test.ts
📚 Learning: 2025-12-16T00:21:32.179Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-16T00:21:32.179Z
Learning: Applies to **/*.test.ts?(x) : Never use `bun test` directly - always use `bun bd test` to run tests with debug build changes

Applied to files:

  • test/regression/issue/25579.test.ts
📚 Learning: 2025-11-24T18:37:30.259Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:30.259Z
Learning: Applies to test/**/*.test.{ts,js,jsx,tsx,mjs,cjs} : Use `-e` flag for single-file tests when spawning Bun processes

Applied to files:

  • test/regression/issue/25579.test.ts
📚 Learning: 2025-12-16T00:21:32.179Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-16T00:21:32.179Z
Learning: Applies to **/*.test.ts?(x) : Avoid shell commands like `find` or `grep` in tests - use Bun's Glob and built-in tools instead

Applied to files:

  • test/regression/issue/25579.test.ts
📚 Learning: 2025-11-24T18:37:30.259Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:30.259Z
Learning: Applies to test/**/*.test.{ts,js,jsx,tsx,mjs,cjs} : Use `bun:test` with files that end in `*.test.{ts,js,jsx,tsx,mjs,cjs}`

Applied to files:

  • test/regression/issue/25579.test.ts
📚 Learning: 2025-11-24T18:37:30.259Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:30.259Z
Learning: Applies to test/js/node/test/{parallel,sequential}/*.js : For test/js/node/test/{parallel,sequential}/*.js files without a .test extension, use `bun bd <file>` instead of `bun bd test <file>` since these expect exit code 0 and don't use bun's test runner

Applied to files:

  • test/regression/issue/25579.test.ts
📚 Learning: 2025-10-26T01:32:04.844Z
Learnt from: Jarred-Sumner
Repo: oven-sh/bun PR: 24082
File: test/cli/test/coverage.test.ts:60-112
Timestamp: 2025-10-26T01:32:04.844Z
Learning: In the Bun repository test files (test/cli/test/*.test.ts), when spawning Bun CLI commands with Bun.spawnSync for testing, prefer using stdio: ["inherit", "inherit", "inherit"] to inherit stdio streams rather than piping them.

Applied to files:

  • test/regression/issue/25579.test.ts
📚 Learning: 2025-11-24T18:37:30.259Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:30.259Z
Learning: Unit tests for specific features are organized by module (e.g., `/test/js/bun/`, `/test/js/node/`)

Applied to files:

  • test/regression/issue/25579.test.ts
🔇 Additional comments (3)
test/regression/issue/25579.test.ts (1)

30-34: Good platform-aware testing for macOS symlink behavior.

The test correctly handles the macOS-specific case where /tmp is symlinked to /private/tmp. This prevents flaky test failures on macOS.

src/js/builtins/shell.ts (2)

151-152: LGTM! Proper fallback to process.cwd() fixes undefined cwd issue.

The addition of ?? process.cwd() correctly ensures that when defaultCwd is undefined, the current working directory is used instead of leaving newCwd as undefined. This properly resolves the issue where $.cwd('.'), $.cwd('./'), and $.cwd('') would result in undefined paths.


279-280: LGTM! Consistent fix applied in ShellPrototype.

This change mirrors the fix in ShellPromise.cwd() at line 152, ensuring consistent behavior across both code paths. The fallback to process.cwd() when defaultCwd is undefined correctly handles current-directory indicators.

Comment on lines +9 to +35
describe("Shell cwd with dot", () => {
test("$.cwd('.') should work and use current directory", async () => {
const result = await Bun.$`pwd`.cwd(".").text();
expect(result.trim()).toBe(process.cwd());
});

test("$.cwd('./') should work and use current directory", async () => {
const result = await Bun.$`pwd`.cwd("./").text();
expect(result.trim()).toBe(process.cwd());
});

test("$.cwd('') should work and use current directory", async () => {
const result = await Bun.$`pwd`.cwd("").text();
expect(result.trim()).toBe(process.cwd());
});

test("$.cwd() without argument should work", async () => {
const result = await Bun.$`pwd`.cwd().text();
expect(result.trim()).toBe(process.cwd());
});

test("$.cwd('/tmp') should work with absolute path", async () => {
const result = await Bun.$`pwd`.cwd("/tmp").text();
// On macOS, /tmp is symlinked to /private/tmp
expect(result.trim()).toMatch(/^(\/tmp|\/private\/tmp)$/);
});
});
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Consider making tests concurrent for better performance.

These tests are independent and don't modify shared state, so they can safely run concurrently. Consider using describe.concurrent or individual test.concurrent calls.

🔎 Optional refactor using concurrent tests
-describe("Shell cwd with dot", () => {
+describe.concurrent("Shell cwd with dot", () => {
   test("$.cwd('.') should work and use current directory", async () => {

As per coding guidelines, prefer concurrent tests when multiple tests spawn processes (Bun.$ internally spawns pwd processes).

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
describe("Shell cwd with dot", () => {
test("$.cwd('.') should work and use current directory", async () => {
const result = await Bun.$`pwd`.cwd(".").text();
expect(result.trim()).toBe(process.cwd());
});
test("$.cwd('./') should work and use current directory", async () => {
const result = await Bun.$`pwd`.cwd("./").text();
expect(result.trim()).toBe(process.cwd());
});
test("$.cwd('') should work and use current directory", async () => {
const result = await Bun.$`pwd`.cwd("").text();
expect(result.trim()).toBe(process.cwd());
});
test("$.cwd() without argument should work", async () => {
const result = await Bun.$`pwd`.cwd().text();
expect(result.trim()).toBe(process.cwd());
});
test("$.cwd('/tmp') should work with absolute path", async () => {
const result = await Bun.$`pwd`.cwd("/tmp").text();
// On macOS, /tmp is symlinked to /private/tmp
expect(result.trim()).toMatch(/^(\/tmp|\/private\/tmp)$/);
});
});
describe.concurrent("Shell cwd with dot", () => {
test("$.cwd('.') should work and use current directory", async () => {
const result = await Bun.$`pwd`.cwd(".").text();
expect(result.trim()).toBe(process.cwd());
});
test("$.cwd('./') should work and use current directory", async () => {
const result = await Bun.$`pwd`.cwd("./").text();
expect(result.trim()).toBe(process.cwd());
});
test("$.cwd('') should work and use current directory", async () => {
const result = await Bun.$`pwd`.cwd("").text();
expect(result.trim()).toBe(process.cwd());
});
test("$.cwd() without argument should work", async () => {
const result = await Bun.$`pwd`.cwd().text();
expect(result.trim()).toBe(process.cwd());
});
test("$.cwd('/tmp') should work with absolute path", async () => {
const result = await Bun.$`pwd`.cwd("/tmp").text();
// On macOS, /tmp is symlinked to /private/tmp
expect(result.trim()).toMatch(/^(\/tmp|\/private\/tmp)$/);
});
});
🤖 Prompt for AI Agents
In test/regression/issue/25579.test.ts around lines 9-35, the tests are
independent and should run concurrently for performance; change the test suite
to run in parallel by replacing describe("Shell cwd with dot", ...) with
describe.concurrent("Shell cwd with dot", ...) or mark each test as
test.concurrent(...) so the Bun.$ spawned processes run concurrently without
shared-state issues.

Comment on lines +10 to +28
test("$.cwd('.') should work and use current directory", async () => {
const result = await Bun.$`pwd`.cwd(".").text();
expect(result.trim()).toBe(process.cwd());
});

test("$.cwd('./') should work and use current directory", async () => {
const result = await Bun.$`pwd`.cwd("./").text();
expect(result.trim()).toBe(process.cwd());
});

test("$.cwd('') should work and use current directory", async () => {
const result = await Bun.$`pwd`.cwd("").text();
expect(result.trim()).toBe(process.cwd());
});

test("$.cwd() without argument should work", async () => {
const result = await Bun.$`pwd`.cwd().text();
expect(result.trim()).toBe(process.cwd());
});
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Consider using parameterized tests for similar test cases.

The first four tests follow an identical pattern with different input values. Using describe.each() would reduce duplication and make the test suite more maintainable.

🔎 Optional refactor using describe.each
-  test("$.cwd('.') should work and use current directory", async () => {
-    const result = await Bun.$`pwd`.cwd(".").text();
-    expect(result.trim()).toBe(process.cwd());
-  });
-
-  test("$.cwd('./') should work and use current directory", async () => {
-    const result = await Bun.$`pwd`.cwd("./").text();
-    expect(result.trim()).toBe(process.cwd());
-  });
-
-  test("$.cwd('') should work and use current directory", async () => {
-    const result = await Bun.$`pwd`.cwd("").text();
-    expect(result.trim()).toBe(process.cwd());
-  });
-
-  test("$.cwd() without argument should work", async () => {
-    const result = await Bun.$`pwd`.cwd().text();
-    expect(result.trim()).toBe(process.cwd());
-  });
+  describe.each([
+    { input: ".", desc: "'.'" },
+    { input: "./", desc: "'./'" },
+    { input: "", desc: "''" },
+    { input: undefined, desc: "without argument" },
+  ])("$.cwd($desc) should work and use current directory", ({ input }) => {
+    test("", async () => {
+      const result = input === undefined 
+        ? await Bun.$`pwd`.cwd().text()
+        : await Bun.$`pwd`.cwd(input).text();
+      expect(result.trim()).toBe(process.cwd());
+    });
+  });
🤖 Prompt for AI Agents
In test/regression/issue/25579.test.ts around lines 10 to 28, the four nearly
identical tests that call Bun.$`pwd`.cwd(...) should be consolidated into a
single parameterized test to reduce duplication; replace the four separate test
blocks with a single describe.each or test.each that iterates over the input
values [".", "./", "", undefined] (use undefined to represent no-argument
.cwd()), run Bun.$`pwd`.cwd(value) (or .cwd() when value is undefined), get
.text(), and assert that result.trim() equals process.cwd(); preserve the
existing trimming and assertion logic and keep individual case labels for
clarity.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

$ls.cwd('.') does not work

1 participant