Skip to content

Conversation

@WA9ACE
Copy link

@WA9ACE WA9ACE commented Dec 3, 2025

What does this PR do?

The default web inspector is self-hostable however the inspection output is permanently pointed to https://debug.bun.sh. Development in closed environments and systems can neither access nor trust the publicly hosted web inspector. This introduces a new environment variable named BUN_INSPECTOR_HOST that when defined will replace the default inspection output domain. It also fails gracefully when encountering trailing slashes, and warns of invalid protocols.

How did you verify your code works?

  • Build Bun from this branch
  • touch test.js
  • Run BUN_INSPECTOR_HOST='https://debug.example.com' build/debug/bun-debug --inspect-wait test.js
  • Ensure that the inspector domain output instead references the BUN_INSPECTOR_HOST variable replacing the default of https://debug.bun.sh such as below.
Inspect in browser:
  https://debug.example.com/#localhost:6499/dqy5c9ihdl

…tems

can neither access or trust the publicly hosted web inspector. This
introduces a new environment variable named BUN_DEBUG_HOST that when
defined will replace the default inspection output domain.
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 3, 2025

Walkthrough

Added an internal getInspectorHost() helper to derive the inspector host from the BUN_INSPECTOR_HOST environment variable (defaulting to https://debug.bun.sh), validate and normalize it, and use it when building browser inspect links instead of a hardcoded URL.

Changes

Cohort / File(s) Summary
Inspector host helper & link generation
src/js/internal/debugger.ts
Added internal getInspectorHost() to read and normalize BUN_INSPECTOR_HOST (default https://debug.bun.sh, strips trailing slash, validates protocol); updated browser inspect URL construction to use getInspectorHost() and form ${getInspectorHost()}/#${host}${pathname} for websocket inspection.

Pre-merge checks

✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: allowing a custom web inspector domain via the BUN_INSPECTOR_HOST environment variable.
Description check ✅ Passed The pull request description includes both required sections with clear explanations and concrete verification steps demonstrating the feature works as intended.

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Disabled knowledge base sources:

  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 01762cb and 224fca3.

📒 Files selected for processing (1)
  • src/js/internal/debugger.ts (2 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
src/js/{builtins,node,bun,thirdparty,internal}/**/*.{ts,js}

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

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

Files:

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

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

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

Files:

  • src/js/internal/debugger.ts
🧠 Learnings (4)
📚 Learning: 2025-11-24T18:37:11.466Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/js/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:11.466Z
Learning: Applies to src/js/{builtins,node,bun,thirdparty,internal}/**/*.{ts,js} : Use `$debug()` for debug logging and `$assert()` for assertions; both are stripped in release builds

Applied to files:

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

Applied to files:

  • src/js/internal/debugger.ts
📚 Learning: 2025-09-12T18:16:50.754Z
Learnt from: RiskyMH
Repo: oven-sh/bun PR: 22606
File: src/glob/GlobWalker.zig:449-452
Timestamp: 2025-09-12T18:16:50.754Z
Learning: For Bun codebase: prefer using `std.fs.path.sep` over manual platform separator detection, and use `bun.strings.lastIndexOfChar` instead of `std.mem.lastIndexOfScalar` for string operations.

Applied to files:

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

Applied to files:

  • src/js/internal/debugger.ts
🔇 Additional comments (2)
src/js/internal/debugger.ts (2)

98-119: Inspector host helper correctly normalizes and validates BUN_INSPECTOR_HOST.

The helper cleanly implements the desired behavior: defaulting to https://debug.bun.sh, stripping a trailing slash, validating an http/https scheme case‑insensitively, warning on invalid values, and falling back safely. This matches the PR’s objectives without introducing new edge‑case risks.


151-151: Browser inspect URL now respects BUN_INSPECTOR_HOST as intended.

Switching the link construction to ${getInspectorHost()}/#${host}${pathname} preserves the previous URL shape while allowing the host to be overridden via the new environment variable, with normalization handled centrally in getInspectorHost().


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

❤️ Share

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

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Disabled knowledge base sources:

  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between e9e9324 and 2c5ffb7.

📒 Files selected for processing (1)
  • src/js/internal/debugger.ts (1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
src/js/{builtins,node,bun,thirdparty,internal}/**/*.{ts,js}

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

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

Files:

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

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

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

Files:

  • src/js/internal/debugger.ts
🧠 Learnings (4)
📚 Learning: 2025-11-24T18:37:11.466Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/js/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:11.466Z
Learning: Applies to src/js/{builtins,node,bun,thirdparty,internal}/**/*.{ts,js} : Use `$debug()` for debug logging and `$assert()` for assertions; both are stripped in release builds

Applied to files:

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

Applied to files:

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

Applied to files:

  • src/js/internal/debugger.ts
📚 Learning: 2025-11-24T18:34:55.173Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/building-bun.mdc:0-0
Timestamp: 2025-11-24T18:34:55.173Z
Learning: Applies to src/**/*.{cpp,zig} : Use `bun bd` or `bun run build:debug` to build debug versions for C++ and Zig source files; creates debug build at `./build/debug/bun-debug`

Applied to files:

  • src/js/internal/debugger.ts
🔇 Additional comments (1)
src/js/internal/debugger.ts (1)

125-129: Add tests and documentation for the BUN_DEBUG_HOST environment variable.

Ensure that:

  1. Tests are added or updated to cover the new environment variable behavior
  2. The environment variable is documented (e.g., in README, docs, or help text)
  3. Edge cases are tested (missing protocol, invalid URLs, malformed hosts)

@WA9ACE
Copy link
Author

WA9ACE commented Dec 3, 2025

I've noticed the trend for BUN_DEBUG prefixed environment variables seems to be for debug builds of bun itself. Would it better to define this as BUN_INSPECTOR_HOST?

Went ahead and changed BUN_DEBUG_HOST to BUN_INSPECTOR_HOST.

inspector host variable to a function that gracefully handles the
suggested trailing slash and protocol states.
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Disabled knowledge base sources:

  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 2c5ffb7 and 01762cb.

📒 Files selected for processing (1)
  • src/js/internal/debugger.ts (2 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
src/js/{builtins,node,bun,thirdparty,internal}/**/*.{ts,js}

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

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

Files:

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

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

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

Files:

  • src/js/internal/debugger.ts
🧠 Learnings (4)
📚 Learning: 2025-11-24T18:37:11.466Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/js/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:11.466Z
Learning: Applies to src/js/{builtins,node,bun,thirdparty,internal}/**/*.{ts,js} : Use `$debug()` for debug logging and `$assert()` for assertions; both are stripped in release builds

Applied to files:

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

Applied to files:

  • src/js/internal/debugger.ts
📚 Learning: 2025-09-12T18:16:50.754Z
Learnt from: RiskyMH
Repo: oven-sh/bun PR: 22606
File: src/glob/GlobWalker.zig:449-452
Timestamp: 2025-09-12T18:16:50.754Z
Learning: For Bun codebase: prefer using `std.fs.path.sep` over manual platform separator detection, and use `bun.strings.lastIndexOfChar` instead of `std.mem.lastIndexOfScalar` for string operations.

Applied to files:

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

Applied to files:

  • src/js/internal/debugger.ts
🔇 Additional comments (1)
src/js/internal/debugger.ts (1)

151-151: LGTM! Clean integration of the inspector host resolver.

The call site correctly uses getInspectorHost() to construct the browser inspection URL, replacing the hardcoded domain. The integration is clean and maintains the existing URL structure.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant