Repository navigation
Conversation
- Skip test on Windows (no SIGUSR1 signal) - Accept both macOS (158) and Linux (138) exit codes
- Add tests verifying SIGUSR1 is ignored when process starts with --inspect, --inspect-wait, or --inspect-brk flags - When debugger is already enabled via CLI, RuntimeInspector's signal handler is not installed, and SIGUSR1 is set to SIG_IGN - Fix RuntimeInspector to use sigaction handler instead of sigwait thread (simpler and works regardless of signal blocking state) - Fix test timing issues by increasing timeouts and using direct kill for POSIX-only tests - Add ignoreSigusr1() and setDefaultSigusr1Action() functions for proper signal disposition control 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
…IGUSR1 tests Replace Bun.sleep() calls with condition-based waiting to avoid flaky tests. Instead of waiting for an arbitrary amount of time, now we read from stderr until the expected "Debugger listening" message appears. Changes: - First test: read stderr until "Debugger listening" appears before killing - Third test: wait for "Debugger listening" before sending second SIGUSR1 - Fifth/sixth tests: remove unnecessary sleeps since signal processing is synchronous and we can kill immediately after sending SIGUSR1
…indows inspector tests Replace Bun.sleep() calls with proper condition-based waits that read from stderr until "Debugger listening" appears. This eliminates potential test flakiness caused by timing dependencies.
…untime-inspector tests Replace Bun.sleep() calls with condition-based waits that read from stderr until "Debugger listening" appears. This eliminates potential flakiness from timing-dependent tests. Changes: - Add waitForDebuggerListening() helper to read stderr until message appears - Update "activates inspector in target process" test - Update "inspector does not activate twice" test - Update "can activate inspector in multiple independent processes" test
Add documentation comment explaining that the hardcoded port 6499 may fail if already in use, matching Node.js SIGUSR1 behavior. Users can work around this by pre-configuring --inspect-port or using --inspect=0 for automatic port selection.
…or tests Add exit code assertions for debug helper processes in the "inspector does not activate twice" and "can activate inspector in multiple independent processes" tests for consistency with other tests in the file.
Use toBeOneOf([158, 138]) instead of a boolean check for clearer error messages on test failure. Also clarify the comment to show the full derivation of each expected exit code.
Extract the duplicated SIGUSR1 configuration logic into a helper function to improve maintainability. The same 12-line block was duplicated in initWithModuleGraph, init, and initBake.
- Add PID validation in Windows tests to fail early on invalid PID file content - Add diagnostic logging for Windows file mapping failures - Remove log() call from signal handler context for async-signal-safety - Remove unused previous_action variable from POSIX signal handler
…sage Bun uses a file mapping mechanism for cross-process inspector activation, which produces different error messages than Node.js's native implementation.
Use the same error message as Node.js ('The system cannot find the file
specified.') when the debug handler file mapping doesn't exist, for
compatibility with existing Node.js tests.
Updates WebKit from 1d0216219a3c to d5bd162d9ab2. Key changes: - Promise system refactored to use new callMicrotask variadic template - Added performPromiseThenWithContext for Bun's context passing - Fixed PromiseReactionJob to properly handle empty vs undefined context - Added @then property to JSInternalPromisePrototype - Restored BunPerformMicrotaskJob and BunInvokeJobWithArguments cases Co-Authored-By: Claude <noreply@anthropic.com>
This updates the WebKit version to use the preview build from PR #135 which fixes async context preservation across await for AsyncLocalStorage. Fixes the failing test: test-diagnostics-channel-tracing-channel-promise-run-stores.js
Use $promiseResolveWithThen to wrap promise chains in readableStreamIntoText and readableStreamIntoArray to ensure they return regular Promise instead of InternalPromise. Builtin async functions return InternalPromise by design, but this caused `stream.text() instanceof Promise` to return false. The fix uses the existing $promiseResolveWithThen function which 'shields' InternalPromise by wrapping it in a regular Promise. Claude-Generated-By: Claude Code (cli/claude-opus-4-5=100%) Claude-Steers: 19 Claude-Permission-Prompts: 1 Claude-Escapes: 0
…to ali/siguser1 # Conflicts: # cmake/tools/SetupWebKit.cmake
The runtime inspector tests were failing on Debian 13 (both x64-asan and
aarch64) due to timeout issues. The inspector activation can be slow on
ASAN builds, causing the target processes to exit before the inspector
banner is printed.
Changes:
- Increase test timeouts from 5s to 30s for the target process scripts
- Add explicit { timeout: 30_000 } option to the slow tests
This fixes the CI failures on debian-13-x64-asan and debian-13-aarch64.
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Collaborator
Author
Contributor
WalkthroughThis PR introduces a comprehensive runtime debugger activation mechanism allowing the Bun inspector to be triggered via SIGUSR1 signals on POSIX systems or Windows remote threads. It includes a new Changes
Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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: 13
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/bun.js/bindings/bindings.cpp (1)
3516-3546: Bug: normalizedasyncContextis not passed to the queued microtask (line 3543).You normalize
asyncContexttojsUndefined()when empty (lines 3535–3537), but theQueuedTaskinitializer uses the raw unnormalized value fromglobalObject->m_asyncContextData.get()->getInternalField(0). This defeats the normalization and can reintroduce empty JSValue edge cases. Pass the normalizedasyncContextvariable instead:Fix
- JSC::QueuedTask task { nullptr, JSC::InternalMicrotask::BunPerformMicrotaskJob, 0, globalObject, microtaskFunction, rejectPromiseFunction, globalObject->m_asyncContextData.get()->getInternalField(0), promise, value }; + JSC::QueuedTask task { nullptr, JSC::InternalMicrotask::BunPerformMicrotaskJob, 0, globalObject, microtaskFunction, rejectPromiseFunction, asyncContext, promise, value };src/cli/Arguments.zig (1)
79-92: Clarify platform semantics in--disable-sigusr1help text.
SIGUSR1is POSIX-only; consider reflecting that in the CLI help string so Windows users aren’t confused.Proposed diff
- clap.parseParam("--disable-sigusr1 Disable SIGUSR1 handler for runtime debugger activation") catch unreachable, + clap.parseParam("--disable-sigusr1 Disable SIGUSR1 handler for runtime debugger activation (POSIX only)") catch unreachable,
🤖 Fix all issues with AI agents
In @.claude/commands/upgrade-webkit.md:
- Line 9: The build command is run from the wrong directory: before running the
existing "bun build.ts debug" command you must change back to the bun root
because the previous steps left you in the vendor/WebKit directory; insert a
directory navigation step (e.g. add "cd ../.." or the appropriate "cd" to return
to the bun root) immediately before the "bun build.ts debug" command so the
build script is executed from the correct project root.
- Around line 25-26: The path casing is inconsistent: change the lowercase
"source/WebCore" occurrence to "Source/WebCore" so it matches the other
"Source/..." entries (e.g., the "Source/JavaScriptCore" line); update the string
"source/WebCore" to "Source/WebCore" in the document to fix the incorrect path
casing.
In `@src/bun.js/bindings/BunDebugger.cpp`:
- Around line 687-696: The extern "C" functions VMManager__requestStopAll and
VMManager__requestResumeAll accept an arbitrary uint32_t and cast it directly to
JSC::VMManager::StopReason; add validation or normalization before casting to
prevent invalid bitmasks from propagating into JSC::VMManager. Implement a small
sanitizer that checks the incoming uint32_t against the defined StopReason
values (or a allowed bitmask) and maps/limits unknown bits to a safe default (or
returns early/logs) before calling
JSC::VMManager::requestStopAll/requestResumeAll, or replace the C interface with
a typed wrapper that accepts JSC::VMManager::StopReason and performs the
conversion/validation in one place so only validated enum values are passed into
the VMManager APIs.
In `@src/bun.js/bindings/VMManager.zig`:
- Around line 1-15: The StopReason enum uses enum(u32) which makes bitwise
combinations awkward; replace it with a plain u32 bitflags pattern or add
helpers to make combining/converting ergonomic: change StopReason to a set of
pub const bitflag values (e.g., pub const StopReason_None: u32 = 0; pub const
StopReason_GC: u32 = 1 << 0; ...) or provide helper functions like
stopReasonFromFlags(flags: u32) and stopReasonToInt(reason: StopReason) and
bitwise combinators (e.g., stopReasonHas(reason, flag)) so callers can use GC |
JSDebugger directly without `@intFromEnum/`@enumFromInt plumbing, and update
usages of StopReason in functions to accept/return u32 flags instead of enum.
In `@src/js/builtins/ReadableStreamInternals.ts`:
- Around line 2249-2255: The chain is incorrectly passing a Promise object as a
then-handler: in the expression using
Promise.$resolve(prom).$then(closer.promise).$then($withoutUTF8BOM) you must
pass a function that returns closer.promise so it's awaited; change the then
handler to a function wrapper (e.g. () => closer.promise) instead of
closer.promise. Keep the other branch (closer.promise.$then($withoutUTF8BOM))
as-is.
In `@src/vm/Semaphore.cpp`:
- Around line 55-58: Bun__Semaphore__create currently uses operator new that can
throw; change the allocation to use nothrow new and return nullptr on allocation
failure: allocate with new (std::nothrow) Bun::Semaphore(value), check the
result and return the pointer or nullptr if allocation failed; also add `#include`
<new> if not already present so std::nothrow is available. Ensure this change is
applied in the Bun__Semaphore__create function (and any similar C ABI factory
functions) so exceptions do not cross the extern "C" boundary.
In `@test/js/bun/runtime-inspector/runtime-inspector-posix.test.ts`:
- Around line 8-67: The readiness loop in the test uses a brittle banner count:
replace the loop condition that checks ((stderr.match(/Bun Inspector/g) ||
[]).length < 2) with a stable check for the inspector-ready signal such as
testing for the websocket URL regex (e.g. /ws:\/\/localhost:\d+\//.test(stderr))
and/or the “Debugger listening” line; update the stderr-read loop (the code
using stderrReader, stderrDecoder, and variable stderr) to wait until that regex
matches before releasing the reader and proceeding.
- Around line 288-336: The test comment incorrectly says the RuntimeInspector
signal handler "should NOT be installed" when started with --inspect; update the
comments in the test "SIGUSR1 is ignored when started with --inspect" and the
inline comment before process.kill to state that the VM explicitly sets SIGUSR1
to be ignored (so the signal is ignored), rather than claiming the handler is
not installed, so the expectation and wording match the VM behavior.
- Around line 69-121: The stdout stream reader in the test "user SIGUSR1
listener takes precedence over inspector activation" is not released; call
reader.releaseLock() after finishing reads (or replace manual reads with await
proc.stdout.text()) to ensure the ReadableStream lock is freed and to simplify
failure handling — update the code around the ReadableStream reader usage and
finalization so reader.releaseLock() is invoked (or use proc.stdout.text()
instead) before awaiting proc.stderr.text() and proc.exited.
- Around line 123-197: The test leaves the stdout reader locked, causing cleanup
issues; after finishing all reads (the loops that await reader.read() and before
awaiting proc.exited / proc.stderr.text()), explicitly cancel or release the
stream reader (call reader.cancel() or reader.releaseLock()) — ideally in a
finally block — so the reader is freed and the process can be awaited and
cleaned up correctly; update the test "multiple SIGUSR1s work after user
installs handler" to call reader.cancel()/reader.releaseLock() on the `reader`
created from `proc.stdout.getReader()`.
- Around line 338-380: The tests call proc.kill() (and send SIGUSR1) but never
assert how the child actually exited; after awaiting proc.exited add an explicit
assertion on the exit semantics (e.g. check proc.exitSignal === "SIGTERM" or
proc.exitCode as appropriate) in addition to the existing inspector-banner
assertion so the test verifies both that the inspector was not reactivated (the
expect(matches?.length ?? 0).toBe(2) check) and that the process terminated due
to the intended kill; make the same change in the sibling test that also uses
proc.kill() and process.kill(proc.pid, "SIGUSR1").
- Around line 263-287: The child currently self-exits after 300ms which races
with inspector activation; remove the hardcoded setTimeout(() =>
process.exit(0), 300) from the spawned script so the child stays alive (keep the
setInterval), then in the test "SIGUSR1 to self activates inspector" wait for
the inspector banner by reading proc.stderr (the existing
proc.stderr.text()/expect(stderr).toContain("Bun Inspector")), and only then
terminate the child from the test (e.g., call proc.kill() and await proc.exited)
so the test deterministically waits for activation before exit.
📜 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 (33)
.claude/commands/upgrade-webkit.mdcmake/tools/SetupWebKit.cmakesrc/bun.js.zigsrc/bun.js/VirtualMachine.zigsrc/bun.js/bindings/BunDebugger.cppsrc/bun.js/bindings/BunGCOutputConstraint.hsrc/bun.js/bindings/BunProcess.cppsrc/bun.js/bindings/VMManager.zigsrc/bun.js/bindings/ZigGlobalObject.cppsrc/bun.js/bindings/bindings.cppsrc/bun.js/bindings/node/crypto/CryptoSignJob.hsrc/bun.js/bindings/webcore/EventInit.hsrc/bun.js/bindings/webcore/HTTPHeaderMap.hsrc/bun.js/bindings/webcore/PerformanceUserTiming.cppsrc/bun.js/bindings/webcrypto/JSCryptoKey.cppsrc/bun.js/bindings/webcrypto/JSCryptoKeyUsage.cppsrc/bun.js/bindings/webcrypto/JSSubtleCrypto.cppsrc/bun.js/event_loop.zigsrc/bun.js/event_loop/RuntimeInspector.zigsrc/bun.js/jsc.zigsrc/cli.zigsrc/cli/Arguments.zigsrc/cli/test_command.zigsrc/codegen/bindgen.tssrc/codegen/bindgenv2/internal/enumeration.tssrc/codegen/bundle-functions.tssrc/js/builtins/ReadableStreamInternals.tssrc/sync/Semaphore.zigsrc/vm/Semaphore.cpptest/js/bun/runtime-inspector/runtime-inspector-posix.test.tstest/js/bun/runtime-inspector/runtime-inspector-windows.test.tstest/js/bun/runtime-inspector/runtime-inspector.test.tstest/js/node/process/process.test.js
💤 Files with no reviewable changes (1)
- test/js/node/process/process.test.js
🧰 Additional context used
📓 Path-based instructions (8)
**/*.zig
📄 CodeRabbit inference engine (CLAUDE.md)
In Zig code, be careful with allocators and use defer for cleanup
Files:
src/bun.js/event_loop.zigsrc/bun.js/bindings/VMManager.zigsrc/bun.js/jsc.zigsrc/cli/Arguments.zigsrc/cli.zigsrc/bun.js.zigsrc/cli/test_command.zigsrc/bun.js/VirtualMachine.zigsrc/sync/Semaphore.zigsrc/bun.js/event_loop/RuntimeInspector.zig
src/**/*.zig
📄 CodeRabbit inference engine (src/CLAUDE.md)
src/**/*.zig: Use the#prefix for private fields in Zig structs, e.g.,struct {#foo: u32 };
Use Decl literals in Zig, e.g.,const decl: Decl = .{ .binding = 0, .value = 0 };
Place@importstatements at the bottom of the file in Zig (auto formatter will handle positioning)
Never use@import()inline inside functions in Zig; always place imports at the bottom of the file or containing struct
Files:
src/bun.js/event_loop.zigsrc/bun.js/bindings/VMManager.zigsrc/bun.js/jsc.zigsrc/cli/Arguments.zigsrc/cli.zigsrc/bun.js.zigsrc/cli/test_command.zigsrc/bun.js/VirtualMachine.zigsrc/sync/Semaphore.zigsrc/bun.js/event_loop/RuntimeInspector.zig
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/webcrypto/JSSubtleCrypto.cppsrc/bun.js/bindings/BunDebugger.cppsrc/bun.js/bindings/webcrypto/JSCryptoKey.cppsrc/bun.js/bindings/webcore/PerformanceUserTiming.cppsrc/bun.js/bindings/bindings.cppsrc/bun.js/bindings/webcrypto/JSCryptoKeyUsage.cppsrc/bun.js/bindings/ZigGlobalObject.cppsrc/bun.js/bindings/BunProcess.cpp
**/*.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/bun/runtime-inspector/runtime-inspector-windows.test.tstest/js/bun/runtime-inspector/runtime-inspector.test.tstest/js/bun/runtime-inspector/runtime-inspector-posix.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/bun/runtime-inspector/runtime-inspector-windows.test.tstest/js/bun/runtime-inspector/runtime-inspector.test.tstest/js/bun/runtime-inspector/runtime-inspector-posix.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 bd test <...test file>to run tests with compiled code changes. Do not usebun testas it will not include your changes.
Usebun:testfor files ending in*.test.{ts,js,jsx,tsx,mjs,cjs}. For test files without .test extension in test/js/node/test/{parallel,sequential}/*.js, usebun bd <file>instead ofbun bd test <file>since they expect exit code 0.
Do not set a timeout on tests. Bun already has timeouts built-in.
Files:
test/js/bun/runtime-inspector/runtime-inspector-windows.test.tstest/js/bun/runtime-inspector/runtime-inspector.test.tstest/js/bun/runtime-inspector/runtime-inspector-posix.test.ts
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/builtins/ReadableStreamInternals.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/builtins/ReadableStreamInternals.ts
🧠 Learnings (69)
📓 Common learnings
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2026-01-05T23:04:01.518Z
Learning: Applies to test/**/*.test.{ts,js,jsx,tsx,mjs,cjs} : Do not set a timeout on tests. Bun already has timeouts built-in.
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2026-01-05T23:04:01.518Z
Learning: Applies to test/**/*.test.{ts,js,jsx,tsx,mjs,cjs} : Use `bun bd test <...test file>` to run tests with compiled code changes. Do not use `bun test` as it will not include your changes.
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.
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
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-16T00:21:32.179Z
Learning: Do not set a timeout when running `bun bd` (debug build command)
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;`.
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2026-01-05T23:04:01.518Z
Learning: Applies to test/**/*.test.{ts,js,jsx,tsx,mjs,cjs} : Use `bun:test` for files ending in `*.test.{ts,js,jsx,tsx,mjs,cjs}`. For test files without .test extension in test/js/node/test/{parallel,sequential}/*.js, use `bun bd <file>` instead of `bun bd test <file>` since they expect exit code 0.
Learnt from: alii
Repo: oven-sh/bun PR: 25474
File: src/bun.js/event_loop/RuntimeInspector.zig:26-26
Timestamp: 2026-01-05T16:48:00.280Z
Learning: In RuntimeInspector.zig (Bun's SIGUSR1 debugger activation handler), the inspector port 6499 is intentionally hardcoded to match Node.js's behavior (which uses hardcoded port 9229 for SIGUSR1 activation). This is a known limitation that matches Node.js parity - if the port is occupied, activation fails gracefully with an error message. Users can work around this by starting the process with --inspect-port=<port> or --inspect=0 for automatic port selection.
📚 Learning: 2026-01-05T23:04:01.518Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2026-01-05T23:04:01.518Z
Learning: Applies to test/**/*.test.{ts,js,jsx,tsx,mjs,cjs} : Use `bun bd test <...test file>` to run tests with compiled code changes. Do not use `bun test` as it will not include your changes.
Applied to files:
.claude/commands/upgrade-webkit.mdtest/js/bun/runtime-inspector/runtime-inspector-windows.test.tstest/js/bun/runtime-inspector/runtime-inspector.test.tstest/js/bun/runtime-inspector/runtime-inspector-posix.test.ts
📚 Learning: 2025-11-14T16:07:01.064Z
Learnt from: RiskyMH
Repo: oven-sh/bun PR: 24719
File: docs/bundler/executables.mdx:527-560
Timestamp: 2025-11-14T16:07:01.064Z
Learning: In the Bun repository, certain bundler features like compile with code splitting (--compile --splitting) are CLI-only and not supported in the Bun.build() JavaScript API. Tests for CLI-only features use backend: "cli" flag (e.g., test/bundler/bundler_compile_splitting.test.ts). The CompileBuildConfig interface correctly restricts these with splitting?: never;. When documenting CLI-only bundler features, add a note clarifying they're not available via the programmatic API.
Applied to files:
.claude/commands/upgrade-webkit.mdsrc/codegen/bundle-functions.ts
📚 Learning: 2025-11-24T18:37:11.466Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/js/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:11.466Z
Learning: The build process converts source TS/JS through preprocessing, bundling, and C++ header generation; assign numeric IDs in A-Z sorted order
Applied to files:
.claude/commands/upgrade-webkit.md
📚 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:
.claude/commands/upgrade-webkit.md
📚 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: Run `bun run zig:check-all` to compile Zig code on all platforms when making platform-specific changes
Applied to files:
.claude/commands/upgrade-webkit.md
📚 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:
.claude/commands/upgrade-webkit.mdtest/js/bun/runtime-inspector/runtime-inspector-windows.test.tstest/js/bun/runtime-inspector/runtime-inspector.test.tstest/js/bun/runtime-inspector/runtime-inspector-posix.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: Only push changes after running `bun bd test <file>` and ensuring tests pass
Applied to files:
.claude/commands/upgrade-webkit.md
📚 Learning: 2026-01-05T23:04:01.518Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2026-01-05T23:04:01.518Z
Learning: Applies to test/**/*.test.{ts,js,jsx,tsx,mjs,cjs} : Use `bun:test` for files ending in `*.test.{ts,js,jsx,tsx,mjs,cjs}`. For test files without .test extension in test/js/node/test/{parallel,sequential}/*.js, use `bun bd <file>` instead of `bun bd test <file>` since they expect exit code 0.
Applied to files:
.claude/commands/upgrade-webkit.mdtest/js/bun/runtime-inspector/runtime-inspector-windows.test.tstest/js/bun/runtime-inspector/runtime-inspector.test.tstest/js/bun/runtime-inspector/runtime-inspector-posix.test.ts
📚 Learning: 2025-11-24T18:36:59.706Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:36:59.706Z
Learning: Applies to src/bun.js/bindings/v8/test/v8/v8.test.ts : Add corresponding test cases to test/v8/v8.test.ts using checkSameOutput() function to compare Node.js and Bun output
Applied to files:
.claude/commands/upgrade-webkit.mdtest/js/bun/runtime-inspector/runtime-inspector-windows.test.tstest/js/bun/runtime-inspector/runtime-inspector.test.tstest/js/bun/runtime-inspector/runtime-inspector-posix.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:
.claude/commands/upgrade-webkit.mdtest/js/bun/runtime-inspector/runtime-inspector-windows.test.tstest/js/bun/runtime-inspector/runtime-inspector.test.tstest/js/bun/runtime-inspector/runtime-inspector-posix.test.ts
📚 Learning: 2026-01-05T16:48:00.280Z
Learnt from: alii
Repo: oven-sh/bun PR: 25474
File: src/bun.js/event_loop/RuntimeInspector.zig:26-26
Timestamp: 2026-01-05T16:48:00.280Z
Learning: In RuntimeInspector.zig (Bun's SIGUSR1 debugger activation handler), the inspector port 6499 is intentionally hardcoded to match Node.js's behavior (which uses hardcoded port 9229 for SIGUSR1 activation). This is a known limitation that matches Node.js parity - if the port is occupied, activation fails gracefully with an error message. Users can work around this by starting the process with --inspect-port=<port> or --inspect=0 for automatic port selection.
Applied to files:
src/bun.js/event_loop.zigtest/js/bun/runtime-inspector/runtime-inspector-windows.test.tstest/js/bun/runtime-inspector/runtime-inspector.test.tssrc/cli/Arguments.zigsrc/bun.js/bindings/BunDebugger.cpptest/js/bun/runtime-inspector/runtime-inspector-posix.test.tssrc/bun.js/VirtualMachine.zigsrc/bun.js/event_loop/RuntimeInspector.zigsrc/bun.js/bindings/BunProcess.cpp
📚 Learning: 2026-01-05T16:32:07.551Z
Learnt from: alii
Repo: oven-sh/bun PR: 25474
File: src/bun.js/event_loop/Sigusr1Handler.zig:0-0
Timestamp: 2026-01-05T16:32:07.551Z
Learning: In Zig codebases (e.g., Bun), treat std.posix.sigaction as returning void and do not perform runtime error handling for its failure. The Zig standard library views sigaction failures as programmer errors (unreachable) because they only occur with invalid signals like SIGKILL/SIGSTOP. Apply this pattern across Zig files that call sigaction (e.g., crash_handler.zig, main.zig, filter_run.zig, process.zig) and ensure failures are not handled as recoverable errors; prefer reaching an explicit unreachable/compile-time assumption when such failures are detected.
Applied to files:
src/bun.js/event_loop.zigsrc/bun.js/bindings/VMManager.zigsrc/bun.js/jsc.zigsrc/cli/Arguments.zigsrc/cli.zigsrc/bun.js.zigsrc/cli/test_command.zigsrc/bun.js/VirtualMachine.zigsrc/sync/Semaphore.zigsrc/bun.js/event_loop/RuntimeInspector.zig
📚 Learning: 2025-10-01T21:59:54.571Z
Learnt from: taylordotfish
Repo: oven-sh/bun PR: 23169
File: src/bun.js/bindings/webcore/JSDOMConvertEnumeration.h:47-74
Timestamp: 2025-10-01T21:59:54.571Z
Learning: In the new bindings generator (bindgenv2) for `src/bun.js/bindings/webcore/JSDOMConvertEnumeration.h`, the context-aware enumeration conversion overloads intentionally use stricter validation (requiring `value.isString()` without ToString coercion), diverging from Web IDL semantics. This is a design decision documented in comments.
Applied to files:
src/bun.js/bindings/webcore/EventInit.hsrc/bun.js/bindings/webcrypto/JSSubtleCrypto.cppsrc/codegen/bindgenv2/internal/enumeration.tssrc/codegen/bindgen.tssrc/bun.js/bindings/webcore/HTTPHeaderMap.hsrc/bun.js/bindings/webcrypto/JSCryptoKey.cppsrc/bun.js/bindings/BunGCOutputConstraint.hsrc/bun.js/bindings/webcore/PerformanceUserTiming.cppsrc/bun.js/bindings/bindings.cppsrc/bun.js/bindings/webcrypto/JSCryptoKeyUsage.cpp
📚 Learning: 2025-12-23T06:50:31.577Z
Learnt from: Jarred-Sumner
Repo: oven-sh/bun PR: 25429
File: src/bun.js/bindings/helpers.h:422-422
Timestamp: 2025-12-23T06:50:31.577Z
Learning: In Bun's C++ bindings, 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. Apply this guidance to src/bun.js/bindings/helpers.h and similar header files in the same bindings directory (i.e., any file that constructs an EmptyIdentifier).
Applied to files:
src/bun.js/bindings/webcore/EventInit.hsrc/bun.js/bindings/node/crypto/CryptoSignJob.hsrc/bun.js/bindings/webcore/HTTPHeaderMap.hsrc/bun.js/bindings/BunGCOutputConstraint.h
📚 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/bindings/VMManager.zigsrc/bun.js/jsc.zigsrc/bun.js/bindings/BunDebugger.cppsrc/bun.js.zigsrc/vm/Semaphore.cppsrc/bun.js/VirtualMachine.zigsrc/bun.js/event_loop/RuntimeInspector.zigsrc/bun.js/bindings/ZigGlobalObject.cpp
📚 Learning: 2025-10-18T20:50:47.750Z
Learnt from: theshadow27
Repo: oven-sh/bun PR: 23798
File: src/bun.js/telemetry.zig:366-373
Timestamp: 2025-10-18T20:50:47.750Z
Learning: In Bun's Zig codebase (src/bun.js/bindings/JSValue.zig), the JSValue enum uses `.null` (not `.js_null`) for JavaScript's null value. Only `js_undefined` has the `js_` prefix to avoid collision with Zig's built-in `undefined` keyword. The correct enum fields are: `js_undefined`, `null`, `true`, `false`, and `zero`.
Applied to files:
src/bun.js/bindings/VMManager.zigsrc/bun.js/jsc.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 : Define properties using HashTableValue arrays in C++ JavaScript class bindings
Applied to files:
src/bun.js/bindings/webcrypto/JSSubtleCrypto.cppsrc/codegen/bindgen.tssrc/bun.js/bindings/webcrypto/JSCryptoKey.cppsrc/bun.js/bindings/webcore/PerformanceUserTiming.cppsrc/bun.js/bindings/webcrypto/JSCryptoKeyUsage.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/webcrypto/JSSubtleCrypto.cppsrc/bun.js/bindings/node/crypto/CryptoSignJob.hsrc/bun.js/jsc.zigsrc/bun.js/bindings/BunDebugger.cppsrc/bun.js/bindings/webcrypto/JSCryptoKey.cppsrc/bun.js/bindings/BunGCOutputConstraint.hsrc/bun.js/bindings/webcore/PerformanceUserTiming.cppsrc/bun.js/bindings/bindings.cppsrc/bun.js/bindings/ZigGlobalObject.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/webcrypto/JSSubtleCrypto.cppsrc/bun.js/jsc.zigsrc/vm/Semaphore.cppsrc/bun.js/bindings/webcore/PerformanceUserTiming.cppsrc/bun.js/bindings/bindings.cppsrc/bun.js/bindings/ZigGlobalObject.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 **/*.test.ts?(x) : For multi-file tests, prefer `tempDir` and `Bun.spawn` over single-file tests
Applied to files:
test/js/bun/runtime-inspector/runtime-inspector-windows.test.tstest/js/bun/runtime-inspector/runtime-inspector.test.tstest/js/bun/runtime-inspector/runtime-inspector-posix.test.ts
📚 Learning: 2026-01-05T23:04:01.518Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2026-01-05T23:04:01.518Z
Learning: Applies to test/**/*-fixture.ts : Test files that spawn Bun processes should end in `*-fixture.ts` to identify them as test fixtures rather than tests themselves.
Applied to files:
test/js/bun/runtime-inspector/runtime-inspector-windows.test.tstest/js/bun/runtime-inspector/runtime-inspector.test.tstest/js/bun/runtime-inspector/runtime-inspector-posix.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/bun/runtime-inspector/runtime-inspector-windows.test.tstest/js/bun/runtime-inspector/runtime-inspector.test.tstest/js/bun/runtime-inspector/runtime-inspector-posix.test.ts
📚 Learning: 2025-11-24T18:37:11.466Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/js/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:11.466Z
Learning: Applies to src/js/{builtins,node,bun,thirdparty,internal}/**/*.{ts,js} : Use `process.platform` and `process.arch` for platform detection; these values are inlined and dead-code eliminated at build time
Applied to files:
test/js/bun/runtime-inspector/runtime-inspector-windows.test.tstest/js/bun/runtime-inspector/runtime-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) : 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/bun/runtime-inspector/runtime-inspector-windows.test.tstest/js/bun/runtime-inspector/runtime-inspector.test.tstest/js/bun/runtime-inspector/runtime-inspector-posix.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/bun/runtime-inspector/runtime-inspector-windows.test.tstest/js/bun/runtime-inspector/runtime-inspector.test.tstest/js/bun/runtime-inspector/runtime-inspector-posix.test.ts
📚 Learning: 2026-01-05T23:04:01.518Z
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.
Applied to files:
test/js/bun/runtime-inspector/runtime-inspector-windows.test.tstest/js/bun/runtime-inspector/runtime-inspector.test.tstest/js/bun/runtime-inspector/runtime-inspector-posix.test.tssrc/bun.js/bindings/BunProcess.cpp
📚 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/bun/runtime-inspector/runtime-inspector-windows.test.tstest/js/bun/runtime-inspector/runtime-inspector.test.tstest/js/bun/runtime-inspector/runtime-inspector-posix.test.ts
📚 Learning: 2026-01-05T23:04:01.518Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2026-01-05T23:04:01.518Z
Learning: Use `-e` flag for single-file tests when spawning Bun processes with `Bun.spawn()`.
Applied to files:
test/js/bun/runtime-inspector/runtime-inspector-windows.test.tstest/js/bun/runtime-inspector/runtime-inspector-posix.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/jsc.zigsrc/bun.js/bindings/BunDebugger.cppsrc/bun.js.zigsrc/bun.js/bindings/bindings.cppsrc/bun.js/bindings/ZigGlobalObject.cppsrc/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/jsc.zigsrc/bun.js/bindings/BunDebugger.cppsrc/bun.js/bindings/webcore/PerformanceUserTiming.cppsrc/bun.js/bindings/bindings.cppsrc/bun.js/bindings/ZigGlobalObject.cpp
📚 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/jsc.zigsrc/bun.js/bindings/BunDebugger.cppsrc/bun.js/bindings/webcore/PerformanceUserTiming.cppsrc/bun.js/bindings/bindings.cppsrc/bun.js/bindings/webcrypto/JSCryptoKeyUsage.cppsrc/bun.js/bindings/ZigGlobalObject.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/jsc.zigsrc/bun.js/bindings/BunDebugger.cppsrc/bun.js.zigsrc/vm/Semaphore.cppsrc/bun.js/bindings/BunGCOutputConstraint.hsrc/codegen/bundle-functions.tssrc/bun.js/bindings/webcore/PerformanceUserTiming.cppsrc/bun.js/bindings/bindings.cppsrc/bun.js/bindings/ZigGlobalObject.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/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/jsc.zigsrc/bun.js/bindings/BunDebugger.cppsrc/vm/Semaphore.cppsrc/bun.js/bindings/webcore/PerformanceUserTiming.cppsrc/bun.js/bindings/ZigGlobalObject.cpp
📚 Learning: 2025-10-11T15:19:30.301Z
Learnt from: mastermakrela
Repo: oven-sh/bun PR: 19167
File: src/bun.js/api.zig:31-31
Timestamp: 2025-10-11T15:19:30.301Z
Learning: The learnings about `pub const js = JSC.Codegen.JS<ClassName>` and re-exporting toJS/fromJS/fromJSDirect apply to class-based Zig bindings with constructors and prototype methods (e.g., those with `new` constructors). They do NOT apply to simple namespace-style API objects like TOMLObject, YAMLObject, and CSVObject which expose static functions via a `create()` method that returns a plain JS object.
Applied to files:
src/bun.js/jsc.zig
📚 Learning: 2025-08-30T09:09:18.384Z
Learnt from: Jarred-Sumner
Repo: oven-sh/bun PR: 22231
File: src/bundler/bundle_v2.zig:48-48
Timestamp: 2025-08-30T09:09:18.384Z
Learning: In Zig, when a module exports a top-level struct, import("./Module.zig") directly returns that struct type and can be used as a type alias without needing to access a field within the module. This is a common pattern in the Bun codebase.
Applied to files:
src/bun.js/jsc.zig
📚 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:
test/js/bun/runtime-inspector/runtime-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/bun/runtime-inspector/runtime-inspector.test.ts
📚 Learning: 2026-01-05T16:49:09.447Z
Learnt from: alii
Repo: oven-sh/bun PR: 25474
File: test/js/bun/runtime-inspector/runtime-inspector.test.ts:93-93
Timestamp: 2026-01-05T16:49:09.447Z
Learning: In Bun test files testing process operations (e.g., process._debugProcess), when testing for non-existent process behavior, using fakePid = 999999999 is safe and preferred. The maximum configurable pid_max on Linux 64-bit is 2^22 = 4,194,304, so 999999999 is guaranteed to never be a valid PID on any supported platform. PID 0 should not be used because it exists (kernel scheduler/swapper) and would test a permission error path rather than "process not found".
Applied to files:
test/js/bun/runtime-inspector/runtime-inspector.test.ts
📚 Learning: 2025-10-16T02:17:35.237Z
Learnt from: markovejnovic
Repo: oven-sh/bun PR: 23710
File: src/analytics.zig:15-21
Timestamp: 2025-10-16T02:17:35.237Z
Learning: In src/analytics.zig and similar files using bun.EnvVar boolean environment variables: the new EnvVar API for boolean flags (e.g., bun.EnvVar.do_not_track.get(), bun.EnvVar.ci.get()) is designed to parse and return boolean values from environment variables, not just check for their presence. This is an intentional design change from the previous presence-based checks using bun.getenvZ().
Applied to files:
src/cli/Arguments.zigsrc/bun.js.zigsrc/bun.js/VirtualMachine.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/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/BunDebugger.cppsrc/bun.js/bindings/BunGCOutputConstraint.hsrc/bun.js/bindings/webcore/PerformanceUserTiming.cppsrc/bun.js/bindings/bindings.cppsrc/bun.js/bindings/ZigGlobalObject.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/BunDebugger.cppsrc/bun.js/bindings/BunGCOutputConstraint.hsrc/bun.js/bindings/ZigGlobalObject.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/BunDebugger.cppsrc/bun.js/bindings/BunGCOutputConstraint.hsrc/bun.js/bindings/bindings.cppsrc/bun.js/bindings/ZigGlobalObject.cpp
📚 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/bindings/BunDebugger.cpp
📚 Learning: 2026-01-05T16:32:15.175Z
Learnt from: alii
Repo: oven-sh/bun PR: 25474
File: src/bun.js/event_loop/Sigusr1Handler.zig:0-0
Timestamp: 2026-01-05T16:32:15.175Z
Learning: In Bun's Zig codebase, std.posix.sigaction returns void (not an error union) and should be called without error handling. The Zig standard library treats sigaction failures as programmer errors (unreachable) since they only occur with invalid signals like SIGKILL/SIGSTOP. This pattern is used consistently across crash_handler.zig, main.zig, filter_run.zig, and process.zig.
Applied to files:
src/bun.js/bindings/BunDebugger.cppsrc/bun.js/bindings/BunProcess.cpp
📚 Learning: 2025-10-25T17:20:19.041Z
Learnt from: theshadow27
Repo: oven-sh/bun PR: 24063
File: test/js/bun/telemetry/server-header-injection.test.ts:5-20
Timestamp: 2025-10-25T17:20:19.041Z
Learning: In the Bun telemetry codebase, tests are organized into two distinct layers: (1) Internal API tests in test/js/bun/telemetry/ use numeric InstrumentKind enum values to test Zig↔JS injection points and low-level integration; (2) Public API tests in packages/bun-otel/test/ use string InstrumentKind values ("http", "fetch", etc.) to test the public-facing BunSDK and instrumentation APIs. This separation allows internal tests to use efficient numeric enums for refactoring flexibility while the public API maintains a developer-friendly string-based interface.
Applied to files:
test/js/bun/runtime-inspector/runtime-inspector-posix.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/bun/runtime-inspector/runtime-inspector-posix.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: Use `BUN_DEBUG_QUIET_LOGS=1` to disable debug logging, or `BUN_DEBUG_<scopeName>=1` to enable specific scoped debug logging in Zig
Applied to files:
src/bun.js.zigsrc/bun.js/VirtualMachine.zig
📚 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:
src/vm/Semaphore.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/vm/Semaphore.cppsrc/bun.js/bindings/BunGCOutputConstraint.hsrc/bun.js/bindings/ZigGlobalObject.cpp
📚 Learning: 2025-10-15T20:19:38.580Z
Learnt from: markovejnovic
Repo: oven-sh/bun PR: 23680
File: cmake/targets/BuildBun.cmake:822-822
Timestamp: 2025-10-15T20:19:38.580Z
Learning: In the Bun codebase, FFI is compiled with tcc (TinyCC), which barely supports C99. The headers `src/bun.js/api/FFI.h` and `src/bun.js/api/ffi-stdbool.h` are only used for FFI compilation with tcc, not for the main Bun target. Therefore, C23 compatibility concerns (such as bool/true/false keyword conflicts) do not apply to these FFI headers.
Applied to files:
src/vm/Semaphore.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 : Include ASSERT_V8_TYPE_LAYOUT_MATCHES(v8::ClassName) macro at the beginning of each V8 class implementation file to verify type layout compatibility with V8
Applied to files:
src/bun.js/bindings/BunGCOutputConstraint.hsrc/bun.js/bindings/ZigGlobalObject.cpp
📚 Learning: 2025-10-17T01:20:22.188Z
Learnt from: taylordotfish
Repo: oven-sh/bun PR: 23755
File: src/bun.js/bindings/BunIDLConvert.h:72-97
Timestamp: 2025-10-17T01:20:22.188Z
Learning: In Bun's IDL bindings (src/bun.js/bindings/BunIDLConvert.h), converters inheriting from `Bun::DefaultTryConverter` automatically get `conversionHasSideEffects = true` via the inheritance chain (`DefaultTryConverter` → `WebCore::DefaultConverter`). No explicit declaration of `conversionHasSideEffects` is needed when using `DefaultTryConverter` as the base.
Applied to files:
src/bun.js/bindings/BunGCOutputConstraint.h
📚 Learning: 2025-11-24T18:37:11.466Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/js/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:11.466Z
Learning: Applies to src/js/{builtins,node,bun,thirdparty,internal}/**/*.{ts,js} : Use private globals and methods with `$` prefix (e.g., `$Array`, `map.$set()`) instead of public JavaScript globals
Applied to files:
src/codegen/bundle-functions.ts
📚 Learning: 2025-11-24T18:37:11.466Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/js/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:11.466Z
Learning: Applies to src/js/{builtins,node,bun,thirdparty,internal}/**/*.ts : Builtin functions must include `this` parameter typing in TypeScript to enable direct method binding in C++
Applied to files:
src/codegen/bundle-functions.ts
📚 Learning: 2025-11-24T18:37:11.466Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/js/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:11.466Z
Learning: Applies to src/js/{builtins,node,bun,thirdparty,internal}/**/*.{ts,js} : Use JSC intrinsics (prefixed with `$`) such as `$Array.from()`, `$isCallable()`, and `$newArrayWithSize()` for performance-critical operations
Applied to files:
src/bun.js/bindings/webcore/PerformanceUserTiming.cpp
📚 Learning: 2025-10-01T21:49:27.862Z
Learnt from: taylordotfish
Repo: oven-sh/bun PR: 23169
File: src/bun.js/bindings/BunIDLConvert.h:29-42
Timestamp: 2025-10-01T21:49:27.862Z
Learning: In Bun's IDL bindings (src/bun.js/bindings/BunIDLConvert.h), IDLStrictNull intentionally treats both undefined and null as null (using isUndefinedOrNull()), matching WebKit's IDLNull & IDLNullable behavior. This is the correct implementation and should not be changed to only accept null.
Applied to files:
src/bun.js/bindings/bindings.cpp
📚 Learning: 2025-09-19T19:55:22.427Z
Learnt from: markovejnovic
Repo: oven-sh/bun PR: 22568
File: src/valkey/js_valkey_functions.zig:853-854
Timestamp: 2025-09-19T19:55:22.427Z
Learning: In Bun's JSPromise API, `resolvedPromiseValue(globalThis: *JSGlobalObject, value: JSValue) JSValue` returns a JSValue directly (not a JSPromise that needs `.toJS()` called on it). This is different from `resolvedPromise()` which returns a *JSPromise that would need `.toJS()`.
Applied to files:
src/bun.js/bindings/bindings.cpp
📚 Learning: 2025-09-05T19:49:26.188Z
Learnt from: markovejnovic
Repo: oven-sh/bun PR: 21728
File: src/valkey/js_valkey_functions.zig:852-867
Timestamp: 2025-09-05T19:49:26.188Z
Learning: In Bun's Zig codebase, `.js_undefined` is a valid way to represent JavaScript's undefined value when working with JSPromise.resolve() and similar JavaScript interop functions. This is the correct pattern to use rather than `jsc.JSValue.jsUndefined()`.
Applied to files:
src/bun.js/bindings/bindings.cpp
📚 Learning: 2025-09-05T19:49:26.188Z
Learnt from: markovejnovic
Repo: oven-sh/bun PR: 21728
File: src/valkey/js_valkey_functions.zig:852-867
Timestamp: 2025-09-05T19:49:26.188Z
Learning: In Bun’s Zig code, `.js_undefined` is a valid and preferred JSValue literal for “undefined” (e.g., resolving JSPromise). Do not refactor usages to `jsc.JSValue.jsUndefined()`, especially in src/valkey/js_valkey_functions.zig unsubscribe().
Applied to files:
src/bun.js/bindings/bindings.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/bindings.cppsrc/bun.js/bindings/ZigGlobalObject.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/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/bindings.cppsrc/bun.js/bindings/ZigGlobalObject.cppsrc/bun.js/bindings/BunProcess.cpp
📚 Learning: 2025-09-02T17:14:46.924Z
Learnt from: taylordotfish
Repo: oven-sh/bun PR: 22227
File: src/safety/alloc.zig:93-95
Timestamp: 2025-09-02T17:14:46.924Z
Learning: In bun's Zig codebase, they use a custom extension of Zig that supports private field syntax with the `#` prefix (e.g., `#allocator`, `#trace`). This is not standard Zig syntax but is valid in their custom implementation. Fields prefixed with `#` are private fields that cannot be accessed from outside the defining struct.
Applied to files:
src/sync/Semaphore.zig
📚 Learning: 2025-11-12T04:11:52.293Z
Learnt from: cirospaciari
Repo: oven-sh/bun PR: 24622
File: src/deps/uws/us_socket_t.zig:112-113
Timestamp: 2025-11-12T04:11:52.293Z
Learning: In Bun's Zig codebase, when passing u32 values to C FFI functions that expect c_uint parameters, no explicit intCast is needed because c_uint is equivalent to u32 on Bun's target platforms and Zig allows implicit coercion between equivalent types. This pattern is used consistently throughout src/deps/uws/us_socket_t.zig in functions like setTimeout, setLongTimeout, and setKeepalive.
Applied to files:
src/sync/Semaphore.zig
📚 Learning: 2025-09-02T18:25:27.976Z
Learnt from: taylordotfish
Repo: oven-sh/bun PR: 22227
File: src/allocators/allocation_scope.zig:284-314
Timestamp: 2025-09-02T18:25:27.976Z
Learning: In bun's custom Zig implementation, the `#` prefix for private fields is valid syntax and should not be flagged as invalid. The syntax `#fieldname` creates private fields that cannot be accessed from outside the defining struct, and usage like `self.#fieldname` is correct within the same struct. This applies to fields like `#parent`, `#state`, `#allocator`, `#trace`, etc. throughout the codebase.
Applied to files:
src/sync/Semaphore.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 : Register new V8 API test functions in the Init method using NODE_SET_METHOD with exports object
Applied to files:
src/bun.js/bindings/ZigGlobalObject.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/ZigGlobalObject.cpp
📚 Learning: 2025-10-15T20:19:37.256Z
Learnt from: markovejnovic
Repo: oven-sh/bun PR: 23680
File: cmake/targets/BuildBun.cmake:822-822
Timestamp: 2025-10-15T20:19:37.256Z
Learning: In the Bun codebase, FFI (Foreign Function Interface) code is compiled separately using TinyCC (tcc), which barely supports C99. Headers like src/bun.js/api/FFI.h and src/bun.js/api/ffi-stdbool.h are only used for FFI compilation with tcc, not with the main compiler. Therefore, C standard changes to the main Bun target do not affect FFI code compilation.
Applied to files:
src/bun.js/bindings/ZigGlobalObject.cppsrc/bun.js/bindings/BunProcess.cpp
📚 Learning: 2025-09-20T05:35:57.318Z
Learnt from: pfgithub
Repo: oven-sh/bun PR: 22534
File: src/bun.js/bindings/headers.h:729-731
Timestamp: 2025-09-20T05:35:57.318Z
Learning: symbols.txt in the Bun codebase is specifically for V8 API mangled symbols (without leading underscore), not for general Bun host functions declared with BUN_DECLARE_HOST_FUNCTION. Host functions are handled through different build mechanisms.
Applied to files:
src/bun.js/bindings/BunProcess.cpp
🧬 Code graph analysis (5)
test/js/bun/runtime-inspector/runtime-inspector-windows.test.ts (2)
test/harness.ts (1)
tempDir(277-284)test/js/node/net/node-net-allowHalfOpen.test.js (1)
decoder(47-47)
test/js/bun/runtime-inspector/runtime-inspector.test.ts (1)
test/harness.ts (1)
tempDir(277-284)
test/js/bun/runtime-inspector/runtime-inspector-posix.test.ts (1)
test/harness.ts (1)
tempDir(277-284)
src/bun.js/bindings/webcore/HTTPHeaderMap.h (1)
packages/bun-uws/src/HttpContext.h (1)
nodiscard(45-89)
src/bun.js/bindings/BunGCOutputConstraint.h (2)
src/bun.js/bindings/JSPropertyIterator.cpp (1)
WTF_DEPRECATED_MAKE_FAST_ALLOCATED(36-36)src/bun.js/bindings/JSFFIFunction.cpp (1)
WTF_DEPRECATED_MAKE_FAST_ALLOCATED(42-42)
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| - Fix the merge conflicts | ||
| - cd ../../ (back to bun) | ||
| - make jsc-build (this will take about 7 minutes) | ||
| - bun build.ts debug |
Contributor
There was a problem hiding this comment.
Missing directory navigation before build command.
The workflow changes from vendor/WebKit directory (lines 5-8) to running bun build.ts debug, but the navigation step back to the bun root directory appears to have been removed. Unless build.ts exists in vendor/WebKit (unlikely), this command will fail. The next occurrence of "cd ../../ (back to bun)" at line 17 confirms we're expected to still be in the vendor/WebKit directory at this point.
📂 Suggested fix
- Fix the merge conflicts
+- cd ../../ (back to bun)
- bun build.ts debug🤖 Prompt for AI Agents
In @.claude/commands/upgrade-webkit.md at line 9, The build command is run from
the wrong directory: before running the existing "bun build.ts debug" command
you must change back to the bun root because the previous steps left you in the
vendor/WebKit directory; insert a directory navigation step (e.g. add "cd ../.."
or the appropriate "cd" to return to the bun root) immediately before the "bun
build.ts debug" command so the build script is executed from the correct project
root.
Comment on lines
+25
to
+26
| - Did JSType in vendor/WebKit/Source/JavaScriptCore have any recent changes? Does the enum values align with whats present in src/bun.js/bindings/JSType.zig? | ||
| - Were there any changes to the webcore code generator? If there are C++ compilation errors, check for differences in some of the generated code in like vendor/WebKit/source/WebCore/bindings/scripts/test/JS/ |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check actual directory structure in vendor/WebKit to verify casing
if [ -d "vendor/WebKit" ]; then
echo "=== Direct check for Source directories ==="
ls -d vendor/WebKit/Source* 2>/dev/null || echo "No Source* found"
ls -d vendor/WebKit/source* 2>/dev/null || echo "No source* found"
echo ""
echo "=== Checking for JavaScriptCore and WebCore subdirectories ==="
find vendor/WebKit -maxdepth 2 -type d -name "JavaScriptCore" 2>/dev/null
find vendor/WebKit -maxdepth 2 -type d -name "WebCore" 2>/dev/null
else
echo "vendor/WebKit directory not found"
fiRepository: oven-sh/bun
Length of output: 88
🌐 Web query:
WebKit repository directory structure Source JavaScriptCore WebCore
💡 Result:
Brief overview of the WebKit source-tree layout (paths are relative to the repository root, usually the Source/ directory):
-
Top-level important folders
- Source/JavaScriptCore — JavaScript engine (parser, JITs, runtime, API, heap, dfg/ftl, llint, etc.). [1]
- Source/WebCore — the browser engine (DOM, rendering, layout, CSS, networking, media, modules, bindings). [2][3]
- Source/WTF — utility library used by WebCore and JavaScriptCore (Vector, HashMap, RefPtr, etc.). [2]
- Source/WebKit — platform front-end shims (Cocoa/GTK/Qt/Windows ports) and embedding API. [2]
-
JavaScriptCore (Source/JavaScriptCore) — typical subfolders
- runtime/ — core interpreter/runtime types
- parser/ — lexer & parser
- llint/, dfg/, ftl/, jit/ — interpreter and JIT backends
- heap/ — garbage-collector implementation
- api/ — C API headers
- parser/, bytecode/, assembler/, debugger/, tests/ — other engine components and tests. [1]
-
WebCore (Source/WebCore) — typical subfolders
- dom/ — DOM implementation
- rendering/ — render tree and layout (renderers, style/)
- css/ and style/ — style parsing and computed style
- loader/ — resource loading (network, appcache, etc.)
- modules/ — optional modules (webaudio, mediastream, webgpu, etc.)
- platform/ and PAL/ — platform abstraction layers
- bindings/ — JS bindings and generated headers
- inspector/, plugins/, storage/, workers/, testing/ — features & tests. [2][3]
If you want the exact, current tree for a specific WebKit branch or tag I can fetch and list Source/JavaScriptCore and Source/WebCore directory contents for that commit or branch — tell me which branch/tag (or “main”) to inspect.
Sources: WebKit docs (JavaScriptCore) [1], WebKit Trac “SourceDirectory” overview [2], WebCore CMake (shows WebCore subdirs) [3].
Incorrect path casing on line 26.
Line 26 uses vendor/WebKit/source/WebCore (lowercase s) while the official WebKit repository structure uses Source/ (capital S) for all top-level directories. Line 25 correctly uses Source/JavaScriptCore, so line 26 should also use Source/WebCore for consistency.
🤖 Prompt for AI Agents
In @.claude/commands/upgrade-webkit.md around lines 25 - 26, The path casing is
inconsistent: change the lowercase "source/WebCore" occurrence to
"Source/WebCore" so it matches the other "Source/..." entries (e.g., the
"Source/JavaScriptCore" line); update the string "source/WebCore" to
"Source/WebCore" in the document to fix the incorrect path casing.
Comment on lines
+687
to
+696
| // Zig bindings for VMManager | ||
| extern "C" void VMManager__requestStopAll(uint32_t reason) | ||
| { | ||
| JSC::VMManager::requestStopAll(static_cast<JSC::VMManager::StopReason>(reason)); | ||
| } | ||
|
|
||
| extern "C" void VMManager__requestResumeAll(uint32_t reason) | ||
| { | ||
| JSC::VMManager::requestResumeAll(static_cast<JSC::VMManager::StopReason>(reason)); | ||
| } |
Contributor
There was a problem hiding this comment.
Potential type-safety improvement: avoid accepting arbitrary uint32_t as StopReason.
If feasible, consider validating/normalizing the bitmask (or exposing a typed wrapper on the C++ side) so invalid bits can’t silently propagate into JSC::VMManager.
🤖 Prompt for AI Agents
In `@src/bun.js/bindings/BunDebugger.cpp` around lines 687 - 696, The extern "C"
functions VMManager__requestStopAll and VMManager__requestResumeAll accept an
arbitrary uint32_t and cast it directly to JSC::VMManager::StopReason; add
validation or normalization before casting to prevent invalid bitmasks from
propagating into JSC::VMManager. Implement a small sanitizer that checks the
incoming uint32_t against the defined StopReason values (or a allowed bitmask)
and maps/limits unknown bits to a safe default (or returns early/logs) before
calling JSC::VMManager::requestStopAll/requestResumeAll, or replace the C
interface with a typed wrapper that accepts JSC::VMManager::StopReason and
performs the conversion/validation in one place so only validated enum values
are passed into the VMManager APIs.
Comment on lines
+1
to
+15
| /// Zig bindings for JSC::VMManager | ||
| /// | ||
| /// VMManager coordinates multiple VMs (workers) and provides the StopTheWorld | ||
| /// mechanism for safely interrupting JavaScript execution at safe points. | ||
| /// | ||
| /// Note: StopReason values are bitmasks (1 << bit_position), not sequential. | ||
| /// This matches the C++ enum in VMManager.h which uses: | ||
| /// enum class StopReason : StopRequestBits { None = 0, GC = 1, WasmDebugger = 2, MemoryDebugger = 4, JSDebugger = 8 } | ||
| pub const StopReason = enum(u32) { | ||
| None = 0, | ||
| GC = 1 << 0, // 1 | ||
| WasmDebugger = 1 << 1, // 2 | ||
| MemoryDebugger = 1 << 2, // 4 | ||
| JSDebugger = 1 << 3, // 8 | ||
| }; |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider a more ergonomic bitflag representation for combined reasons.
If callers ever need GC|JSDebugger, enum(u32) forces @intFromEnum / @enumFromInt plumbing. A u32 bitflags type (with named constants) or helper combinators would be easier to use.
🤖 Prompt for AI Agents
In `@src/bun.js/bindings/VMManager.zig` around lines 1 - 15, The StopReason enum
uses enum(u32) which makes bitwise combinations awkward; replace it with a plain
u32 bitflags pattern or add helpers to make combining/converting ergonomic:
change StopReason to a set of pub const bitflag values (e.g., pub const
StopReason_None: u32 = 0; pub const StopReason_GC: u32 = 1 << 0; ...) or provide
helper functions like stopReasonFromFlags(flags: u32) and
stopReasonToInt(reason: StopReason) and bitwise combinators (e.g.,
stopReasonHas(reason, flag)) so callers can use GC | JSDebugger directly without
`@intFromEnum/`@enumFromInt plumbing, and update usages of StopReason in functions
to accept/return u32 flags instead of enum.
Comment on lines
+2249
to
+2255
| // Use $promiseResolveWithThen to convert InternalPromise to regular Promise | ||
| // since $readStreamIntoSink is an async builtin function that returns InternalPromise | ||
| return $promiseResolveWithThen(Promise, Promise.$resolve(prom).$then(closer.promise).$then($withoutUTF8BOM)); | ||
| } | ||
|
|
||
| return closer.promise.$then($withoutUTF8BOM); | ||
| // Also wrap the non-promise path since closer.promise.$then() returns InternalPromise in builtin context | ||
| return $promiseResolveWithThen(Promise, closer.promise.$then($withoutUTF8BOM)); |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
head -2260 src/js/builtins/ReadableStreamInternals.ts | tail -50Repository: oven-sh/bun
Length of output: 1697
🏁 Script executed:
# Get more context around the function
sed -n '2200,2260p' src/js/builtins/ReadableStreamInternals.tsRepository: oven-sh/bun
Length of output: 1994
🏁 Script executed:
# Find the $createTextStream function to understand closer object
rg -n '\$createTextStream' src/js/builtins/ReadableStreamInternals.ts -A 10 -B 5Repository: oven-sh/bun
Length of output: 1469
🏁 Script executed:
# Search for how $then is used throughout the file
rg -n '\.\$then\(' src/js/builtins/ReadableStreamInternals.ts | head -30Repository: oven-sh/bun
Length of output: 2321
🏁 Script executed:
# Find $createTextStream definition
rg -n 'export function \$createTextStream|^function \$createTextStream' src/js/builtins/ReadableStreamInternals.ts -A 20Repository: oven-sh/bun
Length of output: 37
🏁 Script executed:
# Find $readStreamIntoSink definition
rg -n 'export function \$readStreamIntoSink|^function \$readStreamIntoSink' src/js/builtins/ReadableStreamInternals.ts -A 10Repository: oven-sh/bun
Length of output: 37
🏁 Script executed:
# Look for test files related to readableStreamIntoText
find . -type f -name '*.test.*' -o -name '*.spec.*' | xargs grep -l 'readableStreamIntoText' 2>/dev/null | head -5Repository: oven-sh/bun
Length of output: 37
🏁 Script executed:
# Search in the C++ side for how these functions work
find . -path ./node_modules -prune -o -type f \( -name '*.cpp' -o -name '*.h' \) -print | xargs grep -l 'createTextStream\|readStreamIntoSink' 2>/dev/null | head -5Repository: oven-sh/bun
Length of output: 37
🏁 Script executed:
# Search for createTextStream definition in all files
rg 'createTextStream' --type-list | head -20Repository: oven-sh/bun
Length of output: 444
🏁 Script executed:
# Search more broadly in TypeScript/JavaScript files
rg 'createTextStream' -t ts -t js | head -30Repository: oven-sh/bun
Length of output: 372
🏁 Script executed:
# Try to find C++ implementation
find . -path ./node_modules -prune -o -type f \( -name '*.cpp' -o -name '*.h' \) -print 2>/dev/null | xargs grep -l 'createTextStream\|createTextSink' 2>/dev/null | head -5Repository: oven-sh/bun
Length of output: 37
🏁 Script executed:
# Look for builtin definitions or stubs in the JS files
rg 'TextStream|textStream' src/js/builtins/ReadableStreamInternals.ts -iRepository: oven-sh/bun
Length of output: 473
🏁 Script executed:
# Find and examine the exported createTextStream function
rg -n 'export function createTextStream' src/js/builtins/ReadableStreamInternals.ts -A 30Repository: oven-sh/bun
Length of output: 1080
🏁 Script executed:
# Also check if there are comments or patterns explaining how promises work
rg -n 'closer\.' src/js/builtins/ReadableStreamInternals.ts -B 3 -A 3Repository: oven-sh/bun
Length of output: 678
🏁 Script executed:
# Look for test cases that might show usage
find . -type f \( -name '*.test.ts' -o -name '*.test.js' -o -name '*test*.ts' \) -path '*/stream*' 2>/dev/null | head -10Repository: oven-sh/bun
Length of output: 317
🏁 Script executed:
# Get the complete createTextStream function
sed -n '1248,1350p' src/js/builtins/ReadableStreamInternals.tsRepository: oven-sh/bun
Length of output: 2537
🏁 Script executed:
# Search for how capability is resolved
rg -n 'capability\.(resolve|promise)' src/js/builtins/ReadableStreamInternals.ts -B 2 -A 2Repository: oven-sh/bun
Length of output: 1080
🏁 Script executed:
# Look for $newPromiseCapability to understand its structure
rg -n '\$newPromiseCapability' src/js/builtins/ReadableStreamInternals.ts -B 1 -A 1Repository: oven-sh/bun
Length of output: 1313
🏁 Script executed:
# Check the full return value of createTextStream
sed -n '1248,1363p' src/js/builtins/ReadableStreamInternals.ts | tail -30Repository: oven-sh/bun
Length of output: 662
🏁 Script executed:
# Look for usage in tests to verify the behavior
rg 'readableStreamIntoText' ./test/js/ -A 5 -B 5 | head -50Repository: oven-sh/bun
Length of output: 37
🏁 Script executed:
# Verify what $fulfillPromise does to understand promise resolution
rg -n '\$fulfillPromise' src/js/builtins/ReadableStreamInternals.ts | head -10Repository: oven-sh/bun
Length of output: 590
🏁 Script executed:
# Check how $then is used with function arguments elsewhere
rg '\.\$then\([a-zA-Z_$]' src/js/builtins/ReadableStreamInternals.ts | grep -v '=> ' | head -15Repository: oven-sh/bun
Length of output: 987
Fix incorrect promise chaining on line 2251: closer.promise must be wrapped in a function.
On line 2251, .$then(closer.promise) passes a Promise object as the fulfillment handler. According to Promise/A+ spec, non-function arguments to .then() are treated as identity functions. This means the resolved value of prom (which is undefined) passes through, and closer.promise's value is never awaited or used. The subsequent .$then($withoutUTF8BOM) would receive undefined and fail.
Line 2255 demonstrates the correct pattern with closer.promise.$then($withoutUTF8BOM). The fix wraps closer.promise in a function to properly chain the promises:
Suggested fix
- return $promiseResolveWithThen(Promise, Promise.$resolve(prom).$then(closer.promise).$then($withoutUTF8BOM));
+ return $promiseResolveWithThen(Promise, Promise.$resolve(prom).$then(() => closer.promise).$then($withoutUTF8BOM));📝 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
| // Use $promiseResolveWithThen to convert InternalPromise to regular Promise | |
| // since $readStreamIntoSink is an async builtin function that returns InternalPromise | |
| return $promiseResolveWithThen(Promise, Promise.$resolve(prom).$then(closer.promise).$then($withoutUTF8BOM)); | |
| } | |
| return closer.promise.$then($withoutUTF8BOM); | |
| // Also wrap the non-promise path since closer.promise.$then() returns InternalPromise in builtin context | |
| return $promiseResolveWithThen(Promise, closer.promise.$then($withoutUTF8BOM)); | |
| // Use $promiseResolveWithThen to convert InternalPromise to regular Promise | |
| // since $readStreamIntoSink is an async builtin function that returns InternalPromise | |
| return $promiseResolveWithThen(Promise, Promise.$resolve(prom).$then(() => closer.promise).$then($withoutUTF8BOM)); | |
| } | |
| // Also wrap the non-promise path since closer.promise.$then() returns InternalPromise in builtin context | |
| return $promiseResolveWithThen(Promise, closer.promise.$then($withoutUTF8BOM)); |
🤖 Prompt for AI Agents
In `@src/js/builtins/ReadableStreamInternals.ts` around lines 2249 - 2255, The
chain is incorrectly passing a Promise object as a then-handler: in the
expression using
Promise.$resolve(prom).$then(closer.promise).$then($withoutUTF8BOM) you must
pass a function that returns closer.promise so it's awaited; change the then
handler to a function wrapper (e.g. () => closer.promise) instead of
closer.promise. Keep the other branch (closer.promise.$then($withoutUTF8BOM))
as-is.
Comment on lines
+123
to
+197
| test("multiple SIGUSR1s work after user installs handler", async () => { | ||
| // After user installs their own SIGUSR1 handler, multiple signals should all | ||
| // be delivered to the user handler correctly. | ||
| using dir = tempDir("sigusr1-uninstall-test", { | ||
| "test.js": ` | ||
| const fs = require("fs"); | ||
| const path = require("path"); | ||
|
|
||
| let count = 0; | ||
| process.on("SIGUSR1", () => { | ||
| count++; | ||
| console.log("SIGNAL_" + count); | ||
| if (count >= 3) { | ||
| setTimeout(() => process.exit(0), 100); | ||
| } | ||
| }); | ||
|
|
||
| fs.writeFileSync(path.join(process.cwd(), "pid"), String(process.pid)); | ||
| console.log("READY"); | ||
|
|
||
| setInterval(() => {}, 1000); | ||
| `, | ||
| }); | ||
|
|
||
| await using proc = spawn({ | ||
| cmd: [bunExe(), "test.js"], | ||
| cwd: String(dir), | ||
| env: bunEnv, | ||
| stdout: "pipe", | ||
| stderr: "pipe", | ||
| }); | ||
|
|
||
| const reader = proc.stdout.getReader(); | ||
| const decoder = new TextDecoder(); | ||
|
|
||
| let output = ""; | ||
| while (!output.includes("READY")) { | ||
| const { value, done } = await reader.read(); | ||
| if (done) break; | ||
| output += decoder.decode(value, { stream: true }); | ||
| } | ||
|
|
||
| const pid = parseInt(await Bun.file(join(String(dir), "pid")).text(), 10); | ||
|
|
||
| // Send SIGUSR1s and wait for each handler to respond before sending the next | ||
| for (let i = 1; i <= 3; i++) { | ||
| process.kill(pid, "SIGUSR1"); | ||
| // Wait for handler output before sending next signal | ||
| while (!output.includes(`SIGNAL_${i}`)) { | ||
| const { value, done } = await reader.read(); | ||
| if (done) break; | ||
| output += decoder.decode(value, { stream: true }); | ||
| } | ||
| } | ||
|
|
||
| // Read remaining output until process exits | ||
| while (true) { | ||
| const { value, done } = await reader.read(); | ||
| if (done) break; | ||
| output += decoder.decode(value, { stream: true }); | ||
| } | ||
| output += decoder.decode(); | ||
|
|
||
| const [stderr, exitCode] = await Promise.all([proc.stderr.text(), proc.exited]); | ||
|
|
||
| expect(output).toMatchInlineSnapshot(` | ||
| "READY | ||
| SIGNAL_1 | ||
| SIGNAL_2 | ||
| SIGNAL_3 | ||
| " | ||
| `); | ||
| expect(stderr).not.toContain("Bun Inspector"); | ||
| expect(exitCode).toBe(0); | ||
| }); |
Contributor
There was a problem hiding this comment.
Same reader-lock cleanup issue here; otherwise test intent is clear.
Proposed diff
// Read remaining output until process exits
while (true) {
const { value, done } = await reader.read();
if (done) break;
output += decoder.decode(value, { stream: true });
}
output += decoder.decode();
+ reader.releaseLock();🤖 Prompt for AI Agents
In `@test/js/bun/runtime-inspector/runtime-inspector-posix.test.ts` around lines
123 - 197, The test leaves the stdout reader locked, causing cleanup issues;
after finishing all reads (the loops that await reader.read() and before
awaiting proc.exited / proc.stderr.text()), explicitly cancel or release the
stream reader (call reader.cancel() or reader.releaseLock()) — ideally in a
finally block — so the reader is freed and the process can be awaited and
cleaned up correctly; update the test "multiple SIGUSR1s work after user
installs handler" to call reader.cancel()/reader.releaseLock() on the `reader`
created from `proc.stdout.getReader()`.
Comment on lines
+199
to
+261
| test("inspector does not activate twice via SIGUSR1", async () => { | ||
| using dir = tempDir("sigusr1-twice-test", { | ||
| "test.js": ` | ||
| const fs = require("fs"); | ||
| const path = require("path"); | ||
|
|
||
| fs.writeFileSync(path.join(process.cwd(), "pid"), String(process.pid)); | ||
| console.log("READY"); | ||
|
|
||
| // Keep process alive, exit after a bit | ||
| setTimeout(() => process.exit(0), 500); | ||
| setInterval(() => {}, 1000); | ||
| `, | ||
| }); | ||
|
|
||
| await using proc = spawn({ | ||
| cmd: [bunExe(), "test.js"], | ||
| cwd: String(dir), | ||
| env: bunEnv, | ||
| stdout: "pipe", | ||
| stderr: "pipe", | ||
| }); | ||
|
|
||
| const reader = proc.stdout.getReader(); | ||
| const decoder = new TextDecoder(); | ||
|
|
||
| let output = ""; | ||
| while (!output.includes("READY")) { | ||
| const { value, done } = await reader.read(); | ||
| if (done) break; | ||
| output += decoder.decode(value, { stream: true }); | ||
| } | ||
| reader.releaseLock(); | ||
|
|
||
| const pid = parseInt(await Bun.file(join(String(dir), "pid")).text(), 10); | ||
|
|
||
| // Send first SIGUSR1 and wait for inspector to activate | ||
| process.kill(pid, "SIGUSR1"); | ||
|
|
||
| const stderrReader = proc.stderr.getReader(); | ||
| const stderrDecoder = new TextDecoder(); | ||
| let stderr = ""; | ||
|
|
||
| // Wait for the full banner (header + content + footer) before sending second signal | ||
| while ((stderr.match(/Bun Inspector/g) || []).length < 2) { | ||
| const { value, done } = await stderrReader.read(); | ||
| if (done) break; | ||
| stderr += stderrDecoder.decode(value, { stream: true }); | ||
| } | ||
|
|
||
| // Send second SIGUSR1 - inspector should not activate again | ||
| process.kill(pid, "SIGUSR1"); | ||
|
|
||
| // Read any remaining stderr until process exits | ||
| stderrReader.releaseLock(); | ||
| const [remainingStderr, exitCode] = await Promise.all([proc.stderr.text(), proc.exited]); | ||
| stderr += remainingStderr; | ||
|
|
||
| // Should only see one "Bun Inspector" banner (two occurrences of the text, for header and footer) | ||
| const matches = stderr.match(/Bun Inspector/g); | ||
| expect(matches?.length ?? 0).toBe(2); | ||
| expect(exitCode).toBe(0); | ||
| }); |
Contributor
There was a problem hiding this comment.
High flake risk: child exits after 500ms regardless of inspector activation speed.
On slower/ASAN builds, the process can exit before the banner is printed, turning this into a race. Prefer keeping the child alive and letting the parent proc.kill() once assertions are satisfied.
Comment on lines
+263
to
+287
| test("SIGUSR1 to self activates inspector", async () => { | ||
| await using proc = spawn({ | ||
| cmd: [ | ||
| bunExe(), | ||
| "-e", | ||
| ` | ||
| setTimeout(() => process.exit(0), 300); | ||
| // Small delay to ensure handler is installed | ||
| setTimeout(() => { | ||
| process.kill(process.pid, "SIGUSR1"); | ||
| }, 50); | ||
| setInterval(() => {}, 1000); | ||
| `, | ||
| ], | ||
| env: bunEnv, | ||
| stdout: "pipe", | ||
| stderr: "pipe", | ||
| }); | ||
|
|
||
| const [stderr, exitCode] = await Promise.all([proc.stderr.text(), proc.exited]); | ||
|
|
||
| expect(stderr).toContain("Bun Inspector"); | ||
| expect(exitCode).toBe(0); | ||
| }); | ||
|
|
Contributor
There was a problem hiding this comment.
High flake risk: self-SIGUSR1 test exits after 300ms.
Same race as above; consider waiting for the activation banner (or a deterministic signal) before exiting the child.
🤖 Prompt for AI Agents
In `@test/js/bun/runtime-inspector/runtime-inspector-posix.test.ts` around lines
263 - 287, The child currently self-exits after 300ms which races with inspector
activation; remove the hardcoded setTimeout(() => process.exit(0), 300) from the
spawned script so the child stays alive (keep the setInterval), then in the test
"SIGUSR1 to self activates inspector" wait for the inspector banner by reading
proc.stderr (the existing proc.stderr.text()/expect(stderr).toContain("Bun
Inspector")), and only then terminate the child from the test (e.g., call
proc.kill() and await proc.exited) so the test deterministically waits for
activation before exit.
Comment on lines
+288
to
+336
| test("SIGUSR1 is ignored when started with --inspect", async () => { | ||
| // When the process is started with --inspect, the debugger is already active. | ||
| // The RuntimeInspector signal handler should NOT be installed, so SIGUSR1 | ||
| // should have no effect (default action is terminate, but signal may be ignored). | ||
| using dir = tempDir("sigusr1-inspect-test", { | ||
| "test.js": ` | ||
| const fs = require("fs"); | ||
| const path = require("path"); | ||
|
|
||
| fs.writeFileSync(path.join(process.cwd(), "pid"), String(process.pid)); | ||
| console.log("READY"); | ||
|
|
||
| setTimeout(() => process.exit(0), 500); | ||
| setInterval(() => {}, 1000); | ||
| `, | ||
| }); | ||
|
|
||
| await using proc = spawn({ | ||
| cmd: [bunExe(), "--inspect", "test.js"], | ||
| cwd: String(dir), | ||
| env: bunEnv, | ||
| stdout: "pipe", | ||
| stderr: "pipe", | ||
| }); | ||
|
|
||
| const reader = proc.stdout.getReader(); | ||
| const decoder = new TextDecoder(); | ||
|
|
||
| let output = ""; | ||
| while (!output.includes("READY")) { | ||
| const { value, done } = await reader.read(); | ||
| if (done) break; | ||
| output += decoder.decode(value, { stream: true }); | ||
| } | ||
| reader.releaseLock(); | ||
|
|
||
| const pid = parseInt(await Bun.file(join(String(dir), "pid")).text(), 10); | ||
|
|
||
| // Send SIGUSR1 - should be ignored since RuntimeInspector is not installed | ||
| process.kill(pid, "SIGUSR1"); | ||
|
|
||
| const [stderr, exitCode] = await Promise.all([proc.stderr.text(), proc.exited]); | ||
|
|
||
| // Should only see one "Bun Inspector" banner (from --inspect flag, not from SIGUSR1) | ||
| // The banner has two occurrences of "Bun Inspector" (header and footer) | ||
| const matches = stderr.match(/Bun Inspector/g); | ||
| expect(matches?.length ?? 0).toBe(2); | ||
| expect(exitCode).toBe(0); | ||
| }); |
Contributor
There was a problem hiding this comment.
Comment mismatch: SIGUSR1 is being ignored, not “not installed”.
Per the VM logic, when the debugger is active you’re explicitly setting SIGUSR1 to ignore; updating the comment would make the expectation clearer.
🤖 Prompt for AI Agents
In `@test/js/bun/runtime-inspector/runtime-inspector-posix.test.ts` around lines
288 - 336, The test comment incorrectly says the RuntimeInspector signal handler
"should NOT be installed" when started with --inspect; update the comments in
the test "SIGUSR1 is ignored when started with --inspect" and the inline comment
before process.kill to state that the VM explicitly sets SIGUSR1 to be ignored
(so the signal is ignored), rather than claiming the handler is not installed,
so the expectation and wording match the VM behavior.
Comment on lines
+338
to
+380
| test("SIGUSR1 is ignored when started with --inspect-wait", async () => { | ||
| // When the process is started with --inspect-wait, the debugger is already active. | ||
| // Sending SIGUSR1 should NOT activate the inspector again. | ||
| await using proc = spawn({ | ||
| cmd: [bunExe(), "--inspect-wait", "-e", "setTimeout(() => process.exit(0), 500)"], | ||
| env: bunEnv, | ||
| stdout: "pipe", | ||
| stderr: "pipe", | ||
| }); | ||
|
|
||
| const reader = proc.stderr.getReader(); | ||
| const decoder = new TextDecoder(); | ||
| let stderr = ""; | ||
|
|
||
| while ((stderr.match(/Bun Inspector/g) || []).length < 2) { | ||
| const { value, done } = await reader.read(); | ||
| if (done) break; | ||
| stderr += decoder.decode(value, { stream: true }); | ||
| } | ||
|
|
||
| // Send SIGUSR1 - should be ignored since debugger is already active | ||
| process.kill(proc.pid, "SIGUSR1"); | ||
|
|
||
| // Kill process since --inspect-wait would wait for connection | ||
| // Signal processing is synchronous, so no sleep needed | ||
| proc.kill(); | ||
|
|
||
| // Read any remaining stderr | ||
| while (true) { | ||
| const { value, done } = await reader.read(); | ||
| if (done) break; | ||
| stderr += decoder.decode(value, { stream: true }); | ||
| } | ||
| stderr += decoder.decode(); | ||
| reader.releaseLock(); | ||
|
|
||
| await proc.exited; | ||
|
|
||
| // Should only see one "Bun Inspector" banner (from --inspect-wait flag, not from SIGUSR1) | ||
| // The banner has two occurrences of "Bun Inspector" (header and footer) | ||
| const matches = stderr.match(/Bun Inspector/g); | ||
| expect(matches?.length ?? 0).toBe(2); | ||
| }); |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
These tests terminate the process with proc.kill() but don’t assert exit semantics.
That’s fine if intentional, but if you want extra signal, consider asserting you still only got the original inspector banner and that no second activation occurred (independent of how the process was terminated).
Also applies to: 382-424
🤖 Prompt for AI Agents
In `@test/js/bun/runtime-inspector/runtime-inspector-posix.test.ts` around lines
338 - 380, The tests call proc.kill() (and send SIGUSR1) but never assert how
the child actually exited; after awaiting proc.exited add an explicit assertion
on the exit semantics (e.g. check proc.exitSignal === "SIGTERM" or proc.exitCode
as appropriate) in addition to the existing inspector-banner assertion so the
test verifies both that the inspector was not reactivated (the
expect(matches?.length ?? 0).toBe(2) check) and that the process terminated due
to the intended kill; make the same change in the sibling test that also uses
proc.kill() and process.kill(proc.pid, "SIGUSR1").
Contributor
|
Closing this PR because it has been inactive for more than 90 days. |
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
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.
Summary
{ timeout: 30_000 }option to slow testsTest plan
bun bd test test/js/bun/runtime-inspector/runtime-inspector.test.ts- all 7 tests pass🤖 Generated with Claude Code