fix: respect BUN_JSC_forceRAMSize in process.constrainedMemory()#25489
Open
KushalMeghani1644 wants to merge 14 commits intooven-sh:mainfrom
Open
fix: respect BUN_JSC_forceRAMSize in process.constrainedMemory()#25489KushalMeghani1644 wants to merge 14 commits intooven-sh:mainfrom
KushalMeghani1644 wants to merge 14 commits intooven-sh:mainfrom
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThe PR makes memory-sizing use Changes
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/bun.js/bindings/BunProcess.cpp (1)
1893-1934:process.reportconsistency:totalMemoryhonors forced RAM, butmemoryLimitremains0on non-Windows.
If the objective includes “memory reporting inprocess.reportrespectsBUN_JSC_forceRAMSize”, non-Windows should likely setjavascriptHeap.memoryLimit(and possiblyusedMemory) consistently with the Windows report implementation.- heap->putDirect(vm, JSC::Identifier::fromString(vm, "totalMemory"_s), JSC::jsNumber(JSC::Options::forceRAMSize() ? JSC::Options::forceRAMSize() : WTF::ramSize()), 0); + const auto forcedRAMSize = JSC::Options::forceRAMSize(); + const auto reportedRAMSize = forcedRAMSize ? forcedRAMSize : WTF::ramSize(); + heap->putDirect(vm, JSC::Identifier::fromString(vm, "totalMemory"_s), JSC::jsNumber(reportedRAMSize), 0); @@ - heap->putDirect(vm, JSC::Identifier::fromString(vm, "memoryLimit"_s), jsNumber(0), 0); + heap->putDirect(vm, JSC::Identifier::fromString(vm, "memoryLimit"_s), jsNumber(reportedRAMSize), 0);
📜 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.
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (4)
package.json(1 hunks)src/bun.js/bindings/BunProcess.cpp(3 hunks)src/bun.js/bindings/BunProcessReportObjectWindows.cpp(2 hunks)test/js/node/process/process.test.js(1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
src/**/*.{cpp,zig}
📄 CodeRabbit inference engine (.cursor/rules/building-bun.mdc)
src/**/*.{cpp,zig}: Usebun bdorbun run build:debugto build debug versions for C++ and Zig source files; creates debug build at./build/debug/bun-debug
Run tests usingbun bd test <test-file>with the debug build; never usebun testdirectly as it will not include your changes
Execute files usingbun bd <file> <...args>; never usebun <file>directly as it will not include your changes
Enable debug logs for specific scopes usingBUN_DEBUG_$(SCOPE)=1environment variable
Code generation happens automatically as part of the build process; no manual code generation commands are required
Files:
src/bun.js/bindings/BunProcessReportObjectWindows.cppsrc/bun.js/bindings/BunProcess.cpp
src/bun.js/bindings/**/*.cpp
📄 CodeRabbit inference engine (CLAUDE.md)
src/bun.js/bindings/**/*.cpp: JavaScript class implementations in C++ should create three classes if there's a public constructor:Foo(JSDestructibleObject),FooPrototype(JSNonFinalObject), andFooConstructor(InternalFunction)
Define properties in C++ JavaScript classes using HashTableValue arrays
Add iso subspaces for C++ JavaScript classes with C++ fields
Cache structures in ZigGlobalObject for C++ JavaScript classes
Files:
src/bun.js/bindings/BunProcessReportObjectWindows.cppsrc/bun.js/bindings/BunProcess.cpp
test/**/*.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (.cursor/rules/writing-tests.mdc)
test/**/*.{js,ts,jsx,tsx}: Write tests as JavaScript and TypeScript files using Jest-style APIs (test,describe,expect) and import frombun:test
Usetest.eachand data-driven tests to reduce boilerplate when testing multiple similar cases
Files:
test/js/node/process/process.test.js
test/**/*.test.{ts,js,jsx,tsx,mjs,cjs}
📄 CodeRabbit inference engine (test/CLAUDE.md)
test/**/*.test.{ts,js,jsx,tsx,mjs,cjs}: Usebun:testwith 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 useport: 0to get a random port
Prefer concurrent tests over sequential tests usingtest.concurrentordescribe.concurrentwhen multiple tests spawn processes or write files, unless it's very difficult to make them concurrent
When spawning Bun processes in tests, usebunExeandbunEnvfromharnessto ensure the same build of Bun is used and debug logging is silenced
Use-eflag for single-file tests when spawning Bun processes
UsetempDir()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, usePromise.withResolversto create a promise that can be resolved or rejected from a callback
Do not set a timeout on tests. Bun already has timeouts
UseBuffer.alloc(count, fill).toString()instead of'A'.repeat(count)to create repetitive strings in tests, as ''.repeat is very slow in debug JavaScriptCore builds
Usedescribeblocks for grouping related tests
Always useawait usingorusingto 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
Usedescribe.each()for parameterized tests
UsetoMatchSnapshot()for snapshot testing
UsebeforeAll(),afterEach(),beforeEach()for setup/teardown in tests
Track resources (servers, clients) in arrays for cleanup inafterEach()
Files:
test/js/node/process/process.test.js
🧠 Learnings (43)
📓 Common learnings
Learnt from: Jarred-Sumner
Repo: oven-sh/bun PR: 22279
File: test/js/web/structured-clone-fastpath.test.ts:12-12
Timestamp: 2025-08-31T09:08:12.104Z
Learning: In Bun, process.memoryUsage.rss() is called as a direct function, not as process.memoryUsage().rss like in Node.js. This is the correct API shape for Bun.
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
📚 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/V8*.h : Add BUN_EXPORT visibility attribute to all public V8 API functions to ensure proper symbol export across platforms
Applied to files:
src/bun.js/bindings/BunProcessReportObjectWindows.cppsrc/bun.js/bindings/BunProcess.cpp
📚 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/V8*.cpp : Use JSC::WriteBarrier for heap-allocated references in V8 objects and implement visitChildren() for custom heap objects to support garbage collection
Applied to files:
src/bun.js/bindings/BunProcessReportObjectWindows.cppsrc/bun.js/bindings/BunProcess.cpp
📚 Learning: 2025-11-24T18:37:47.899Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/AGENTS.md:0-0
Timestamp: 2025-11-24T18:37:47.899Z
Learning: Applies to src/bun.js/bindings/v8/**/<UNKNOWN> : <UNKNOWN>
Applied to files:
src/bun.js/bindings/BunProcessReportObjectWindows.cppsrc/bun.js/bindings/BunProcess.cpp
📚 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/V8*.h : Create V8 class headers with .h extension following the pattern V8ClassName.h that include pragma once, v8.h, V8Local.h, V8Isolate.h, and declare classes extending from Data with BUN_EXPORT static methods
Applied to files:
src/bun.js/bindings/BunProcessReportObjectWindows.cppsrc/bun.js/bindings/BunProcess.cpp
📚 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/bun.js/bindings/BunProcessReportObjectWindows.cpppackage.jsonsrc/bun.js/bindings/BunProcess.cpp
📚 Learning: 2025-08-31T09:08:12.104Z
Learnt from: Jarred-Sumner
Repo: oven-sh/bun PR: 22279
File: test/js/web/structured-clone-fastpath.test.ts:12-12
Timestamp: 2025-08-31T09:08:12.104Z
Learning: In Bun, process.memoryUsage.rss() is called as a direct function, not as process.memoryUsage().rss like in Node.js. This is the correct API shape for Bun.
Applied to files:
src/bun.js/bindings/BunProcessReportObjectWindows.cpptest/js/node/process/process.test.jssrc/bun.js/bindings/BunProcess.cpp
📚 Learning: 2025-12-02T05:59:51.485Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-02T05:59:51.485Z
Learning: Applies to src/bun.js/bindings/**/*.cpp : Add iso subspaces for C++ JavaScript classes with C++ fields
Applied to files:
src/bun.js/bindings/BunProcessReportObjectWindows.cppsrc/bun.js/bindings/BunProcess.cpp
📚 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/V8*.cpp : Create V8 class implementations with .cpp extension following the pattern V8ClassName.cpp that include the header, v8_compatibility_assertions.h, use ASSERT_V8_TYPE_LAYOUT_MATCHES macro, and implement methods using isolate->currentHandleScope()->createLocal<T>() for handle creation
Applied to files:
src/bun.js/bindings/BunProcessReportObjectWindows.cppsrc/bun.js/bindings/BunProcess.cpp
📚 Learning: 2025-12-02T05:59:51.485Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-02T05:59:51.485Z
Learning: Applies to src/bun.js/bindings/**/*.cpp : JavaScript class implementations in C++ should create three classes if there's a public constructor: `Foo` (JSDestructibleObject), `FooPrototype` (JSNonFinalObject), and `FooConstructor` (InternalFunction)
Applied to files:
src/bun.js/bindings/BunProcessReportObjectWindows.cppsrc/bun.js/bindings/BunProcess.cpp
📚 Learning: 2025-12-02T05:59:51.485Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-02T05:59:51.485Z
Learning: Applies to src/bun.js/bindings/**/*.cpp : Cache structures in ZigGlobalObject for C++ JavaScript classes
Applied to files:
src/bun.js/bindings/BunProcessReportObjectWindows.cppsrc/bun.js/bindings/BunProcess.cpp
📚 Learning: 2025-11-24T18:35:25.883Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/javascriptcore-class.mdc:0-0
Timestamp: 2025-11-24T18:35:25.883Z
Learning: Applies to *.cpp : Implement function definitions using `JSC_DEFINE_HOST_FUNCTION` macro, performing type checking with `jsDynamicCast`, throwing this-type errors when type checking fails, and returning encoded JSValue
Applied to files:
src/bun.js/bindings/BunProcessReportObjectWindows.cppsrc/bun.js/bindings/BunProcess.cpp
📚 Learning: 2025-11-24T18:35:25.883Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/javascriptcore-class.mdc:0-0
Timestamp: 2025-11-24T18:35:25.883Z
Learning: Applies to *.cpp : When constructing JavaScript objects, retrieve the structure from the global object using `zigGlobalObject->m_JSX509CertificateClassStructure.get(zigGlobalObject)` or similar pattern
Applied to files:
src/bun.js/bindings/BunProcessReportObjectWindows.cpp
📚 Learning: 2025-11-24T18:35:25.883Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/javascriptcore-class.mdc:0-0
Timestamp: 2025-11-24T18:35:25.883Z
Learning: Applies to *.cpp : If implementing a JavaScript class in C++ with publicly accessible Constructor and Prototype, use three classes: (1) `class Foo : public JSC::DestructibleObject` if there are C++ class members (add destructor), or use `JSC::constructEmptyObject(vm, structure)` and `putDirectOffset` if only JS properties; (2) `class FooPrototype : public JSC::JSNonFinalObject`; (3) `class FooConstructor : public JSC::InternalFunction`
Applied to files:
src/bun.js/bindings/BunProcessReportObjectWindows.cpp
📚 Learning: 2025-11-24T18:35:25.883Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/javascriptcore-class.mdc:0-0
Timestamp: 2025-11-24T18:35:25.883Z
Learning: Applies to *.cpp : Implement getter functions using `JSC_DEFINE_CUSTOM_GETTER` macro, performing type checking with `jsDynamicCast`, throwing errors for type mismatches, and returning encoded JSValue
Applied to files:
src/bun.js/bindings/BunProcessReportObjectWindows.cpp
📚 Learning: 2025-11-24T18:35:25.883Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/javascriptcore-class.mdc:0-0
Timestamp: 2025-11-24T18:35:25.883Z
Learning: Applies to *.cpp : Implement prototype classes following the pattern: inherit from `JSC::JSNonFinalObject`, implement `create()`, `subspaceFor()`, `createStructure()`, `DECLARE_INFO`, and `finishCreation()` methods
Applied to files:
src/bun.js/bindings/BunProcessReportObjectWindows.cpp
📚 Learning: 2025-11-24T18:35:25.883Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/javascriptcore-class.mdc:0-0
Timestamp: 2025-11-24T18:35:25.883Z
Learning: Applies to *.cpp : For classes, prototypes, and constructors: add `JSC::LazyClassStructure` to ZigGlobalObject.h, initialize in `GlobalObject::finishCreation()`, visit in `GlobalObject::visitChildrenImpl()`, and implement a setup function that creates prototype, constructor, and main class structures
Applied to files:
src/bun.js/bindings/BunProcessReportObjectWindows.cpp
📚 Learning: 2025-11-24T18:35:25.883Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/javascriptcore-class.mdc:0-0
Timestamp: 2025-11-24T18:35:25.883Z
Learning: Applies to *.cpp : Define properties on the prototype using `JSC_DECLARE_HOST_FUNCTION` for methods and `JSC_DECLARE_CUSTOM_GETTER`/`JSC_DECLARE_CUSTOM_SETTER` for accessors, organized in a const HashTableValue array
Applied to files:
src/bun.js/bindings/BunProcessReportObjectWindows.cppsrc/bun.js/bindings/BunProcess.cpp
📚 Learning: 2025-11-24T18:35:25.883Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/javascriptcore-class.mdc:0-0
Timestamp: 2025-11-24T18:35:25.883Z
Learning: Applies to *.cpp : For classes without Constructor, use `JSC::LazyProperty<JSGlobalObject, Structure>` instead of `JSC::LazyClassStructure` in ZigGlobalObject.h, initialize in `GlobalObject::finishCreation()`, and visit in `GlobalObject::visitChildrenImpl()`
Applied to files:
src/bun.js/bindings/BunProcessReportObjectWindows.cpp
📚 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/V8*.cpp : Use localToJSValue() to convert V8 handles to JSC values and perform JSC operations within V8 method implementations
Applied to files:
src/bun.js/bindings/BunProcessReportObjectWindows.cpp
📚 Learning: 2025-10-14T04:04:17.132Z
Learnt from: Jarred-Sumner
Repo: oven-sh/bun PR: 23636
File: src/shell/interpreter.zig:672-0
Timestamp: 2025-10-14T04:04:17.132Z
Learning: In Bun's shell interpreter (src/shell/interpreter.zig), the JavaScript garbage collector's memory reporting API only allows reporting extra memory allocation once at object creation time. Therefore, `estimated_size_for_gc` is intentionally set once during initialization and not updated afterward, even though fields like `root_io`, `vm_args_utf8`, and `export_env` may be mutated later. The `estimatedSize()` method returns this cached initial value, while `memoryCost()` computes the size on-demand.
Applied to files:
src/bun.js/bindings/BunProcessReportObjectWindows.cppsrc/bun.js/bindings/BunProcess.cpp
📚 Learning: 2025-10-14T04:04:47.894Z
Learnt from: Jarred-Sumner
Repo: oven-sh/bun PR: 23636
File: src/shell/ParsedShellScript.zig:15-0
Timestamp: 2025-10-14T04:04:47.894Z
Learning: In src/shell/ParsedShellScript.zig, the JavaScript garbage collector's memory reporting API only allows reporting extra memory allocation once at object creation time. Therefore, `estimated_size_for_gc` is intentionally set once during initialization (via `#computeEstimatedSizeForGC()`) and not updated afterward, even though fields like `args`, `export_env`, and `cwd` may be mutated by methods like `setCwd()`, `setEnv()`, and `take()`. The `estimatedSize()` method returns this cached initial value, while `memoryCost()` computes the size on-demand.
Applied to files:
src/bun.js/bindings/BunProcessReportObjectWindows.cppsrc/bun.js/bindings/BunProcess.cpp
📚 Learning: 2025-09-05T18:44:43.223Z
Learnt from: markovejnovic
Repo: oven-sh/bun PR: 21728
File: src/bun.js/bindings/bindings.cpp:6429-6432
Timestamp: 2025-09-05T18:44:43.223Z
Learning: The JSC::JSMap::size() method in JavaScriptCore returns a value that can be represented as uint32_t, and the binding functions in src/bun.js/bindings/bindings.cpp correctly use uint32_t as the return type for JSC__JSMap__size. JavaScript Maps are practically limited to 2^32 - 1 elements.
Applied to files:
src/bun.js/bindings/BunProcessReportObjectWindows.cppsrc/bun.js/bindings/BunProcess.cpp
📚 Learning: 2025-11-20T19:51:32.288Z
Learnt from: markovejnovic
Repo: oven-sh/bun PR: 24880
File: packages/bun-vscode/package.json:382-385
Timestamp: 2025-11-20T19:51:32.288Z
Learning: In the Bun repository, dependencies may be explicitly added to package.json files (even when not directly imported in code) to force version upgrades on transitive dependencies, particularly as part of Aikido security scanner remediation to ensure vulnerable transitive dependencies resolve to patched versions.
Applied to files:
package.json
📚 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: Write JS builtins for Bun's Node.js compatibility and APIs, and run `bun bd` after changes
Applied to files:
package.json
📚 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:
package.jsontest/js/node/process/process.test.js
📚 Learning: 2025-11-24T18:34:55.173Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/building-bun.mdc:0-0
Timestamp: 2025-11-24T18:34:55.173Z
Learning: Applies to src/**/*.{cpp,zig} : Use `bun bd` or `bun run build:debug` to build debug versions for C++ and Zig source files; creates debug build at `./build/debug/bun-debug`
Applied to files:
package.json
📚 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 `$debug()` for debug logging and `$assert()` for assertions; both are stripped in release builds
Applied to files:
package.json
📚 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:
package.json
📚 Learning: 2025-11-20T19:51:59.485Z
Learnt from: markovejnovic
Repo: oven-sh/bun PR: 24880
File: packages/bun-lambda/package.json:12-13
Timestamp: 2025-11-20T19:51:59.485Z
Learning: In the bun repository, explicitly adding transitive dependencies as direct dependencies in package.json is acceptable and encouraged when addressing security vulnerabilities (e.g., adding "braces": "^3.0.3" to force upgrade from vulnerable versions < 3.0.3 affected by CVE-2024-4068). This pattern ensures vulnerable transitive dependencies are upgraded even when direct dependencies haven't updated their requirements.
Applied to files:
package.json
📚 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:
package.json
📚 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/js/node/process/process.test.js
📚 Learning: 2025-11-24T18:35:39.205Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/registering-bun-modules.mdc:0-0
Timestamp: 2025-11-24T18:35:39.205Z
Learning: Add tests for new Bun runtime functionality
Applied to files:
test/js/node/process/process.test.js
📚 Learning: 2025-11-24T18:35:50.422Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/writing-tests.mdc:0-0
Timestamp: 2025-11-24T18:35:50.422Z
Learning: Applies to test/cli/**/*.{js,ts,jsx,tsx} : When testing Bun as a CLI, use the `spawn` API from `bun` with the `bunExe()` and `bunEnv` from `harness` to execute Bun commands and validate exit codes, stdout, and stderr
Applied to files:
test/js/node/process/process.test.js
📚 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/js/node/process/process.test.js
📚 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: Ensure V8 API tests compare identical C++ code output between Node.js and Bun through the test suite validation process
Applied to files:
test/js/node/process/process.test.js
📚 Learning: 2025-12-02T05:59:51.485Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-02T05:59:51.485Z
Learning: Applies to test/**/*.test.{ts,tsx} : Verify your test fails with `USE_SYSTEM_BUN=1 bun test <file>` and passes with `bun bd test <file>` - tests are not valid if they pass with `USE_SYSTEM_BUN=1`
Applied to files:
test/js/node/process/process.test.js
📚 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/js/node/process/process.test.js
📚 Learning: 2025-11-08T04:06:33.198Z
Learnt from: Jarred-Sumner
Repo: oven-sh/bun PR: 24491
File: test/js/bun/transpiler/declare-global.test.ts:17-17
Timestamp: 2025-11-08T04:06:33.198Z
Learning: In Bun test files, `await using` with Bun.spawn() is the preferred pattern for spawned processes regardless of whether they are short-lived or long-running. Do not suggest replacing `await using proc = Bun.spawn(...)` with `const proc = Bun.spawn(...); await proc.exited;`.
Applied to files:
test/js/node/process/process.test.js
📚 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/js/node/process/process.test.js
📚 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/js/node/process/process.test.js
📚 Learning: 2025-12-02T05:59:51.485Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-02T05:59:51.485Z
Learning: Applies to test/**/*.test.{ts,tsx} : For multi-file tests in Bun test suite, prefer using `tempDir` and `Bun.spawn`
Applied to files:
test/js/node/process/process.test.js
📚 Learning: 2025-12-02T05:59:51.485Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-02T05:59:51.485Z
Learning: Applies to src/bun.js/bindings/**/*.cpp : Define properties in C++ JavaScript classes using HashTableValue arrays
Applied to files:
src/bun.js/bindings/BunProcess.cpp
🧬 Code graph analysis (1)
test/js/node/process/process.test.js (1)
test/harness.ts (2)
bunExe(102-105)bunEnv(49-64)
🪛 Cppcheck (2.18.0)
src/bun.js/bindings/BunProcess.cpp
[information] 100-100: Include file
(missingIncludeSystem)
[information] Too many #ifdef configurations - cppcheck only checks 12 configurations. Use --force to check all configurations. For more details, use --enable=information.
(toomanyconfigs)
[information] Too many #ifdef configurations - cppcheck only checks 12 configurations. Use --force to check all configurations. For more details, use --enable=information.
(toomanyconfigs)
[information] Too many #ifdef configurations - cppcheck only checks 12 configurations. Use --force to check all configurations. For more details, use --enable=information.
(toomanyconfigs)
🔇 Additional comments (3)
src/bun.js/bindings/BunProcessReportObjectWindows.cpp (1)
17-17: Header addition OK; please ensure it builds across all Windows configs.Run (per repo guidelines) a debug build that includes this TU, plus the relevant tests.
src/bun.js/bindings/BunProcess.cpp (2)
3004-3008:process.constrainedMemory()fix looks correct and matches PR intent.
Cleanly prefersJSC::Options::forceRAMSize()and falls back toWTF::ramSize().
100-101: The<JavaScriptCore/Options.h>header is already included in the codebase and used in multiple established binding files (RegularExpression.cpp,JSNodePerformanceHooksHistogramConstructor.cpp,BunProcessReportObjectWindows.cpp). This is not a new or untested dependency—it's a standard JSC header already integrated into the build system. No compilation concerns are warranted.
package.json
Outdated
| "@lezer/cpp": "^1.1.3", | ||
| "@types/bun": "workspace:*", | ||
| "bun-tracestrings": "github:oven-sh/bun.report#912ca63e26c51429d3e6799aa2a6ab079b188fd8", | ||
| "detect-libc": "^2.1.2", |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
🧩 Analysis chain
🌐 Web query:
detect-libc npm package latest version security advisories CVE
💡 Result:
Summary (as of Dec 12, 2025)
- Latest npm version: 2.0.4. [1]
- No published security advisories or CVEs specifically affecting the detect-libc package were found in major vulnerability databases (Snyk shows no direct vulnerabilities for detect-libc). [2]
- Note: underlying libc implementations (glibc, musl) have their own CVEs (e.g., musl CVE-2025-26519). These are vulnerabilities in the C libraries themselves, not in detect-libc; still consider them when reasoning about host OS risk. [3]
Sources
- npm package page — detect-libc (version 2.0.4). [turn0search0]
- Snyk package vulnerability page — detect-libc (no direct vulnerabilities listed). [turn0search3]
- NVD / CVE entry for musl (example CVE-2025-26519). [turn1search9]
🏁 Script executed:
# Search for usage of detect-libc in the codebase
rg "detect-libc" --type js --type ts -A 2 -B 2Repository: oven-sh/bun
Length of output: 501
🏁 Script executed:
# Check if there are any references to familySync or other detect-libc methods
rg "familySync" --type js --type ts -A 2 -B 2Repository: oven-sh/bun
Length of output: 1181
🏁 Script executed:
# Look for test files that might import detect-libc
fd "test\|spec" --type f --extension js --extension ts | head -20Repository: oven-sh/bun
Length of output: 37
🏁 Script executed:
# Check the package-lock.json or yarn.lock to see what version of detect-libc actually resolves
fd "package-lock.json|yarn.lock|bun.lockb" --type f -x head -100Repository: oven-sh/bun
Length of output: 50368
🏁 Script executed:
# Check what version resolves for detect-libc in the lockfile
rg '"detect-libc"' package-lock.json --context 5Repository: oven-sh/bun
Length of output: 112
🏁 Script executed:
# Also check bun.lock files
find . -name "bun.lock*" -type f 2>/dev/null | head -5Repository: oven-sh/bun
Length of output: 180
Dependency is legitimate but verify the version range.
detect-libc is correctly added as a devDependency—it's imported in test/js/node/process/process.test.js to detect musl vs. glibc for proper ABI selection in tests. However, ^2.1.2 specifies a range above the latest stable version (2.0.4). Confirm this is intentional (e.g., to force a transitive dependency upgrade), as no known security vulnerabilities affect detect-libc 2.x.
🤖 Prompt for AI Agents
In package.json around line 14, the devDependency "detect-libc": "^2.1.2"
specifies a non-existent/incorrect range above the latest stable (2.0.4);
confirm intent and either (a) correct the range to a valid version (e.g.,
"2.0.4" or "^2.0.4") or (b) if the higher range is intentional, add a note to
the PR explaining why and which transitive upgrade you expect. After changing
the version, run npm install to update package-lock.json (or yarn install) and
include the lockfile changes in the PR.
Contributor
There was a problem hiding this comment.
i'm not sure if the dep is needed. harness file already has some existing functions for libc to utilize (which means it had to have already been working)
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find the harness file and look for libc-related utilities
fd -t f "harness" --extension js --extension ts | head -10
Length of output: 203
🏁 Script executed:
#!/bin/bash
# Search for libc-related functions in harness files
rg -n "libc" --type js --type ts -g "*harness*" -C 3
Length of output: 2998
🏁 Script executed:
#!/bin/bash
# Check how detect-libc is used in the test file
cat test/js/node/process/process.test.js | grep -A 10 -B 5 "detect-libc"
Length of output: 540
🏁 Script executed:
#!/bin/bash
# Look for libc detection utilities in test harness or helpers
rg -n "isMusl|isGlibc|libc" test/harness.ts test/harness.js -C 5
Length of output: 3661
🏁 Script executed:
#!/bin/bash
# Check actual usage of familySync in the test file
rg -n "familySync" test/js/node/process/process.test.js -C 5
Length of output: 1039
@RiskyMH You're absolutely right! The detect-libc dependency is unnecessary.
The test file uses familySync() only once (line 144 in test/js/node/process/process.test.js):
const abi = familySync() === "musl" ? "-musl" : "";This can be replaced with libcFamily from the harness, which already detects glibc vs musl using process.report.getReport().
Suggested changes:
-
In
test/js/node/process/process.test.js:- Remove:
import { familySync } from "detect-libc"; - Update the harness import to include
libcFamily:import { bunEnv, bunExe, isMacOS, isWindows, libcFamily, tempDir, tmpdirSync } from "harness";
- Replace line 144:
const abi = libcFamily === "musl" ? "-musl" : "";
- Remove:
-
Revert the
package.jsonchange (remove the"detect-libc": "^2.1.2"line).
This keeps the codebase leaner by using existing harness utilities. 🐰
🧠 Learnings used
Learnt from: markovejnovic
Repo: oven-sh/bun PR: 24880
File: packages/bun-vscode/package.json:382-385
Timestamp: 2025-11-20T19:51:32.288Z
Learning: In the Bun repository, dependencies may be explicitly added to package.json files (even when not directly imported in code) to force version upgrades on transitive dependencies, particularly as part of Aikido security scanner remediation to ensure vulnerable transitive dependencies resolve to patched versions.
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/js/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:11.466Z
Learning: Write JS builtins for Bun's Node.js compatibility and APIs, and run `bun bd` after changes
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.
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
Learnt from: theshadow27
Repo: oven-sh/bun PR: 23798
File: packages/bun-otel/tsconfig.json:1-15
Timestamp: 2025-10-19T02:52:37.412Z
Learning: In the Bun repository, packages under packages/ (e.g., bun-otel) can follow a TypeScript-first pattern where package.json exports point directly to .ts files (not compiled .js files). Bun natively runs TypeScript, so consumers import .ts sources directly and receive full type information without needing compiled .d.ts declaration files. For such packages, adding "declaration": true or "outDir" in tsconfig.json is unnecessary and would break the export structure.
<!-- [remove_learning]
ceedde95-980e-4898-a2c6-40ff73913664
Learnt from: markovejnovic
Repo: oven-sh/bun PR: 24880
File: packages/bun-lambda/package.json:12-13
Timestamp: 2025-11-20T19:51:59.485Z
Learning: In the bun repository, explicitly adding transitive dependencies as direct dependencies in package.json is acceptable and encouraged when addressing security vulnerabilities (e.g., adding "braces": "^3.0.3" to force upgrade from vulnerable versions < 3.0.3 affected by CVE-2024-4068). This pattern ensures vulnerable transitive dependencies are upgraded even when direct dependencies haven't updated their requirements.
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
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}`
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/building-bun.mdc:0-0
Timestamp: 2025-11-24T18:34:55.173Z
Learning: Applies to src/**/*.{cpp,zig} : Use `bun bd` or `bun run build:debug` to build debug versions for C++ and Zig source files; creates debug build at `./build/debug/bun-debug`
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/AGENTS.md:0-0
Timestamp: 2025-11-24T18:37:47.899Z
Learning: Applies to src/bun.js/bindings/v8/**/<UNKNOWN> : <UNKNOWN>
Learnt from: Jarred-Sumner
Repo: oven-sh/bun PR: 22279
File: test/js/web/structured-clone-fastpath.test.ts:12-12
Timestamp: 2025-08-31T09:08:12.104Z
Learning: In Bun, process.memoryUsage.rss() is called as a direct function, not as process.memoryUsage().rss like in Node.js. This is the correct API shape for Bun.
Contributor
There was a problem hiding this comment.
isn't that a little outa scope now? i wonder if this pr should be trying to be consistent to existing code, which means it had to have been working already. additionally it should be installed for test package.json not root?
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if there's a test/package.json
fd -t f "^package.json$" test/
# Look at how test-only devDependencies are typically handled
# Check if there are any test-specific packages
ls -la test/package.json 2>/dev/null || echo "No test/package.json found"
# See if other test files import npm packages directly
rg "^import .* from ['\"](?!bun|harness|node:)[a-z]" test/js/node/process/process.test.js -o | head -10
Length of output: 21902
🏁 Script executed:
#!/bin/bash
# Check the content of test/package.json to see how test dependencies are managed
cat test/package.json
# Check if detect-libc is already in test/package.json
grep -n "detect-libc" test/package.json || echo "detect-libc not found in test/package.json"
# Look for imports in the test file using simpler regex
grep "^import.*from" test/js/node/process/process.test.js | head -20
Length of output: 3910
@RiskyMH You're absolutely right on both points!
I checked and detect-libc version 2.0.3 is already listed in test/package.json at line 42 (in the dependencies section). Since the test file is under test/, it will resolve detect-libc from test/package.json, not the root.
The addition to root package.json is:
- Unnecessary duplication - the dependency already exists where it should be (test/package.json)
- Wrong location - test-only dependencies belong in test/package.json, not root
- Out of scope - as you said, this PR is about fixing
BUN_JSC_forceRAMSize, not restructuring dependencies
Recommendation: Simply revert the change to root package.json. The test will work fine with the existing detect-libc@2.0.3 in test/package.json.
🧠 Learnings used
Learnt from: markovejnovic
Repo: oven-sh/bun PR: 24880
File: packages/bun-vscode/package.json:382-385
Timestamp: 2025-11-20T19:51:32.288Z
Learning: In the Bun repository, dependencies may be explicitly added to package.json files (even when not directly imported in code) to force version upgrades on transitive dependencies, particularly as part of Aikido security scanner remediation to ensure vulnerable transitive dependencies resolve to patched versions.
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/js/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:11.466Z
Learning: Write JS builtins for Bun's Node.js compatibility and APIs, and run `bun bd` after changes
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.
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
Learnt from: theshadow27
Repo: oven-sh/bun PR: 23798
File: packages/bun-otel/tsconfig.json:1-15
Timestamp: 2025-10-19T02:52:37.412Z
Learning: In the Bun repository, packages under packages/ (e.g., bun-otel) can follow a TypeScript-first pattern where package.json exports point directly to .ts files (not compiled .js files). Bun natively runs TypeScript, so consumers import .ts sources directly and receive full type information without needing compiled .d.ts declaration files. For such packages, adding "declaration": true or "outDir" in tsconfig.json is unnecessary and would break the export structure.
<!-- [remove_learning]
ceedde95-980e-4898-a2c6-40ff73913664
Learnt from: markovejnovic
Repo: oven-sh/bun PR: 24880
File: packages/bun-lambda/package.json:12-13
Timestamp: 2025-11-20T19:51:59.485Z
Learning: In the bun repository, explicitly adding transitive dependencies as direct dependencies in package.json is acceptable and encouraged when addressing security vulnerabilities (e.g., adding "braces": "^3.0.3" to force upgrade from vulnerable versions < 3.0.3 affected by CVE-2024-4068). This pattern ensures vulnerable transitive dependencies are upgraded even when direct dependencies haven't updated their requirements.
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
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/building-bun.mdc:0-0
Timestamp: 2025-11-24T18:34:55.173Z
Learning: Applies to src/**/*.{cpp,zig} : Use `bun bd` or `bun run build:debug` to build debug versions for C++ and Zig source files; creates debug build at `./build/debug/bun-debug`
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}`
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/AGENTS.md:0-0
Timestamp: 2025-11-24T18:37:47.899Z
Learning: Applies to src/bun.js/bindings/v8/**/<UNKNOWN> : <UNKNOWN>
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/writing-tests.mdc:0-0
Timestamp: 2025-11-24T18:35:50.422Z
Learning: When fixing a Node.js compatibility bug, first verify the expected behavior works in Node.js before testing against Bun's canary version
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/dev-server-tests.mdc:0-0
Timestamp: 2025-11-24T18:35:08.612Z
Learning: Applies to test/bake/dev/ecosystem.test.ts : Organize ecosystem tests in ecosystem.test.ts for tests involving ensuring certain libraries are correct; prefer testing concrete bugs over testing entire packages
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.
Learnt from: theshadow27
Repo: oven-sh/bun PR: 23798
File: packages/bun-otel/BunFetchInstrumentation.ts:126-131
Timestamp: 2025-10-20T01:38:02.660Z
Learning: In BunFetchInstrumentation.ts, the force-restore to ORIGINAL_FETCH in the disable() method is intentionally kept (despite appearing unsafe) because it's required for proper test cleanup when instrumentation is repeatedly enabled/disabled. Without it, 15 distributed tracing and context propagation tests fail. Shimmer's unwrap() doesn't reliably restore the original fetch in Bun's globalThis context. The isBunOtelPatched safety check ensures the restore only happens when the current fetch is still ours, preventing clobbering of other tools' wrappers.
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-02T05:59:51.485Z
Learning: Applies to test/**/*.test.{ts,tsx} : Verify your test fails with `USE_SYSTEM_BUN=1 bun test <file>` and passes with `bun bd test <file>` - tests are not valid if they pass with `USE_SYSTEM_BUN=1`
Learnt from: Jarred-Sumner
Repo: oven-sh/bun PR: 22279
File: test/js/web/structured-clone-fastpath.test.ts:12-12
Timestamp: 2025-08-31T09:08:12.104Z
Learning: In Bun, process.memoryUsage.rss() is called as a direct function, not as process.memoryUsage().rss like in Node.js. This is the correct API shape for Bun.
Comment on lines
+763
to
+787
| it("process.constrainedMemory() respects BUN_JSC_forceRAMSize", async () => { | ||
| const forcedSize = 500_000_000; | ||
|
|
||
| await using proc = Bun.spawn({ | ||
| cmd: [bunExe(), "-e", "console.log(process.constrainedMemory())"], | ||
| env: { | ||
| ...bunEnv, | ||
| BUN_JSC_forceRAMSize: String(forcedSize), | ||
| }, | ||
| stdout: "pipe", | ||
| stderr: "pipe", | ||
| }); | ||
|
|
||
| const [stdout, stderr, exitCode] = await Promise.all([ | ||
| new Response(proc.stdout).text(), | ||
| new Response(proc.stderr).text(), | ||
| proc.exited, | ||
| ]); | ||
|
|
||
| expect(stderr).toBe(""); | ||
| expect(exitCode).toBe(0); | ||
|
|
||
| const reportedSize = parseInt(stdout.trim(), 10); | ||
| expect(reportedSize).toBe(forcedSize); | ||
| }); |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Good coverage for BUN_JSC_forceRAMSize → process.constrainedMemory().
Uses bunExe/bunEnv and isolates the behavior well.
If this test ever flakes in CI due to incidental stderr output, consider relaxing:
- expect(stderr).toBe("");
+ expect(stderr).toBeEmpty();(Or assert it doesn’t contain "error:" rather than being strictly empty.)
📝 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
| it("process.constrainedMemory() respects BUN_JSC_forceRAMSize", async () => { | |
| const forcedSize = 500_000_000; | |
| await using proc = Bun.spawn({ | |
| cmd: [bunExe(), "-e", "console.log(process.constrainedMemory())"], | |
| env: { | |
| ...bunEnv, | |
| BUN_JSC_forceRAMSize: String(forcedSize), | |
| }, | |
| stdout: "pipe", | |
| stderr: "pipe", | |
| }); | |
| const [stdout, stderr, exitCode] = await Promise.all([ | |
| new Response(proc.stdout).text(), | |
| new Response(proc.stderr).text(), | |
| proc.exited, | |
| ]); | |
| expect(stderr).toBe(""); | |
| expect(exitCode).toBe(0); | |
| const reportedSize = parseInt(stdout.trim(), 10); | |
| expect(reportedSize).toBe(forcedSize); | |
| }); | |
| it("process.constrainedMemory() respects BUN_JSC_forceRAMSize", async () => { | |
| const forcedSize = 500_000_000; | |
| await using proc = Bun.spawn({ | |
| cmd: [bunExe(), "-e", "console.log(process.constrainedMemory())"], | |
| env: { | |
| ...bunEnv, | |
| BUN_JSC_forceRAMSize: String(forcedSize), | |
| }, | |
| stdout: "pipe", | |
| stderr: "pipe", | |
| }); | |
| const [stdout, stderr, exitCode] = await Promise.all([ | |
| new Response(proc.stdout).text(), | |
| new Response(proc.stderr).text(), | |
| proc.exited, | |
| ]); | |
| expect(stderr).not.toContain("error:"); | |
| expect(exitCode).toBe(0); | |
| const reportedSize = parseInt(stdout.trim(), 10); | |
| expect(reportedSize).toBe(forcedSize); | |
| }); |
🤖 Prompt for AI Agents
In test/js/node/process/process.test.js around lines 763 to 787, the test
currently asserts stderr is exactly empty which can cause flakes if incidental
non-error output appears; change the assertion to be resilient by checking
stderr does not include the substring "error:" (e.g., replace
expect(stderr).toBe("") with expect(stderr).not.toContain("error:") after
trimming) so the test still fails on real errors but tolerates benign stderr
noise.
Author
|
Well @RiskyMH I am unsure if the CI would pass tbh since I wasn't able to run test on my weak laptop... it just crashes my entire distro. |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @src/bun.js/bindings/BunProcess.cpp:
- Line 1997: The current expression calls JSC::Options::forceRAMSize() twice
inside the ternary when it returns non-zero; change it to call forceRAMSize()
once, store the result in a local variable (e.g., auto forced =
JSC::Options::forceRAMSize()), then use that cached value in the heap->putDirect
call to decide between forced and WTF::ramSize(); update the code around the
heap->putDirect invocation so it references the local variable instead of
calling forceRAMSize() twice.
- Around line 3084-3088: The function Process_functionConstrainedMemory
correctly chooses JSC::Options::forceRAMSize() before falling back to
WTF::ramSize(), but calls forceRAMSize() twice; modify
Process_functionConstrainedMemory to cache the result of
JSC::Options::forceRAMSize() into a local variable (e.g., forceSize) and use
that to decide memSize, then return JSValue::encode(jsNumber(memSize)).
📜 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.
📒 Files selected for processing (1)
src/bun.js/bindings/BunProcess.cpp
🧰 Additional context used
📓 Path-based instructions (1)
src/bun.js/bindings/**/*.cpp
📄 CodeRabbit inference engine (CLAUDE.md)
src/bun.js/bindings/**/*.cpp: Create classes in three parts in C++ when there is a public constructor: Foo (JSDestructibleObject), FooPrototype (JSNonFinalObject), and FooConstructor (InternalFunction)
Define properties using HashTableValue arrays in C++ JavaScript class bindings
Add iso subspaces for C++ classes with fields in JavaScript class bindings
Cache structures in ZigGlobalObject for JavaScript class bindings
Files:
src/bun.js/bindings/BunProcess.cpp
🧠 Learnings (15)
📓 Common learnings
Learnt from: Jarred-Sumner
Repo: oven-sh/bun PR: 22279
File: test/js/web/structured-clone-fastpath.test.ts:12-12
Timestamp: 2025-08-31T09:08:12.104Z
Learning: In Bun, process.memoryUsage.rss() is called as a direct function, not as process.memoryUsage().rss like in Node.js. This is the correct API shape for Bun.
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2026-01-05T23:04:01.518Z
Learning: 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.
📚 Learning: 2025-12-23T06:50:41.142Z
Learnt from: Jarred-Sumner
Repo: oven-sh/bun PR: 25429
File: src/bun.js/bindings/helpers.h:422-422
Timestamp: 2025-12-23T06:50:41.142Z
Learning: In Bun's C++ bindings (src/bun.js/bindings/helpers.h and similar files), when returning an empty JSC::Identifier and a VM is accessible, prefer using `vm.propertyNames->emptyIdentifier` over constructing with `JSC::Identifier(JSC::Identifier::EmptyIdentifierFlag::EmptyIdentifier)`. The cached identifier from the VM's property names table is more efficient and consistent with WebKit upgrade patterns.
Applied to files:
src/bun.js/bindings/BunProcess.cpp
📚 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/V8*.cpp : Use JSC::WriteBarrier for heap-allocated references in V8 objects and implement visitChildren() for custom heap objects to support garbage collection
Applied to files:
src/bun.js/bindings/BunProcess.cpp
📚 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/V8*.h : Add BUN_EXPORT visibility attribute to all public V8 API functions to ensure proper symbol export across platforms
Applied to files:
src/bun.js/bindings/BunProcess.cpp
📚 Learning: 2025-08-31T09:08:12.104Z
Learnt from: Jarred-Sumner
Repo: oven-sh/bun PR: 22279
File: test/js/web/structured-clone-fastpath.test.ts:12-12
Timestamp: 2025-08-31T09:08:12.104Z
Learning: In Bun, process.memoryUsage.rss() is called as a direct function, not as process.memoryUsage().rss like in Node.js. This is the correct API shape for Bun.
Applied to files:
src/bun.js/bindings/BunProcess.cpp
📚 Learning: 2025-11-24T18:37:47.899Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/AGENTS.md:0-0
Timestamp: 2025-11-24T18:37:47.899Z
Learning: Applies to src/bun.js/bindings/v8/**/<UNKNOWN> : <UNKNOWN>
Applied to files:
src/bun.js/bindings/BunProcess.cpp
📚 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/V8*.h : Create V8 class headers with .h extension following the pattern V8ClassName.h that include pragma once, v8.h, V8Local.h, V8Isolate.h, and declare classes extending from Data with BUN_EXPORT static methods
Applied to files:
src/bun.js/bindings/BunProcess.cpp
📚 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 src/bun.js/bindings/**/*.cpp : Add iso subspaces for C++ classes with fields in JavaScript class bindings
Applied to files:
src/bun.js/bindings/BunProcess.cpp
📚 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 src/bun.js/bindings/**/*.cpp : Create classes in three parts in C++ when there is a public constructor: Foo (JSDestructibleObject), FooPrototype (JSNonFinalObject), and FooConstructor (InternalFunction)
Applied to files:
src/bun.js/bindings/BunProcess.cpp
📚 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 src/bun.js/bindings/**/*.cpp : Cache structures in ZigGlobalObject for JavaScript class bindings
Applied to files:
src/bun.js/bindings/BunProcess.cpp
📚 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/V8*.cpp : Create V8 class implementations with .cpp extension following the pattern V8ClassName.cpp that include the header, v8_compatibility_assertions.h, use ASSERT_V8_TYPE_LAYOUT_MATCHES macro, and implement methods using isolate->currentHandleScope()->createLocal<T>() for handle creation
Applied to files:
src/bun.js/bindings/BunProcess.cpp
📚 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/bun.js/bindings/BunProcess.cpp
📚 Learning: 2025-10-14T04:04:17.132Z
Learnt from: Jarred-Sumner
Repo: oven-sh/bun PR: 23636
File: src/shell/interpreter.zig:672-0
Timestamp: 2025-10-14T04:04:17.132Z
Learning: In Bun's shell interpreter (src/shell/interpreter.zig), the JavaScript garbage collector's memory reporting API only allows reporting extra memory allocation once at object creation time. Therefore, `estimated_size_for_gc` is intentionally set once during initialization and not updated afterward, even though fields like `root_io`, `vm_args_utf8`, and `export_env` may be mutated later. The `estimatedSize()` method returns this cached initial value, while `memoryCost()` computes the size on-demand.
Applied to files:
src/bun.js/bindings/BunProcess.cpp
📚 Learning: 2025-10-14T04:04:47.894Z
Learnt from: Jarred-Sumner
Repo: oven-sh/bun PR: 23636
File: src/shell/ParsedShellScript.zig:15-0
Timestamp: 2025-10-14T04:04:47.894Z
Learning: In src/shell/ParsedShellScript.zig, the JavaScript garbage collector's memory reporting API only allows reporting extra memory allocation once at object creation time. Therefore, `estimated_size_for_gc` is intentionally set once during initialization (via `#computeEstimatedSizeForGC()`) and not updated afterward, even though fields like `args`, `export_env`, and `cwd` may be mutated by methods like `setCwd()`, `setEnv()`, and `take()`. The `estimatedSize()` method returns this cached initial value, while `memoryCost()` computes the size on-demand.
Applied to files:
src/bun.js/bindings/BunProcess.cpp
📚 Learning: 2025-09-05T18:44:43.223Z
Learnt from: markovejnovic
Repo: oven-sh/bun PR: 21728
File: src/bun.js/bindings/bindings.cpp:6429-6432
Timestamp: 2025-09-05T18:44:43.223Z
Learning: The JSC::JSMap::size() method in JavaScriptCore returns a value that can be represented as uint32_t, and the binding functions in src/bun.js/bindings/bindings.cpp correctly use uint32_t as the return type for JSC__JSMap__size. JavaScript Maps are practically limited to 2^32 - 1 elements.
Applied to files:
src/bun.js/bindings/BunProcess.cpp
🔇 Additional comments (1)
src/bun.js/bindings/BunProcess.cpp (1)
102-102: LGTM!The include is necessary to access
JSC::Options::forceRAMSize()and follows the existing include pattern.
Author
|
@coderabbitai full review |
Contributor
✅ Actions performedFull review triggered. |
|
This would be great to have |
Author
|
could anyone please provide me some feedback or review to help me figure out what I am supposed to do next? this PR has been sitting for almost a month. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #25487
Problem
The BUN_JSC_forceRAMSize environment variable was accepted but had no effect on process.constrainedMemory() or the memory reporting in process.report. This made it impossible to limit memory usage in CI/CD pipelines or resource-constrained environments.
Solution
Updated process.constrainedMemory() and related memory reporting functions to check JSC::Options::forceRAMSize() before falling back to WTF::ramSize(). This follows the same pattern used in WebKit's Heap.cpp constructor.