Conversation
|
Warning Rate limit exceeded@shuakami has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 6 minutes and 39 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (8)
WalkthroughThe PR introduces node:inspector support with Zig/C++ bindings for initialization and coordination, TypeScript implementations for debugger lifecycle management and inspector control, global bindings exposing inspector functions, and tests for the new functionality. Changes
Possibly related PRs
Suggested reviewers
Pre-merge checks✅ Passed checks (2 passed)
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 |
There was a problem hiding this comment.
Actionable comments posted: 7
📜 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 (9)
cmake/targets/BuildBun.cmakesrc/bun.js/Debugger.zigsrc/bun.js/bindings/BunDebugger.cppsrc/bun.js/bindings/ExposeNodeModuleGlobals.cppsrc/bun.js/bindings/InternalModuleRegistry.cppsrc/js/internal/debugger.tssrc/js/node/inspector.tstest/js/internal/debugger/programmatic-control.test.tstest/js/node/inspector/inspector.test.ts
🧰 Additional context used
📓 Path-based instructions (10)
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/internal/debugger/programmatic-control.test.tstest/js/node/inspector/inspector.test.ts
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/internal/debugger/programmatic-control.test.tstest/js/node/inspector/inspector.test.ts
**/*.test.ts?(x)
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.test.ts?(x): Never usebun testdirectly - always usebun bd testto run tests with debug build changes
For single-file tests, prefer-eflag overtempDir
For multi-file tests, prefertempDirandBun.spawnover single-file tests
UsenormalizeBunSnapshotto normalize snapshot output of tests
Never write tests that check for 'panic', 'uncaught exception', or similar strings in test output
UsetempDirfromharnessto create temporary directories - do not usetmpdirSyncorfs.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 usesetTimeoutin tests; instead await the condition to be met
Verify tests fail withUSE_SYSTEM_BUN=1 bun test <file>and pass withbun bd test <file>- tests are invalid if they pass with USE_SYSTEM_BUN=1
Test files must end with.test.tsor.test.tsx
Avoid shell commands likefindorgrepin tests - use Bun's Glob and built-in tools instead
Files:
test/js/internal/debugger/programmatic-control.test.tstest/js/node/inspector/inspector.test.ts
test/**/*.test.ts?(x)
📄 CodeRabbit inference engine (CLAUDE.md)
Always use
port: 0in tests - do not hardcode ports or use custom random port number functions
Files:
test/js/internal/debugger/programmatic-control.test.tstest/js/node/inspector/inspector.test.ts
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/ExposeNodeModuleGlobals.cppsrc/bun.js/Debugger.zigsrc/bun.js/bindings/InternalModuleRegistry.cppsrc/bun.js/bindings/BunDebugger.cpp
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/ExposeNodeModuleGlobals.cppsrc/bun.js/bindings/InternalModuleRegistry.cppsrc/bun.js/bindings/BunDebugger.cpp
src/**/*.zig
📄 CodeRabbit inference engine (.cursor/rules/building-bun.mdc)
Use
bun.Output.scoped(.${SCOPE}, .hidden)for creating debug logs in Zig codeImplement core functionality in Zig, typically in its own directory in
src/
src/**/*.zig: Private fields in Zig are fully supported using the#prefix:struct { #foo: u32 };
Use decl literals in Zig for declaration initialization:const decl: Decl = .{ .binding = 0, .value = 0 };
Prefer@importat the bottom of the file (auto formatter will move them automatically)
Files:
src/bun.js/Debugger.zig
**/*.zig
📄 CodeRabbit inference engine (.cursor/rules/zig-javascriptcore-classes.mdc)
**/*.zig: Expose generated bindings in Zig structs usingpub const js = JSC.Codegen.JS<ClassName>with trait conversion methods:toJS,fromJS, andfromJSDirect
Use consistent parameter nameglobalObjectinstead ofctxin Zig constructor and method implementations
Usebun.JSError!JSValuereturn type for Zig methods and constructors to enable proper error handling and exception propagation
Implement resource cleanup usingdeinit()method that releases resources, followed byfinalize()called by the GC that invokesdeinit()and frees the pointer
UseJSC.markBinding(@src())in finalize methods for debugging purposes before callingdeinit()
For methods returning cached properties in Zig, declare external C++ functions usingextern fnandcallconv(JSC.conv)calling convention
Implement getter functions with naming patternget<PropertyName>in Zig that acceptthisandglobalObjectparameters and returnJSC.JSValue
Access JavaScript CallFrame arguments usingcallFrame.argument(i), check argument count withcallFrame.argumentCount(), and getthiswithcallFrame.thisValue()
For reference-counted objects, use.deref()in finalize instead ofdestroy()to release references to other JS objectsIn Zig code, be careful with allocators and use defer for cleanup
Files:
src/bun.js/Debugger.zig
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 literalrequire()statements only; dynamic requires are not permitted
Export modules usingexport 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 frominternal/validatorsand throw$ERR_*error codes for invalid arguments
Useprocess.platformandprocess.archfor platform detection; these values are inlined and dead-code eliminated at build time
Files:
src/js/node/inspector.tssrc/js/internal/debugger.ts
src/js/{builtins,node,bun,thirdparty,internal}/**/*.ts
📄 CodeRabbit inference engine (src/js/CLAUDE.md)
Builtin functions must include
thisparameter typing in TypeScript to enable direct method binding in C++
Files:
src/js/node/inspector.tssrc/js/internal/debugger.ts
🧠 Learnings (70)
📓 Common learnings
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: Applies to **/js_*.zig : Implement proper memory management with reference counting using `ref()`/`deref()` in JavaScript bindings
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/zig-javascriptcore-classes.mdc:0-0
Timestamp: 2025-11-24T18:36:08.558Z
Learning: Applies to **/*.zig : Use `JSC.markBinding(src())` in finalize methods for debugging purposes before calling `deinit()`
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: Applies to **/*js_bindings.classes.ts : Use `JSC.Codegen` correctly to generate necessary binding code for JavaScript-Zig integration
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 : Builtin functions must include `this` parameter typing in TypeScript to enable direct method binding in C++
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/zig-javascriptcore-classes.mdc:0-0
Timestamp: 2025-11-24T18:36:08.558Z
Learning: Applies to **/*.zig : Expose generated bindings in Zig structs using `pub const js = JSC.Codegen.JS<ClassName>` with trait conversion methods: `toJS`, `fromJS`, and `fromJSDirect`
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: Applies to **/js_*.zig : Implement `JSYourFeature` struct in a file like `js_your_feature.zig` to create JavaScript bindings for Zig functionality
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
Learnt from: theshadow27
Repo: oven-sh/bun PR: 23798
File: test/js/bun/http/node-telemetry.test.ts:27-203
Timestamp: 2025-10-19T04:55:33.099Z
Learning: In test/js/bun/http/node-telemetry.test.ts and the Bun.telemetry._node_binding API, after the architecture refactor, the _node_binding interface only contains two methods: handleIncomingRequest(req, res) and handleWriteHead(res, statusCode). The handleRequestFinish hook and other lifecycle hooks were removed during simplification. Both current methods are fully tested.
📚 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/internal/debugger/programmatic-control.test.tstest/js/node/inspector/inspector.test.tssrc/bun.js/Debugger.zigsrc/bun.js/bindings/BunDebugger.cpp
📚 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/js/internal/debugger/programmatic-control.test.tstest/js/node/inspector/inspector.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/js/internal/debugger/programmatic-control.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/js/internal/debugger/programmatic-control.test.ts
📚 Learning: 2025-11-24T18:35:08.612Z
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/bundle.test.ts : Organize bundle tests in bundle.test.ts for tests concerning bundling bugs that only occur in DevServer
Applied to files:
test/js/internal/debugger/programmatic-control.test.tstest/js/node/inspector/inspector.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/**/*.test.ts?(x) : Always use `port: 0` in tests - do not hardcode ports or use custom random port number functions
Applied to files:
test/js/internal/debugger/programmatic-control.test.tstest/js/node/inspector/inspector.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/**/*-fixture.ts : Test files that spawn Bun processes should end in `*-fixture.ts` to identify them as test fixtures and not tests themselves
Applied to files:
test/js/internal/debugger/programmatic-control.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/js/internal/debugger/programmatic-control.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/js/internal/debugger/programmatic-control.test.ts
📚 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/**/*.{js,ts,jsx,tsx} : Write tests as JavaScript and TypeScript files using Jest-style APIs (`test`, `describe`, `expect`) and import from `bun:test`
Applied to files:
test/js/internal/debugger/programmatic-control.test.ts
📚 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/internal/debugger/programmatic-control.test.tstest/js/node/inspector/inspector.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/js/internal/debugger/programmatic-control.test.ts
📚 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/internal/debugger/programmatic-control.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/js/internal/debugger/programmatic-control.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/js/internal/debugger/programmatic-control.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/js/internal/debugger/programmatic-control.test.tstest/js/node/inspector/inspector.test.ts
📚 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/ExposeNodeModuleGlobals.cppsrc/bun.js/Debugger.zigsrc/bun.js/bindings/InternalModuleRegistry.cppsrc/bun.js/bindings/BunDebugger.cppsrc/js/node/inspector.tssrc/js/internal/debugger.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-module/main.cpp : Register new V8 API test functions in the Init method using NODE_SET_METHOD with exports object
Applied to files:
src/bun.js/bindings/ExposeNodeModuleGlobals.cppsrc/bun.js/Debugger.zigsrc/bun.js/bindings/InternalModuleRegistry.cppsrc/bun.js/bindings/BunDebugger.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/ExposeNodeModuleGlobals.cppcmake/targets/BuildBun.cmakesrc/bun.js/Debugger.zigsrc/bun.js/bindings/InternalModuleRegistry.cppsrc/bun.js/bindings/BunDebugger.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/src/symbols.txt : Add symbol names without leading underscore to src/symbols.txt for each new V8 API method
Applied to files:
src/bun.js/bindings/ExposeNodeModuleGlobals.cppsrc/bun.js/Debugger.zigsrc/bun.js/bindings/InternalModuleRegistry.cppsrc/bun.js/bindings/BunDebugger.cpp
📚 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: Applies to src/bun.js/bindings/BunObject+exports.h : Add an entry to the `FOR_EACH_GETTER` macro in `src/bun.js/bindings/BunObject+exports.h` when registering new features
Applied to files:
src/bun.js/bindings/ExposeNodeModuleGlobals.cppsrc/bun.js/bindings/InternalModuleRegistry.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/ExposeNodeModuleGlobals.cppsrc/bun.js/bindings/InternalModuleRegistry.cppsrc/bun.js/bindings/BunDebugger.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/src/symbols.dyn : Add symbol names with leading underscore and semicolons in braces to src/symbols.dyn for each new V8 API method
Applied to files:
src/bun.js/bindings/ExposeNodeModuleGlobals.cppsrc/bun.js/bindings/BunDebugger.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/ExposeNodeModuleGlobals.cppsrc/bun.js/bindings/InternalModuleRegistry.cppsrc/bun.js/bindings/BunDebugger.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/ExposeNodeModuleGlobals.cppsrc/bun.js/bindings/InternalModuleRegistry.cppsrc/bun.js/bindings/BunDebugger.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: Write JS builtins for Bun's Node.js compatibility and APIs, and run `bun bd` after changes
Applied to files:
src/bun.js/bindings/ExposeNodeModuleGlobals.cppsrc/bun.js/Debugger.zigsrc/bun.js/bindings/BunDebugger.cpp
📚 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: Applies to src/bun.js/api/BunObject.zig : Implement getter functions in `src/bun.js/api/BunObject.zig` that return your feature, and export them in the `exportAll()` function
Applied to files:
src/bun.js/bindings/ExposeNodeModuleGlobals.cppsrc/bun.js/Debugger.zig
📚 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/ExposeNodeModuleGlobals.cppsrc/bun.js/Debugger.zigsrc/bun.js/bindings/InternalModuleRegistry.cppsrc/bun.js/bindings/BunDebugger.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 : Expose C++ class constructors to Zig using `extern "C"` functions following the pattern `Bun__JSClassName(Zig::GlobalObject*)` that return the encoded JSValue constructor
Applied to files:
src/bun.js/bindings/ExposeNodeModuleGlobals.cppsrc/bun.js/bindings/BunDebugger.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 *.zig : In Zig, declare external C++ functions and wrap them in public methods using the convention `extern fn Bun__ClassName__toJS(...)` and `pub fn toJS(...)`
Applied to files:
src/bun.js/bindings/ExposeNodeModuleGlobals.cppsrc/bun.js/Debugger.zig
📚 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 : To create JavaScript objects from Zig, implement C++ functions following the `Bun__ClassName__toJS(Zig::GlobalObject*, NativeType*)` convention that construct and return the JavaScript object as an encoded JSValue
Applied to files:
src/bun.js/bindings/ExposeNodeModuleGlobals.cpp
📚 Learning: 2025-11-24T18:36:08.558Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/zig-javascriptcore-classes.mdc:0-0
Timestamp: 2025-11-24T18:36:08.558Z
Learning: Applies to **/*.zig : Implement getter functions with naming pattern `get<PropertyName>` in Zig that accept `this` and `globalObject` parameters and return `JSC.JSValue`
Applied to files:
src/bun.js/bindings/ExposeNodeModuleGlobals.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 : Define properties using HashTableValue arrays in C++ JavaScript class bindings
Applied to files:
src/bun.js/bindings/ExposeNodeModuleGlobals.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/ExposeNodeModuleGlobals.cppsrc/bun.js/bindings/BunDebugger.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 : For objects that need internal fields, extend InternalFieldObject class instead of directly extending Data
Applied to files:
src/bun.js/bindings/ExposeNodeModuleGlobals.cppsrc/bun.js/bindings/InternalModuleRegistry.cpp
📚 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:
cmake/targets/BuildBun.cmake
📚 Learning: 2025-10-01T21:48:38.278Z
Learnt from: taylordotfish
Repo: oven-sh/bun PR: 23169
File: src/bun.js/bindings/Bindgen/IDLTypes.h:1-3
Timestamp: 2025-10-01T21:48:38.278Z
Learning: In the Bun codebase, for `BunIDL*` and `Bindgen*` headers (e.g., BunIDLTypes.h, Bindgen/IDLTypes.h), it's acceptable to rely on transitive includes for standard library headers like <type_traits> and <utility> rather than including them explicitly.
Applied to files:
cmake/targets/BuildBun.cmake
📚 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} : Execute files using `bun bd <file> <...args>`; never use `bun <file>` directly as it will not include your changes
Applied to files:
cmake/targets/BuildBun.cmake
📚 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:
cmake/targets/BuildBun.cmake
📚 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:
cmake/targets/BuildBun.cmakesrc/bun.js/bindings/InternalModuleRegistry.cppsrc/bun.js/bindings/BunDebugger.cpp
📚 Learning: 2025-10-19T04:55:33.099Z
Learnt from: theshadow27
Repo: oven-sh/bun PR: 23798
File: test/js/bun/http/node-telemetry.test.ts:27-203
Timestamp: 2025-10-19T04:55:33.099Z
Learning: In test/js/bun/http/node-telemetry.test.ts and the Bun.telemetry._node_binding API, after the architecture refactor, the _node_binding interface only contains two methods: handleIncomingRequest(req, res) and handleWriteHead(res, statusCode). The handleRequestFinish hook and other lifecycle hooks were removed during simplification. Both current methods are fully tested.
Applied to files:
test/js/node/inspector/inspector.test.tssrc/bun.js/Debugger.zigsrc/js/node/inspector.ts
📚 Learning: 2025-11-24T18:35:08.612Z
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/**/*.test.ts : Use `dev.write()`, `dev.patch()`, and `dev.delete()` to mutate the filesystem instead of `node:fs` APIs, as dev server functions are hooked to wait for hot-reload and notify clients
Applied to files:
test/js/node/inspector/inspector.test.ts
📚 Learning: 2025-10-18T05:23:24.403Z
Learnt from: theshadow27
Repo: oven-sh/bun PR: 23798
File: test/js/bun/telemetry-server.test.ts:91-100
Timestamp: 2025-10-18T05:23:24.403Z
Learning: In the Bun codebase, telemetry tests (test/js/bun/telemetry-*.test.ts) should focus on telemetry API behavior: configure/disable/isEnabled, callback signatures and invocation, request ID correlation, and error handling. HTTP protocol behaviors like status code normalization (e.g., 200 with empty body → 204) should be tested in HTTP server tests (test/js/bun/http/), not in telemetry tests. Keep separation of concerns: telemetry tests verify the telemetry API contract; HTTP tests verify HTTP semantics.
Applied to files:
test/js/node/inspector/inspector.test.ts
📚 Learning: 2025-09-03T01:30:58.001Z
Learnt from: markovejnovic
Repo: oven-sh/bun PR: 21728
File: test/js/valkey/valkey.test.ts:264-271
Timestamp: 2025-09-03T01:30:58.001Z
Learning: For test/js/valkey/valkey.test.ts PUB/SUB tests, avoid arbitrary sleeps and async-forEach. Instead, resolve a Promise from the subscriber callback when the expected number of messages is observed and await it with a bounded timeout (e.g., withTimeout + Promise.withResolvers) to account for Redis server→subscriber propagation.
Applied to files:
test/js/node/inspector/inspector.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/js/node/inspector/inspector.test.ts
📚 Learning: 2025-11-24T18:35:08.612Z
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/**/*.test.ts : Assert console messages using `c.expectMessage()` with single or multiple arguments; any unasserted logs fail the test to catch unexpected re-evaluations or reloads
Applied to files:
test/js/node/inspector/inspector.test.ts
📚 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: When fixing a Node.js compatibility bug, first verify the expected behavior works in Node.js before testing against Bun's canary version
Applied to files:
test/js/node/inspector/inspector.test.ts
📚 Learning: 2025-11-24T18:36:08.558Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/zig-javascriptcore-classes.mdc:0-0
Timestamp: 2025-11-24T18:36:08.558Z
Learning: Applies to src/bun.js/bindings/generated_classes_list.zig : Include new class bindings in `src/bun.js/bindings/generated_classes_list.zig` to register them with the code generator
Applied to files:
src/bun.js/Debugger.zigsrc/bun.js/bindings/InternalModuleRegistry.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/src/napi/napi.zig : For each new V8 C++ method, add both GCC/Clang and MSVC mangled symbol names to the V8API struct in src/napi/napi.zig using extern fn declarations
Applied to files:
src/bun.js/Debugger.zig
📚 Learning: 2025-11-24T18:36:08.558Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/zig-javascriptcore-classes.mdc:0-0
Timestamp: 2025-11-24T18:36:08.558Z
Learning: Applies to **/*.zig : Use `JSC.markBinding(src())` in finalize methods for debugging purposes before calling `deinit()`
Applied to files:
src/bun.js/Debugger.zigsrc/bun.js/bindings/InternalModuleRegistry.cpp
📚 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: Applies to **/js_*.zig : Implement proper memory management with reference counting using `ref()`/`deref()` in JavaScript bindings
Applied to files:
src/bun.js/Debugger.zig
📚 Learning: 2025-11-03T20:43:06.996Z
Learnt from: pfgithub
Repo: oven-sh/bun PR: 24273
File: src/bun.js/test/snapshot.zig:19-19
Timestamp: 2025-11-03T20:43:06.996Z
Learning: In Bun's Zig codebase, when storing JSValue objects in collections like ArrayList, use `jsc.Strong.Optional` (not raw JSValue). When adding values, wrap them with `jsc.Strong.Optional.create(value, globalThis)`. In cleanup code, iterate the collection calling `.deinit()` on each Strong.Optional item before calling `.deinit()` on the ArrayList itself. This pattern automatically handles GC protection. See examples in src/bun.js/test/ScopeFunctions.zig and src/bun.js/node/node_cluster_binding.zig.
Applied to files:
src/bun.js/Debugger.zigsrc/bun.js/bindings/InternalModuleRegistry.cpp
📚 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: Applies to **/js_*.zig : Use `bun.JSError!JSValue` for proper error propagation in JavaScript bindings
Applied to files:
src/bun.js/Debugger.zigsrc/bun.js/bindings/InternalModuleRegistry.cpp
📚 Learning: 2025-11-24T18:36:08.558Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/zig-javascriptcore-classes.mdc:0-0
Timestamp: 2025-11-24T18:36:08.558Z
Learning: Applies to **/*.zig : Expose generated bindings in Zig structs using `pub const js = JSC.Codegen.JS<ClassName>` with trait conversion methods: `toJS`, `fromJS`, and `fromJSDirect`
Applied to files:
src/bun.js/Debugger.zig
📚 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-module/main.cpp : Create test functions in test/v8/v8-module/main.cpp that take FunctionCallbackInfo<Value> parameter, use the test V8 API, print results for comparison with Node.js, and return Undefined
Applied to files:
src/bun.js/bindings/InternalModuleRegistry.cppsrc/bun.js/bindings/BunDebugger.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 V8_UNIMPLEMENTED() macro for functions not yet implemented in V8 compatibility classes
Applied to files:
src/bun.js/bindings/InternalModuleRegistry.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/InternalModuleRegistry.cppsrc/bun.js/bindings/BunDebugger.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/InternalModuleRegistry.cppsrc/bun.js/bindings/BunDebugger.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/InternalModuleRegistry.cppsrc/bun.js/bindings/BunDebugger.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 isolate->currentHandleScope()->createLocal<T>() to create local V8 handles and ensure all V8 values are created within an active handle scope
Applied to files:
src/bun.js/bindings/InternalModuleRegistry.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/BunDebugger.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/BunDebugger.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 constructor classes inheriting from `JSC::InternalFunction`, implementing `create()`, `subspaceFor()`, `createStructure()`, `DECLARE_INFO`, and `finishCreation()` methods, with separate call and construct handlers
Applied to files:
src/bun.js/bindings/BunDebugger.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/BunDebugger.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/BunDebugger.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/BunDebugger.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 `$debug()` for debug logging and `$assert()` for assertions; both are stripped in release builds
Applied to files:
src/js/internal/debugger.ts
📚 Learning: 2025-10-26T04:50:17.892Z
Learnt from: Jarred-Sumner
Repo: oven-sh/bun PR: 24086
File: src/bun.js/api/server.zig:2534-2535
Timestamp: 2025-10-26T04:50:17.892Z
Learning: In src/bun.js/api/server.zig, u32 is acceptable for route indices and websocket_context_index fields, as the practical limit of 4 billion contexts will never be reached.
Applied to files:
src/js/internal/debugger.ts
📚 Learning: 2025-10-17T20:50:58.644Z
Learnt from: taylordotfish
Repo: oven-sh/bun PR: 23755
File: src/bun.js/api/bun/socket/Handlers.zig:154-159
Timestamp: 2025-10-17T20:50:58.644Z
Learning: In Bun socket configuration error messages (src/bun.js/api/bun/socket/Handlers.zig), use the user-facing JavaScript names "data" and "drain" instead of internal field names "onData" and "onWritable", as these are the names users see in the API according to SocketConfig.bindv2.ts.
Applied to files:
src/js/internal/debugger.ts
🧬 Code graph analysis (5)
test/js/internal/debugger/programmatic-control.test.ts (2)
test/harness.ts (2)
tempDir(277-284)bunExe(102-105)packages/bun-inspector-protocol/src/protocol/jsc/index.d.ts (1)
Response(2793-2806)
test/js/node/inspector/inspector.test.ts (3)
test/js/bun/http/bun-websocket-cpu-fixture.js (1)
ws(25-25)test/js/node/test_runner/fixtures/02-hooks.js (1)
JSON(7-7)packages/bun-inspector-protocol/src/protocol/jsc/index.d.ts (1)
Response(2793-2806)
src/bun.js/bindings/InternalModuleRegistry.cpp (1)
src/bun.js/bindings/BunDebugger.cpp (5)
globalObject(88-91)globalObject(88-88)globalObject(235-238)globalObject(248-295)globalObject(248-248)
src/bun.js/bindings/BunDebugger.cpp (2)
src/bun.js/bindings/ScriptExecutionContext.cpp (2)
globalObject(93-96)globalObject(93-93)src/bun.js/bindings/ZigGlobalObject.cpp (8)
create(371-376)create(371-371)create(378-383)create(378-378)create(385-390)create(385-385)create(392-397)create(392-392)
src/js/internal/debugger.ts (1)
src/js/node/async_hooks.ts (1)
exit(136-138)
🪛 Cppcheck (2.18.0)
src/bun.js/bindings/ExposeNodeModuleGlobals.cpp
[information] 74-74: Include file
(missingIncludeSystem)
src/bun.js/bindings/InternalModuleRegistry.cpp
[information] 16-16: Include file
(missingInclude)
[information] 19-19: Include file
(missingIncludeSystem)
[information] 20-20: Include file
(missingIncludeSystem)
[information] 21-21: Include file
(missingIncludeSystem)
[information] 22-22: Include file
(missingIncludeSystem)
[information] 23-23: Include file
(missingIncludeSystem)
[information] 24-24: Include file
(missingInclude)
src/bun.js/bindings/BunDebugger.cpp
[information] 19-19: Include file
(missingIncludeSystem)
[information] 20-20: Include file
(missingIncludeSystem)
[information] 21-21: Include file
(missingIncludeSystem)
[information] 22-22: Include file
(missingIncludeSystem)
[information] 23-23: Include file
(missingIncludeSystem)
[information] 24-24: Include file
(missingInclude)
[information] 26-26: Include file
(missingInclude)
[information] 19-19: Include file
(missingIncludeSystem)
[information] 42-42: Include file
(missingInclude)
[information] 52-52: Include file
(missingIncludeSystem)
[information] 59-59: Include file
(missingIncludeSystem)
[information] 74-74: Include file
(missingIncludeSystem)
[information] 49-49: Include file
(missingIncludeSystem)
[information] 50-50: Include file
(missingIncludeSystem)
[information] 51-51: Include file
(missingIncludeSystem)
[information] 52-52: Include file
(missingIncludeSystem)
[information] 53-53: Include file
(missingInclude)
[information] 57-57: Include file
(missingInclude)
[information] 62-62: Include file
(missingIncludeSystem)
[information] 63-63: Include file
(missingIncludeSystem)
[information] 715-715: Include file
(missingInclude)
🔇 Additional comments (24)
cmake/targets/BuildBun.cmake (1)
1314-1314: LGTM!The addition of
xmlliteas a Windows linker dependency is properly documented and aligns with libarchive's requirements for xar format support.src/js/internal/debugger.ts (3)
99-121: LGTM!The type definitions and safe callback wrappers are well-designed. The empty catch blocks are appropriate here since these are optional callbacks where failures should not interrupt the inspector lifecycle.
300-311: LGTM!The
stop()method properly clears the server reference before attempting shutdown, preventing double-stop issues. The error suppression is appropriate for graceful cleanup scenarios.
492-611: LGTM!The enhanced error handling properly coordinates
fatalOnErrorwithreportError, and the backend cleanup in the catch block prevents resource leaks when connection fails.test/js/internal/debugger/programmatic-control.test.ts (2)
139-145: Template literal evaluated at definition time, not per iteration.The
${i}in the template literal is evaluated when each loop iteration creates thetest.jsfile, so it will correctly containrun 0andrun 1. However, this relies on JavaScript's closure behavior for the loop variable. The code works as intended, but consider adding a comment or using a function-scoped approach for clarity.
21-54: LGTM!The test properly uses
port: 0via--inspect=0,using/await usingfor cleanup, and validates both stdout content and exit code. The ephemeral port verification is a good addition.src/bun.js/bindings/InternalModuleRegistry.cpp (2)
16-24: LGTM!Forward declarations are correctly structured for the inspector binding host functions. The static analysis warnings about missing includes are false positives since these are forward declarations that will be resolved at link time.
175-212: LGTM!The lazy initialization of
__bunInspectorbefore loading NodeInspector is a clean approach. The property attributes (DontEnum, DontDelete, ReadOnly) appropriately protect the binding from user tampering.One consideration: the
hasPropertycheck on line 178 performs a prototype chain lookup. Since__bunInspectoris set directly on the global object, usinghasOwnPropertyequivalent orgetDirectOffsetmight be marginally faster, though this is likely negligible since it only runs once per global object lifetime.test/js/node/inspector/inspector.test.ts (2)
33-61: LGTM!Comprehensive smoke test that validates the full inspector lifecycle including WebSocket-based CDP communication. Good practice to close the WebSocket before calling
inspector.close().Minor note: The
wsWebSocket instance isn't usingusing/await using, but sincews.close()is called explicitly before the test ends, cleanup is handled correctly.
63-70: LGTM!Good state stability test that validates the inspector can be reliably opened and closed multiple times without resource leaks or state corruption.
src/bun.js/Debugger.zig (2)
233-270: LGTM! Well-structured node:inspector initialization with proper synchronization.The implementation correctly:
- Uses
jsc.markBinding(@src())for debugging as per coding guidelines- Handles the case where debugger thread hasn't started by using a whitespace URL placeholder
- Properly cleans up futex state on
create()failure to avoid deadlocks- Uses futex-based wait/wake for efficient blocking until the debugger thread bootstraps
One consideration: the whitespace URL
" "being used as a stop command is a bit implicit. A comment explaining this convention is already present (lines 247-248), which is good.
272-285: LGTM! Proper wait-for-debugger implementation.The function correctly sets up the blocking wait state by:
- Early returning if no debugger is configured
- Setting
wait_for_connection = .foreverfor indefinite waiting- Setting
must_block_until_connected = trueto trigger the blocking behavior- Calling
poll_ref.ref(vm)to keep the event loop alive during the waitsrc/js/node/inspector.ts (4)
10-35: LGTM! Robust binding validation with proper caching.The
getBinding()function properly:
- Caches the binding to avoid repeated lookups
- Validates all required methods exist before caching
- Throws a descriptive error with issue number if binding is missing
63-84: LGTM! Thorough port validation.The
parsePort()function handles all input types correctly with appropriate error messages. The validation is comprehensive and follows the expected behavior.
86-114: LGTM! Flexible argument normalization matching Node.js behavior.The function correctly handles the various calling conventions:
open()- defaultsopen(port)- custom portopen(port, host)- custom port and hostopen(port, host, wait)- all explicitopen(true)- default port/host, wait for debuggeropen(port, true)- custom port, wait for debugger
116-134: LGTM! Clean open() implementation with proper idempotency.The implementation correctly:
- Checks if already open and handles the wait flag in that case
- Formats IPv6 addresses with brackets for URL compatibility
- Generates a random pathname for security
- Delegates to the native binding
src/bun.js/bindings/ExposeNodeModuleGlobals.cpp (2)
70-75: LGTM! Proper forward declarations for inspector host functions.The declarations correctly match the
JSC_DECLARE_HOST_FUNCTIONpattern used in BunDebugger.cpp.
94-127: LGTM! Well-structured internal binding exposure.The
__bunInspectorobject is correctly:
- Created with space for exactly 4 methods
- Populated with methods matching the expected
NativeInspectorBindinginterface in TypeScript- Made non-enumerable, read-only, and non-deletable to prevent user tampering
The function arity (2 for open, 0 for others) matches the expected signatures.
src/bun.js/bindings/BunDebugger.cpp (6)
41-73: LGTM! Clean synchronization primitives for cross-thread inspector operations.The
nodeInspectorBeginOp,nodeInspectorWaitOp, andnodeInspectorSignalDonefunctions implement a proper request-response pattern:
beginOpsets the pending flag and clears previous errorwaitOpblocks on the condition variable until operation completessignalDoneclears the pending flag and notifies waitersThe use of
isolatedCopy()for the error string ensures thread-safe string ownership.
616-636: LGTM! Thread-safe URL update with proper success/error handling.The function correctly:
- Acquires the lock before modifying shared state
- Only clears the error on success (non-empty URL)
- Signals completion to unblock the waiting thread
707-760: LGTM! RobustjsBunInspectorOpenimplementation.The implementation correctly:
- Validates the URL argument is a string
- Ensures the debugger thread is initialized before proceeding
- Is idempotent (returns early if already open, optionally waiting)
- Uses the begin/wait pattern for synchronous completion
- Propagates errors from the debugger thread as JS exceptions
- Sets
fatalOnError=falseso errors are reported rather than exiting the process
762-803: Minor: Lock acquired twice in sequence.Lines 773-778 and 782-784 acquire the lock separately. This is correct behavior but slightly inefficient. However, since this is an infrequent operation (closing the inspector), it's not a performance concern.
The implementation correctly handles the edge case where URL exists but debugger thread is missing by clearing the URL state.
657-705: LGTM! Clean refactoring of internal debugger invocation.The
callInternalDebuggerfunction now:
- Passes the URL string directly instead of port/path parsing
- Provides callbacks for the debugger thread to report URL and errors
- Supports both fatal (CLI
--inspect) and non-fatal (programmaticopen()) error handlingThe
scheduleInternalDebuggerCallhelper correctly isolates the URL string copy and posts the task to the debugger thread.
832-849: LGTM! CLI debugger startup updated to use new signature.The CLI path correctly:
- Sets
fatalOnError=trueso errors exit the process (matching Node.js behavior for--inspect)- Passes the URL through the new hooks so
node:inspector.url()can reflect the CLI-started inspector
| function randomPathname(): string { | ||
| try { | ||
| const crypto = require("node:crypto"); | ||
| if (typeof crypto.randomUUID === "function") { | ||
| return "/" + crypto.randomUUID().replaceAll("-", ""); | ||
| } | ||
| if (typeof crypto.randomBytes === "function") { | ||
| return "/" + crypto.randomBytes(16).toString("hex"); | ||
| } | ||
| } catch { | ||
| // ignore | ||
| } | ||
| return "/" + Math.random().toString(16).slice(2) + Math.random().toString(16).slice(2); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider the security implications of the Math.random fallback.
The randomPathname() function falls back to Math.random() if crypto is unavailable. While this is unlikely to occur in practice (Bun always has crypto), the fallback path produces a predictable URL path that could theoretically allow unauthorized debugger connections.
Node.js uses cryptographically random UUIDs for inspector paths specifically to prevent unauthorized access. If the crypto fallback is truly just for edge cases that won't happen in practice, this is acceptable—but worth noting.
🤖 Prompt for AI Agents
In src/js/node/inspector.ts around lines 48–61, the function currently falls
back to Math.random() when crypto is unavailable, producing a predictable
inspector path; replace that insecure fallback with a secure behavior: either
fail-fast by throwing an error (or returning null) when no cryptographic RNG is
available so the inspector cannot start with a predictable URL, or ensure you
obtain CSPRNG bytes from a trusted source (e.g., require a crypto/CSPRNG
polyfill or platform API) and use those bytes to build the pathname; update call
sites to handle the error/null if you choose the fail-fast approach.
| function wsOpen(ws: WebSocket) { | ||
| return new Promise<void>((resolve, reject) => { | ||
| ws.once("open", () => resolve()); | ||
| ws.once("error", err => reject(err)); | ||
| }); | ||
| } | ||
|
|
||
| test("inspector.console", () => { | ||
| function wsMessage(ws: WebSocket) { | ||
| return new Promise<CDPMessage>((resolve, reject) => { | ||
| ws.once("message", data => { | ||
| try { | ||
| resolve(JSON.parse(data.toString())); | ||
| } catch (e) { | ||
| reject(e); | ||
| } | ||
| }); | ||
| ws.once("error", err => reject(err)); | ||
| }); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider cleaning up event listeners to prevent edge cases.
Both wsOpen and wsMessage register two once listeners (success and error), but when one fires, the other remains registered briefly. While unlikely to cause issues in practice due to once, consider using Promise.withResolvers with explicit cleanup for robustness:
🔎 Suggested improvement
function wsOpen(ws: WebSocket) {
- return new Promise<void>((resolve, reject) => {
- ws.once("open", () => resolve());
- ws.once("error", err => reject(err));
- });
+ const { promise, resolve, reject } = Promise.withResolvers<void>();
+ const onOpen = () => { ws.off("error", onError); resolve(); };
+ const onError = (err: Error) => { ws.off("open", onOpen); reject(err); };
+ ws.once("open", onOpen);
+ ws.once("error", onError);
+ return promise;
}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In test/js/node/inspector/inspector.test.ts around lines 13 to 31, the wsOpen
and wsMessage helpers attach two once listeners (success and error) but don't
explicitly remove the counterpart listener when one fires; update each Promise
to ensure the alternate listener is removed on resolution or rejection (for
example call ws.off("error", ...) inside the open handler and ws.off("open",
...) inside the error handler for wsOpen; similarly remove "error" when
"message" resolves and remove "message" when "error" rejects in wsMessage) so no
stray listeners remain and resources are cleaned up.
| test.skip("node:inspector waitForDebugger (no setTimeout, use subprocess connector)", () => { | ||
| // TODO: This test crashes due to a known issue in bun's inspector implementation | ||
| // when the WebSocket connection is closed. The crash occurs in | ||
| // Inspector::FrontendRouter::disconnectFrontend. | ||
| inspector.open(0, "127.0.0.1", false); | ||
| const u = inspector.url(); | ||
| expect(u).toBeString(); | ||
|
|
||
| // Spawn another bun process to connect and signal runIfWaitingForDebugger. | ||
| const connector = Bun.spawn({ | ||
| cmd: [ | ||
| process.execPath, | ||
| "-e", | ||
| ` | ||
| const { WebSocket } = require("ws"); | ||
| const url = process.argv[1]; | ||
| const ws = new WebSocket(url); | ||
| ws.on("open", () => { | ||
| ws.send(JSON.stringify({ id: 1, method: "Runtime.runIfWaitingForDebugger" })); | ||
| ws.close(); | ||
| }); | ||
| ws.on("error", (e) => { | ||
| console.error(e); | ||
| process.exit(2); | ||
| }); | ||
| `, | ||
| u!, | ||
| ], | ||
| stdout: "ignore", | ||
| stderr: "inherit", | ||
| }); | ||
|
|
||
| // Should return once the connector attaches. | ||
| inspector.waitForDebugger(); | ||
|
|
||
| // Ensure connector exits successfully. | ||
| expect(connector.exitCode).toBe(0); | ||
|
|
||
| inspector.close(); | ||
| }); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
When unskipping: await connector.exited before checking exit code.
The test accesses connector.exitCode synchronously, but the subprocess may still be running. When this test is unskipped, use await connector.exited first:
🔎 Suggested fix for future
- // Ensure connector exits successfully.
- expect(connector.exitCode).toBe(0);
+ // Ensure connector exits successfully.
+ expect(await connector.exited).toBe(0);Also, per coding guidelines, consider using await using for Bun.spawn cleanup.
🤖 Prompt for AI Agents
In test/js/node/inspector/inspector.test.ts around lines 89 to 128, the test
reads connector.exitCode synchronously but the spawned subprocess may still be
running; update the test to await connector.exited before asserting the exit
code (e.g., await connector.exited) and, per coding guidelines, wrap the
Bun.spawn in an await using or ensure proper disposal/cleanup so the subprocess
is awaited and closed before calling inspector.close().
a9ace1d to
d671c76
Compare
|
Addressed the feedback:
|
feat(inspector): implement node:inspector open/close/url/waitForDebugger [2/4]
Summary
Implement the core
node:inspectormodule APIs (open,close,url,waitForDebugger) by adding native C++ bindings that communicate with the internal debugger thread via condition variable synchronization.Changes
src/js/node/inspector.tsComplete rewrite of the stub implementation.
New types:
New helper functions:
getBinding()- Retrieves and cachesglobalThis.__bunInspectorparsePort()- Validates port number (0-65535), handles string/number/undefinednormalizeOpenArgs()- Handles Node.js flexible argument patterns (open(true),open(port, true), etc.)formatHostForURL()- Wraps IPv6 literals in bracketsrandomPathname()- Generates UUID-based path for WebSocket URLImplemented functions:
open(port?, host?, wait?)- Formatsws://host:port/uuidURL, calls native bindingclose()- Stops the inspector serverurl()- Returns current WebSocket URL orundefinedwaitForDebugger()- Blocks until debugger connects; opens inspector if not already openSession class:
NotImplementedwith message indicating PR3 will implement itsrc/bun.js/bindings/BunDebugger.cppNew includes:
New extern declarations:
New shared state for cross-thread synchronization:
New helper functions:
nodeInspectorBeginOp()- Marks operation pending, clears errornodeInspectorWaitOp()- Blocks until operation completes, returns error stringnodeInspectorSignalDone()- Signals completion via condition variableNew host functions:
jsFunctionSetInspectorUrl- Called by debugger thread to report final URLjsFunctionReportInspectorError- Called by debugger thread to report errorsjsBunInspectorOpen- Initializes debugger thread, schedules open, waits for resultjsBunInspectorClose- Sends stop command (whitespace URL), waits for completionjsBunInspectorUrl- Returns cached URL (thread-safe read)jsBunInspectorWaitForDebugger- Calls Zig export to block until connectionRefactored functions:
callInternalDebugger()fromBun__startJSDebuggerThreadscheduleInternalDebuggerCall()for cross-thread dispatch viapostTaskConcurrentlyBun__startJSDebuggerThreadnow usescallInternalDebugger()with new callback parametersModified
callInternalDebuggersignature (10 arguments now):Removed dead code:
src/bun.js/bindings/ExposeNodeModuleGlobals.cppNew forward declarations:
New code in
Bun__ExposeNodeModuleGlobals:__bunInspectorobject withopen/close/url/waitForDebuggermethodssrc/bun.js/bindings/InternalModuleRegistry.cppNew forward declarations:
New code in
requireId():id == Field::NodeInspector__bunInspectoralready exists viahasProperty()require("node:inspector"))src/bun.js/Debugger.zigNew exports:
Bun__nodeInspectorInitbehavior:vm.debuggeris initialized (creates if null)from_environment_variable = " "(whitespace = stop command)Bun__nodeInspectorWaitForDebuggerbehavior:wait_for_connection = .forevermust_block_until_connected = truewaitForDebuggerIfNecessary()Removed doc comments (cleanup):
cmake/targets/BuildBun.cmakeWindows build fix:
test/js/node/inspector/inspector.test.tsNew imports:
New helper types and functions:
New tests (4 total):
open/close/url + websocket smoke- Full lifecycle with CDPRuntime.evaluatemessageopen/close loop (state stability)- 3 iterations of open/closeopen error path: port in use throws- Verifies error handling withoutprocess.exit(1)waitForDebugger (skipped)- Skipped due to pre-existing crash inInspector::FrontendRouter::disconnectFrontendRemoved old stub tests:
Thread Safety
nodeInspectorUrl/nodeInspectorErrorprotected bynodeInspectorLockisolatedCopy()for strings crossing thread boundariesError Handling
fatalOnError=falsefor programmatic API → throws JS exceptionfatalOnError=truefor CLI--inspect→ preserves existingprocess.exit(1)behaviorreportError()→nodeInspectorError→throwVMTypeError()Known Issues
waitForDebugger test skipped
The
waitForDebuggertest is skipped (test.skip) due to a pre-existing crash in bun's inspector implementation:This crash occurs when a WebSocket client disconnects from the inspector. The issue is in WebKit's
FrontendRouter::disconnectFrontendand is not introduced by PR2. ThewaitForDebugger()function itself works correctly - the crash only happens during connection teardown.This should be tracked and fixed separately from the node:inspector implementation.
Backward Compatibility
--inspectcontinues to work unchangednode:inspector.url()now reflects--inspectURL (unified state vianodeInspectorUrl)Testing
Dependencies
setInspectorUrl/reportErrorcallbacks andstop()methodRelated
Relates to #2445