inspector: handle BigInt/Symbol in returnByValue instead of asserting

[robobun]

What does this PR do?

Runtime.evaluate / Runtime.callFunctionOn / Debugger.evaluateOnCallFrame with returnByValue: true on a result that holds a BigInt or Symbol reached ASSERT_NOT_REACHED in Inspector::jsToInspectorValue (InjectedScriptBase.cpp:96), aborting an assertions build of the debuggee. Release builds fell through to nullptr and the caller reported the unrelated "Object has too long reference chain (must not be longer than 1000)" error.

Repro

// bun --inspect=127.0.0.1:<port>/tok -e "setInterval(()=>{},1000)" &
// connect WebSocket to ws://127.0.0.1:<port>/tok, send:
{"id":1,"method":"Runtime.evaluate","params":{"expression":"[1n]","returnByValue":true}}

Before (assertions build):

SHOULD NEVER BE REACHED
vendor/WebKit/Source/JavaScriptCore/inspector/InjectedScriptBase.cpp(96) : RefPtr<JSON::Value> Inspector::jsToInspectorValue(JSC::JSGlobalObject *, JSC::JSValue, int)

debuggee exits SIGABRT.

Before (release): {"error":{"code":-32000,"message":"Object has too long reference chain (must not be longer than 1000)"}}, debuggee alive.

Also reproduces with ({a:1n}), Symbol("s"), ({b:Symbol()}), and over Debugger.evaluateOnCallFrame while paused. A bare 1n is fine because InjectedScriptSource.js never stores it on RemoteObject.value.

Cause

jsToInspectorValue has arms for null/undefined, boolean, number, string, and object, then ASSERT_NOT_REACHED(). BigInt and Symbol (introduced after this code was written) fall through. When returnByValue is set, InjectedScriptSource.js stores the raw value on RemoteObject.value and the C++ side recurses into it.

Fix

The fix is in oven-sh/WebKit#384:

• Add an explicit isBigInt() || isSymbol() arm returning nullptr (unserializable), avoiding the assert.
• Replace the "reference chain" error string with "Object couldn't be returned by value", matching V8's inspector wording (node -e '...' returns the same for [1n]) and accurate for both the depth-limit and unserializable-primitive cases.

This PR bumps WEBKIT_VERSION to the preview build of that change and adds a --inspect regression test covering [1n], {a:1n}, Symbol(), {b:Symbol()}, plus the already-working bare 1n.

Verification

$ USE_SYSTEM_BUN=1 bun test test/cli/inspect/inspect.test.ts -t "Runtime.evaluate returnByValue"
 4 fail ("Object has too long reference chain" instead of "Object couldn't be returned by value")
 1 pass (bare 1n)

After: all 5 pass; the debuggee stays alive on an assertions build.

Note: WEBKIT_VERSION currently points at the autobuild-preview-pr-384-* tag and will be switched to the merged oven-sh/WebKit main sha before this PR merges.

[robobun]

Status: oven-sh/WebKit#384 CI is green across all 38 lanes and the autobuild-preview-pr-384-0cd81acb preview release is published. Built and verified locally:

$ bun bd test test/cli/inspect/inspect.test.ts -t "Runtime.evaluate returnByValue"
(pass) Runtime.evaluate returnByValue with BigInt/Symbol > [1n]
(pass) Runtime.evaluate returnByValue with BigInt/Symbol > ({a: 1n})
(pass) Runtime.evaluate returnByValue with BigInt/Symbol > Symbol("s")
(pass) Runtime.evaluate returnByValue with BigInt/Symbol > ({b: Symbol()})
(pass) Runtime.evaluate returnByValue with BigInt/Symbol > bare 1n
 5 pass, 0 fail

Against the original repro on the assertions-enabled bun bd binary:

reply: {"error":{"code":-32000,"message":"Object couldn't be returned by value",...},"id":1}
debuggee: alive

(was SHOULD NEVER BE REACHED + SIGABRT on the debuggee before).

Fail-before is intact (USE_SYSTEM_BUN=1 still fails 4/5 with "Object has too long reference chain"). Bun CI should now build; WEBKIT_VERSION will be switched to the merged oven-sh/WebKit main sha before this PR merges.

[robobun]

^{Updated 2:06 PM PT - Aug 3rd, 2026}

✅ @robobun, your commit 34087f992d72dfeedc646b9955a015c1fcfc0174 passed in Build #88218! 🎉

---

🧪   To try this PR locally:

bunx bun-pr 36841

That installs a local version of the PR into your bun-36841 executable, so you can run:

bun-36841 --bun

[coderabbitai]

Warning

Review limit reached

@robobun, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 5 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: d5a1ce80-cf14-42d8-841c-65313624ea8c

📥 Commits

Reviewing files that changed from the base of the PR and between bfe8527 and 34087f9.

📒 Files selected for processing (1)

• test/cli/inspect/inspect.test.ts

Walkthrough

The PR updates the default WebKit preview release tag and adds inspector regression tests for Runtime.evaluate with BigInt- and Symbol-containing values.

Changes

WebKit preview update

Layer / File(s)	Summary
Update WebKit preview identifier scripts/build/deps/webkit.ts	The default WEBKIT_VERSION export now uses autobuild-preview-pr-384-0cd81acb.

Inspector evaluation regressions

Layer / File(s)	Summary
Add Runtime.evaluate regression coverage test/cli/inspect/inspect.test.ts	WebSocket inspector tests validate "Object couldn't be returned by value" responses, process survival, and successful bare 1n evaluation.

Possibly related PRs

• oven-sh/bun#35754: Updates the same WEBKIT_VERSION dependency.
• oven-sh/bun#35910: Updates WEBKIT_VERSION to an autobuild preview tag.
• oven-sh/bun#36794: Updates the exported WEBKIT_VERSION to another WebKit preview build.

Suggested reviewers: jarred-sumner

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly identifies the main inspector change for BigInt and Symbol values with returnByValue.
Description check	✅ Passed	The description includes the required change summary and verification sections with detailed reproduction, cause, fix, and test results.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

---

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@scripts/build/deps/webkit.ts`:
- Line 6: Update WEBKIT_VERSION to a published immutable WebKit revision that
exists on the remote, replacing the unavailable
autobuild-preview-pr-384-0cd81acb preview tag. Use the WebKit webkitVersion or
prebuilt.identity value as the revision source, not a separate dependency commit
field.

In `@test/cli/inspect/inspect.test.ts`:
- Around line 423-430: Update the WebSocket waits around the connection and
reply promises to settle on every terminal event: reject both phases on close
and error, catch JSON.parse failures and reject the reply promise, and remove
all attached listeners after resolve or reject. Preserve successful open and
message resolution while ensuring no promise remains pending.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 9b9dc113-4169-4058-9dc0-0d5972574e52

📥 Commits

Reviewing files that changed from the base of the PR and between 074656d and bfe8527.

📒 Files selected for processing (2)

• scripts/build/deps/webkit.ts
• test/cli/inspect/inspect.test.ts

[claude]

Test changes look good after the follow-ups — the hoisted spawnInspectee() now kills the child on the discovery-failure path and both WebSocket waits reject on close/error, so my earlier inline notes are resolved. Not approving because the substantive fix is a WebKit dependency bump (oven-sh/WebKit#384) that a human should sign off on, and WEBKIT_VERSION is still on the autobuild-preview-pr-384-* tag pending that PR's merge.

What was reviewed:

• spawnInspectee() hoist: verified the two existing http-metadata callers still assign metadataInspectee before any assertion so afterEach cleanup is intact.
• New returnByValue tests: exact-message assertion on the V8 wording, debuggee-liveness check via exitCode/signalCode, and the bare-1n control case guarding against over-broad rejection.
• Confirmed {once: true} on the open-phase close listener means a later post-reply close cannot spuriously reject an already-settled promise.

Extended reasoning...

Overview

This PR carries a fix that lives in oven-sh/WebKit#384 (adding a BigInt/Symbol arm to Inspector::jsToInspectorValue and rewording the fallthrough error). On the Bun side it does two things: bumps WEBKIT_VERSION in scripts/build/deps/webkit.ts to a preview build of that WebKit change, and adds a regression describe in test/cli/inspect/inspect.test.ts covering [1n], {a:1n}, Symbol(), {b:Symbol()}, plus a bare-1n control. Follow-up commits hoisted the pre-existing spawnInspectee() helper to file scope (reused by the http-metadata describe and the new describe), added child.kill() on the URL-discovery failure path, and wired every terminal WebSocket event to reject the awaited promises.

Security risks

None introduced by the test file. The WebKit bump changes vendored JS-engine code; the specific patch (returning nullptr for BigInt/Symbol in the inspector's JSON serializer and changing an error string) is narrow, but any WebKit version change is engine-level and I cannot review the upstream diff from here.

Level of scrutiny

High, because of the vendored-dependency bump. REVIEW.md flags "Dependencies & vendoring" as a situational area needing its own checklist, and the JS engine is the most load-bearing dependency in the tree. The Bun-side test changes on their own would be low-scrutiny — they follow existing patterns in the same file, are hermetic (spawn a local inspectee on 127.0.0.1:0, no network), and assert exact error shape rather than substring matches.

Other factors

• WEBKIT_VERSION is currently autobuild-preview-pr-384-0cd81acb, which the PR body and author explicitly say must be swapped for the merged main sha before this lands. Approving now would greenlight a preview pin.
• My two prior inline comments (third copy of the spawn/discover block; cleanup-ordering regression from the hoist) were both addressed in ca97e16 and 34087f9 respectively; I re-read the resulting code and both fixes look correct.
• The describe("websocket", ...) block still has its own inline copy of the discovery loop — the author left it because it parameterises --inspect=... per test case, which is a reasonable scope boundary for this PR.
• No bugs were reported by the bug-hunting pass on the current head.